Skip to content
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 the KPI metrics handler (with no qualifying path) exactly once to each routing #3255

Merged
merged 4 commits into from
Aug 13, 2021
Merged

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented Aug 6, 2021

Resolves #3240

The extended KPI metrics implementation in microprofile/server adds a handler in the chain that places a "deferrable" KPI metrics context in the request context. (Later, JerseySupport retrieves and updates the metrics context.)

Previously, the code that added the handler would see if the Application had an explicit setting for the path/context and, if so, would use that in adding the handler which added the metrics context to the request.

That was the problem--because the paths within that context would not match the routing for the handler, the KPI metrics context would (incorrectly) not be added to the request context.

The fix always adds this context-establishing handler to each routing so all requests will have the KPI metrics context set up and so all requests will update the metrics context (and the metrics it delegates to).

In fact, it was slightly inefficient to use the application path in the first place. In the rare (but possible) case of multiple JAX-RS apps, the old approach would have inserted the context-adding handler once for each application. This would not have caused the KPI metrics to be updated multiple times because the attempt to add the KPI context to the request context multiple times would have removed all the but last one, but we might as well avoid that inefficiency on each request.

To avoid this problem, the code makes sure to insert the context-adding handler only once to each named routing or the unnamed routing.

The PR also includes a test which configures the executor with 1 thread and blasts two concurrent requests to make sure the deferred metric is updated and will show a non-zero count.

@tjquinno tjquinno added this to the 2.4.0 milestone Aug 6, 2021
@tjquinno tjquinno requested a review from spericas August 6, 2021 20:04
@tjquinno tjquinno self-assigned this Aug 6, 2021
@tjquinno tjquinno changed the title For each JAX-RS app, always add the KPI metrics handler with no path Always add the KPI metrics handler (with no qualifying path) to each routing Aug 12, 2021
@tjquinno tjquinno changed the title Always add the KPI metrics handler (with no qualifying path) to each routing Add the KPI metrics handler (with no qualifying path) to each routing (and do so only once) Aug 12, 2021
@tjquinno tjquinno changed the title Add the KPI metrics handler (with no qualifying path) to each routing (and do so only once) Add the KPI metrics handler (with no qualifying path) exactly once to each routing Aug 13, 2021
@tjquinno tjquinno merged commit f295765 into helidon-io:master Aug 13, 2021
@tjquinno tjquinno deleted the kpi-deferred branch August 13, 2021 17:00
tjquinno added a commit that referenced this pull request Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Key performance indicator (KPI) "deferred" metric always 0 if explicit Application class used
2 participants