-
Notifications
You must be signed in to change notification settings - Fork 324
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 micrometer support #1303
Add micrometer support #1303
Conversation
- Ignore CompositeMeterRegistry - Make compatible with Micrometer 1.0
I've also tested this manually with These are the metrics it creates:
One issue is that the metrics starting with This is intentional. Quoting @graphaelli:
|
As docs go, we might want to add a section on how to visualize a timer and a gauge with Lens and/or TSVB. I suspect visualizing a timer may be problematic with Lens as we have to calculate a weighted average based on the |
Started looking into it, but there is a consistent failure in integration tests at its current state |
Thx for the notice. Should be fixed with the latest commit. Let's see... |
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.
Beautiful!
A few minor comments.
Main one - why do we serialize this differently (to a buffer first and then to the actual connection)?
apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java
Show resolved
Hide resolved
apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java
Outdated
Show resolved
Hide resolved
...crometer-plugin/src/main/java/co/elastic/apm/agent/micrometer/MicrometerMetricsReporter.java
Show resolved
Hide resolved
...crometer-plugin/src/main/java/co/elastic/apm/agent/micrometer/MicrometerMetricsReporter.java
Outdated
Show resolved
Hide resolved
...crometer-plugin/src/main/java/co/elastic/apm/agent/micrometer/MicrometerMetricsReporter.java
Outdated
Show resolved
Hide resolved
...eter-plugin/src/test/java/co/elastic/apm/agent/micrometer/MicrometerMetricsReporterTest.java
Show resolved
Hide resolved
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.
🎉
apm-agent-core/src/main/java/co/elastic/apm/agent/report/IntakeV2ReportingEventHandler.java
Show resolved
Hide resolved
Histogram support will be added later once elastic/apm-server#3195 is resolved |
What does this PR do?
Closes #893
Checklist
Added an API method or config option? Document in which version this will be introducedWe're not instrumenting a particular method in
MeterRegistry
, just all public methods. The plugin is compiled against 1.0.1 to make sure we don't call methods that have been introduced later. We're only calling public API methods that are guaranteed to be stable in 1.x. Testing against the most recent version doesn't seem necessary. I've tested with the currently most recent version 1.5 manually.