-
Notifications
You must be signed in to change notification settings - Fork 565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for MP Metrics 2.3 #2245
Conversation
…mer to data structures Signed-off-by: [email protected] <[email protected]>
…g to MP metrics 2.3 Signed-off-by: [email protected] <[email protected]>
…mer to data structures Signed-off-by: [email protected] <[email protected]>
…y-annotated methods; not yet working
…eTimer metric for each JAX-RS method
…ints to trigger REST.request SimpleTimer updates
…ts; defaults to false
Signed-off-by: [email protected] <[email protected]>
…ng metrics Signed-off-by: [email protected] <[email protected]>
…cs to microprofile/tests/tck/tck-metrics so it cannot even accidentally be used by developers
…REST.request metrics
…ickstart uses of REST.request
…od returning the current flavor but instead removed the method that sets the flavor -- which is essential
…e archetype to emphasize that the feature is there without turning it on.
@@ -166,6 +184,19 @@ curl -H "Accept: application/json" http://localhost:8080/metrics/vendor/grpc.re | |||
|
|||
NOTE: You cannot get the individual fields of a metric. For example, you cannot target http://localhost:8080/metrics/vendor/grpc.requests.meter.count. | |||
|
|||
==== Controlling `REST.request` metrics | |||
Helidon implements the optional family of metrics, all with the name `REST.request`, as described in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a link here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sigh. I had started to include one and got distracted and forgot to. Adding it.
|
||
@Override | ||
public void jsonData(JsonObjectBuilder builder, MetricID metricID) { | ||
JsonObjectBuilder myBuilder = JSON.createObjectBuilder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dim recollection here that this is quite expensive to invoke with every call; I seem to recall some other class that can be cached statically to give you a new builder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of our metric implementations use the same pattern. The JSON
field (somewhat misnamed) is declared this way:
static final JsonBuilderFactory JSON =
Json.createBuilderFactory(Collections.emptyMap());
IIRC the real performance drain was in the createBuilderFactory
call but it probably would be good to review all uses of this pattern to see if we can speed things up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the way to do it. We need to cache the builder factory. JsonObjectBuilder
itself is mutable and cannot be used from more than one thread (as it builds a single JSON Object).
= Arrays.asList(Counted.class, Metered.class, Timed.class, Gauge.class, ConcurrentGauge.class, | ||
SimplyTimed.class); | ||
|
||
private static final List<Class<? extends Annotation>> JAX_RS_ANNOTATIONS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably good enough. Note that strictly speaking a JAX-RS annotation is one that is, in turn, annotated with HttpMethod
. I'd probably leave this line alone anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to make sure I understand.
I think you're suggesting that instead of iterating through the individual annotation classes {GET.class, etc.) to see if one is present on a given method we could look for
HttpMethod.class`. Is that it?
Related, the @Observes @WithAnnotations
clause currently lists the individual JAX-RS-related annotations (GET.class
, etc.) in the method declaration. Could we also use HttpMethod.class
there?
Resolves #2140
REST.request
overviewQuite a few of the changes below support the
REST.request
optional feature, in which each JAX-RS endpoint is automatically measured by a distinctSimpleTimer
metric. Here's the overview of how that's implemented:SimpleTimer
metric for each endpoint.SimpleTimer
instance and uses it to measure the work done in the intercepted method.Summary of changes by module
metrics/metrics
Add
HelidonSimpleTimer
impl of newSimpleTimer
metricUpdate
Registry
withSimpleTimer
-related methodsNew tests for
SimpleTimer
microprofile/metrics
Add infrastructure for
@SimplyTimed
in various classesAdd internal
@SyntheticSimplyTimed
; marker interface so CDI will trigger interception.Add interceptor for above that looks up or creates the matching
SimpleTimer
to update.Update CDI extension:
@SyntheticSimplyTimed
to each JAX-RS endpoint method so CDI will automatically invoke the corresponding interceptor.@ProcessAnnotatedType
) and then register (during@AfterDeploymentValidation
) theSimpleTimer
metrics for each method with@SyntheticSimplyTimed
Add tests
tck/tck-metrics
Add dependency to include optional tests
Because MP metrics 2.3.2 uses non-standard constructs, add an
ArrayParamConverter
and its provider to CDIAdd CDI extension to register the converter provider
Add
UrlResourceProvider
as in several other TCK projects; TCK fetches deployment URIexamples/archetypes
Add config setting enabling REST.request behavior in MP quickstart example and archetype.
Add explicit config setting to disable REST.request in other examples as a reminder that the feature is there without turning it on.
docs
MP metrics guide
Update example output because REST.request metrics now appear
Add brief section describing config control of the feature
Add brief discussion of
SimpleTimer
and@SimplyTimed
SE metrics guide
Add brief discussion of
SimpleTimer