PAYARA-931 Payara Micro Request Tracing #1107

Pandrex247 · 2016-09-23T13:45:30Z

Adds in command line options to enable and configure request tracing in Payara Micro.
Also turns on the Notification service by default.

The new options are:

--enableRequestTracing - Just a flag; you don't need to add true or false afterwards.
--requestTracingThresholdUnit - Accepts a String value determining the time unit, defaulting to seconds if no value is given. The accepted strings are (case insensitive): NANOSECONDS, MICROSECONDS, MILLISECONDS, SECONDS, MINUTES, HOURS, and DAYS.
--requestTracingThresholdValue - Accepts a long to determine the threshold value before a request is traced, defaulting to 30 if no value is entered.

As an example of usage:
java -jar payaramicro.jar --enableRequestTracing --requestTracingThresholdUnit NANOSECONDS --requestTracingThresholdValue 7

payara-ci · 2016-09-23T13:45:32Z

Can one of the admins verify this patch?

payara-ci · 2016-09-23T13:45:32Z

Can one of the admins verify this patch?

smillidge · 2016-09-25T19:18:43Z

...erver/extras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/PayaraMicro.java

+                    case "--enableRequestTracing":
+                        enableRequestTracing = true;
+                        break;
+                    case "--requestTracingThresholdUnit":


There should be some validation on the the units passed in on the command line to ensure they are valid units.

smillidge · 2016-09-25T19:19:57Z

appserver/extras/payara-micro/payara-micro-core/src/main/resources/microdomain.xml

@@ -155,6 +155,12 @@ Portions Copyright [2016] [Payara Foundation and/or its affiliates]
            </transaction-service>


microdomain nocluster also need configuring correctly

entries to microdomain-nocluster.xml

Pandrex247 · 2016-09-29T08:21:58Z

Updated with required changes.
I've also made it so it accepts lowercase characters for the requestTracingThresholdUnit.

smillidge · 2016-10-06T18:06:48Z

jenkins test please

smillidge · 2016-10-07T11:09:09Z

jenkins test please

payara-ci · 2016-10-07T15:17:13Z

One or more tests have failed

michaelranaldo · 2016-10-07T15:19:20Z

jenkins test please

payara-ci · 2016-10-07T18:34:53Z

All tests have passed

jerrinot · 2016-10-07T20:46:54Z

...erver/extras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/PayaraMicro.java

