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

Implement Micrometer integration, with some new abstractions for interceptors and retrofit to MP metrics #2873

Merged
merged 79 commits into from
Apr 13, 2021

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented Mar 24, 2021

Resolves #2344.

This is a re-work of #2761 which is now obsolete.

Although many files are affected, I used the first few commits to "chunk" the changes into more reasonably reviewable sections. This PR has evolved in parallel with #2868, including some rebases. The original chunks here are still conceptually valid although some things (like renaming some parameters and classes) appear later in the PR history.

There would have been considerable overlap between MP metrics and Micrometer code. Much of this PR abstracts common behavior out into interfaces and classes in the new service-common/rest and service-common/rest-cdi components and then makes use of those from SE and MP metrics and the Micrometer integration.

PR #2868 abstracted quite a bit of common behavior for interceptors into separate classes and interfaces. This PR:

  • Moves the previously-abstracted classes and interfaces into the new common modules, adjusting SE and MP metrics to use them in their new locations.
  • Adds Micrometer support using the abstractions.
  • Adds doc and examples.

What follows is a much longer description of the work in this PR.

A Possibly Digestible Guided Review

This PR is similar to its earlier incarnation as #2761.

Because the original metrics SE and MP components share so much with their new Micrometer counterparts, this PR combined with the earlier MP-only PR #2868 (which added some abstracting and async support) together have three missions:

  • refactor common behavior into the new service-common modules,
  • migrate the existing metrics implementation to the new model, and
  • build the new implementation for Micrometer on the new model.

Abstractions

Look at the new service-common modules rest and rest-cdi.

service-common/rest

Contains HelidonRestServiceSupport, an abstract superclass for xxxSupport SE classes. It handles config-based customization of the service’s endpoint using web-contextand sets up and configures CORS for the endpoint.

service-common/rest-cdi

  • HelidonRestCdiExtension - abstract superclass for CDI extensions which themselves use xxxSupport classes. Helps in creating the underlying xxxSupport service, discarding vetoed types or beans from annotation processing, and more.
  • HelidonInterceptor - interface applicable to interceptors in many components - Has several default methods. Provides both “before-only” and “before-and-after” semantics around intercepted invocations.
  • InterceptionRunner and its implementation - Not used outside the package, but used by HelidonInterceptor to invoke the distinct behavior in the interceptors and invoke the intercepted Executable. Automatically supports asynchronous JAX-RS endpoints, invoking each interceptor’s post-completion action only after the async work has completed.

Those of you who reviewed the earlier MP-only PR will notice that the public HelidonRestCdiExtension and HelidonInterceptor expose a smaller and simpler surface area than their predecessors from the earlier PR. The runner interface and implementation are now invisible outside the package.

The evolved metrics components

  • metrics/metrics - The SE MetricsSupport class has migrated to use the new abstraction.
  • microprofile/metrics - The MP CDI extension is shorter and the interceptors are much simpler (and should be faster, and also honor any removals or adjustments to the annotations that other CDI extensions might have made).

The new integrations/micrometer components

  • /micrometer (SE) for the new MicrometerSupport class
  • /cdi (MP) for the new extension and interceptors

Other pieces

  • Doc - docs/se/metrics and docs/mp/metrics. It seemed best to add the new doc for Micrometer support under the metrics sections, thinking that users might well go there first looking for information on Micrometer.
  • Examples - examples/integrations/micrometer/se and …/mp

The Helidon Micrometer implementation largely follows the plan laid out in this gist: https://github.com/oracle/helidon/wiki/Options-for-Micrometer-in-Helidon (see the “short-term” option).

…cutable (method or constructor); use that from the CDI extension in finding and storing all interception points and (importantly) the metrics to be updated at each; revise some utility methods that retreive annotation info from executables to return more than one instance
@tjquinno tjquinno added this to the 2.3.0 milestone Mar 24, 2021
@tjquinno tjquinno self-assigned this Mar 24, 2021
@tjquinno tjquinno marked this pull request as draft March 24, 2021 21:05
…method but leave it to concrete impl classes to use it to qualify recording producers (rather than always imposing that restriction)
… to concrete impls of HelidonRestCdiExtension, rather than in its observeManagedBeans method
…icrometer to metrics - se/metrics and mp/metrics are where the Micrometer pages are
…e and remove it from the metrics and micrometer subclasses; change MetricsInterceptorBase extend HelidonInterceptor.Base (and remove now-superfluous method impls)
spericas
spericas previously approved these changes Apr 8, 2021
Copy link
Member

@spericas spericas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tjquinno tjquinno merged commit a5dedcf into helidon-io:master Apr 13, 2021
@tjquinno tjquinno deleted the async-with-common branch April 13, 2021 17:22
aseovic pushed a commit to aseovic/helidon that referenced this pull request Apr 26, 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.

Micrometer support
3 participants