+                    case "--requestTracingThresholdValue":
+                        try {
+                            requestTracingThresholdValue = 
+                                    Long.parseLong(args[i + 1]);


this smells like ArrayIndexOutOfBoundsException when a user forgets to include the actual units.

jerrinot · 2016-10-07T20:53:29Z

Two parameters for a single thing look a bit awkward.

Why not
--requestTracingThreshold 2 <-- defaults to seconds = common case

+parsing units from the value itself when needed?
--requestTracingThreshold 500ms

From the other hand It could become somewhat clumsy with microseconds (us or μs ?) or with days (1 days?)

Pandrex247 · 2016-10-11T08:16:46Z

I like the idea of a single option, we could even go full hog and combine all 3 into one (enable, time, and units) - though in either situation I'm fond of keeping the discrete options for flexibility. I'll have a play and see what I can cobble together.

I'll check for any obvious OutOfBounds cases, but I believe several of the configuration options for Payara Micro can fall afoul of this, so is probably worth its own PR to fix it rather than tacking it onto this one.

jerrinot · 2016-10-11T14:47:18Z

...erver/extras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/PayaraMicro.java

@@ -1410,6 +1431,112 @@ private void scanArgs(String[] args) {
                    case "--disablePhoneHome":
                        disablePhoneHome = true;
                        break;
+                    case "--requestTracing":


this is screaming for refactoring.

Sure. I've just noticed that this option makes the --enableRequestTracing one completely obsolete, so I'll rename it to that.

Pandrex247 · 2016-10-11T15:05:23Z

Heavily modified the --enableRequestTracing option.

The option now enables the request tracing service, and optionally configures the threshold unit and value. I've also added extra unit validation (to both this option, and the original one), allowing you to now enter:

Nanoseconds (case-insensitive), Nanosecond (case-insensitive), ns
Microseconds (case-insensitive), Microsecond (case-insensitive), us, μs
Milliseconds (case-insensitive), Millisecond (case-insensitive), ms
Seconds (case-insensitive), Second (case-insensitive), s
Minutes (case-insensitive), Minute (case-insensitive), Mins (case-insensitive), Min (case-insensitive), m
Hours (case-insensitive), Hour (case-insensitive), h
Days (case-insensitive), Day (case-insensitive), d

I decided against enforcing proper grammar and refusing options such as "3day", as not everyone cares as much as I do.

Example usages:

Enable request tracing - java -jar payara-micro.jar --enableRequestTracing
Enable request tracing and set threshold unit to nanoseconds (value defaults to 30) - java -jar payara-micro.jar --enableRequestTracing ns
Enable request tracing and set threshold value to 2 (unit defaults to seconds) - java -jar payara-micro.jar --enableRequestTracing 2
Enable request tracing and set threshold to 4 microseconds - java -jar payara-micro.jar --enableRequestTracing 4ms

In its current state, entering something like --enableRequestTracing 4 minutes will have the minutes ignored. If this is particularly wanted though, it can be done.

I've elected to not fix the potential Array index out of bounds issues in this PR, as we're looking at spinning the scanArgs method out into a proper parsing class.

Note, these options override each other without warning (like most others) - the last one entered will override the previous. For example:

java -jar payara-micro.jar --requestTracingThresholdUnit DAYS --enableRequestTracing 1ns will enable request tracing, and set the threshold to 1 Nanosecond.
java -jar payara-micro.jar --enableRequestTracing 1ns --requestTracingThresholdUnit DAYS will enable request tracing, and set the threshold to 1 Day.

smillidge · 2016-10-11T19:27:20Z

jenkins test please

payara-ci · 2016-10-11T19:27:31Z

One or more tests have failed

smillidge · 2016-10-23T17:14:40Z

jenkins test please

payara-ci · 2016-10-23T21:51:45Z

All tests have passed

* PAYARA-931 Add in non-persistent request tracing to Payara Micro * PAYARA-931 Rework to work with UberJar option * PAYARA-931 Missed Help options * PAYARA-931 Add in validation and allow lowercase. Also adds in required entries to microdomain-nocluster.xml * PAYARA-931 Correct message * PAYARA-931 Add in extra unit validation, and add in convenience option * PAYARA-931 Add help text * PAYARA-931 Formatting * PAYARA-931 Missed milliseconds * PAYARA-931 Missed minutes! * PAYARA-931 Shorthand minute * PAYARA-931 Remove obsolete option, and rename new option in place. * PAYARA-931 Formatting

Pandrex247 added 3 commits September 22, 2016 16:53

PAYARA-931 Add in non-persistent request tracing to Payara Micro

e6cda72

PAYARA-931 Rework to work with UberJar option

22dabdd

PAYARA-931 Missed Help options

7efe105

fturizo added the 3:DevInProgress label Sep 23, 2016

smillidge requested changes Sep 25, 2016

View reviewed changes

PAYARA-931 Add in validation and allow lowercase. Also adds in required

898328f

entries to microdomain-nocluster.xml

PAYARA-931 Correct message

2849bbe

smillidge approved these changes Oct 6, 2016

View reviewed changes

jerrinot reviewed Oct 7, 2016

View reviewed changes

Pandrex247 added 6 commits October 11, 2016 15:06

PAYARA-931 Add in extra unit validation, and add in convenience option

c4bdade

PAYARA-931 Add help text

41a8fa5

PAYARA-931 Formatting

939845c

PAYARA-931 Missed milliseconds

4d41aaa

PAYARA-931 Missed minutes!

e323b58

PAYARA-931 Shorthand minute

ae604b4

jerrinot reviewed Oct 11, 2016

View reviewed changes

Pandrex247 added 2 commits October 11, 2016 16:11

PAYARA-931 Remove obsolete option, and rename new option in place.

bff5252

PAYARA-931 Formatting

d55b47e

smillidge approved these changes Oct 11, 2016

View reviewed changes

smillidge merged commit deb2bd6 into payara:master Oct 24, 2016

fturizo removed the 3:DevInProgress label Oct 24, 2016

Pandrex247 deleted the PAYARA-931-Payara-Micro-Request-tracing branch November 17, 2016 09:20

OndroMih added this to the Payara 4.1.1.164 milestone Nov 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PAYARA-931 Payara Micro Request Tracing #1107

PAYARA-931 Payara Micro Request Tracing #1107

Pandrex247 commented Sep 23, 2016 •

edited

Loading

payara-ci commented Sep 23, 2016

payara-ci commented Sep 23, 2016

smillidge Sep 25, 2016

smillidge Sep 25, 2016

Pandrex247 commented Sep 29, 2016

smillidge commented Oct 6, 2016

smillidge commented Oct 7, 2016

payara-ci commented Oct 7, 2016

michaelranaldo commented Oct 7, 2016

payara-ci commented Oct 7, 2016

jerrinot Oct 7, 2016

jerrinot commented Oct 7, 2016 •

edited

Loading

Pandrex247 commented Oct 11, 2016

jerrinot Oct 11, 2016

Pandrex247 Oct 11, 2016

Pandrex247 commented Oct 11, 2016 •

edited

Loading

smillidge commented Oct 11, 2016

payara-ci commented Oct 11, 2016

smillidge commented Oct 23, 2016

payara-ci commented Oct 23, 2016

		@@ -155,6 +155,12 @@ Portions Copyright [2016] [Payara Foundation and/or its affiliates]
		</transaction-service>

PAYARA-931 Payara Micro Request Tracing #1107

PAYARA-931 Payara Micro Request Tracing #1107

Conversation

Pandrex247 commented Sep 23, 2016 • edited Loading

payara-ci commented Sep 23, 2016

payara-ci commented Sep 23, 2016

smillidge Sep 25, 2016

Choose a reason for hiding this comment

smillidge Sep 25, 2016

Choose a reason for hiding this comment

Pandrex247 commented Sep 29, 2016

smillidge commented Oct 6, 2016

smillidge commented Oct 7, 2016

payara-ci commented Oct 7, 2016

michaelranaldo commented Oct 7, 2016

payara-ci commented Oct 7, 2016

jerrinot Oct 7, 2016

Choose a reason for hiding this comment

jerrinot commented Oct 7, 2016 • edited Loading

Pandrex247 commented Oct 11, 2016

jerrinot Oct 11, 2016

Choose a reason for hiding this comment

Pandrex247 Oct 11, 2016

Choose a reason for hiding this comment

Pandrex247 commented Oct 11, 2016 • edited Loading

smillidge commented Oct 11, 2016

payara-ci commented Oct 11, 2016

smillidge commented Oct 23, 2016

payara-ci commented Oct 23, 2016

Pandrex247 commented Sep 23, 2016 •

edited

Loading

jerrinot commented Oct 7, 2016 •

edited

Loading

Pandrex247 commented Oct 11, 2016 •

edited

Loading