Skip to content

Micrometer to OpenTelemetry Bridge #43831

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 23, 2025
Merged

Micrometer to OpenTelemetry Bridge #43831

merged 2 commits into from
Jan 23, 2025

Conversation

@brunobat
Copy link
Contributor

@brunobat brunobat commented Oct 11, 2024 •
edited
Loading

solves #43621

The implementation uses the opentelemetry-micrometer-1.5 library.

This allows us to create a Micrometer registry implemented with the OpenTelemetry SDK and APIs.

All telemetry data created with both Micrometer and OpenTelemetry will be processed by the same OpenTelemetry SDK instance from the quarkus-opentelemetry extension and will use it's configuration and exporters.

quarkus-micrometer extension configurations should also work, as verified on some tests.

Missing:

  • Native tests
  • Tests with exemplars using OTel Tracing.
  • Documentation updates
  • Decide what to do with the OTel metrics auto instrumentation
  • Move the code to the OTel extension? No.
  • Disable x.histogram in DistributionSummary
  • Silence io.ope.ins.mic.v1_.OpenTelemetryMeterRegistry] (main) A MeterFilter is being configured ...
  • Enable otel metrics and disable otel metrics instrumentation

@quarkus-bot
Copy link

quarkus-bot bot commented Oct 11, 2024

/cc @ebullient (micrometer)

@github-actions
Copy link

github-actions bot commented Oct 11, 2024 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@brunobat brunobat mentioned this pull request Oct 11, 2024
Closed
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from 7b7cc9f to acd0b25 Compare December 18, 2024 17:23
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from acd0b25 to 35f4d7a Compare January 2, 2025 17:41
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch 3 times, most recently from 73b8d37 to b7ae30d Compare January 13, 2025 11:13
@quarkus-bot quarkus-bot bot added area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/tracing labels Jan 13, 2025
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch 2 times, most recently from d384469 to 7375f46 Compare January 13, 2025 11:49
@brunobat brunobat marked this pull request as ready for review January 13, 2025 11:49
@brunobat brunobat requested review from alesj and radcortez January 13, 2025 11:49
brunobat
brunobat commented Jan 13, 2025
View reviewed changes
.../main/java/io/quarkus/micrometer/opentelemetry/deployment/MicrometerOtelBridgeProcessor.java Outdated
// // registered to this registry...
// // It's unavoidable because of how Quarkus startup works and users cannot do anything about it.
// // see: https://github.com/micrometer-metrics/micrometer/issues/4920#issuecomment-2298348202
// systemProperty.produce(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@radcortez I wonder why this didn't work here... I had to set these properties in the runtime application.properties

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from 7375f46 to cece5ed Compare January 13, 2025 12:41
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from cece5ed to 6647f75 Compare January 13, 2025 14:13
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from 6647f75 to 17812ae Compare January 14, 2025 11:46
@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Jan 14, 2025
@quarkus-bot

This comment has been minimized.

Sign in to view
geoand
geoand reviewed Jan 21, 2025
View reviewed changes
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from 3bfddac to 02659ec Compare January 21, 2025 09:38
@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand geoand force-pushed the micrometer-to-otel-bridge branch from 02659ec to 53c8374 Compare January 21, 2025 11:26
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat
Copy link
Contributor Author

Thanks @geoand :)

@geoand
Copy link
Contributor

geoand commented Jan 21, 2025

🙏🏽

@geoand geoand mentioned this pull request Jan 21, 2025
Merged
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from 53c8374 to 47d5c7b Compare January 21, 2025 16:42
@brunobat
Copy link
Contributor Author

rebased

@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat
Copy link
Contributor Author

There's something wrong with the branch... It's not piking up the conflicts on native-tests.json.
Do not merge.

@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand @brunobat
Ensure logging is setup before Micrometer configures registries
e6b38d8 
This is done because we need sometimes need to tweak the
logging categories and the corresponding build items
have no effect if logging has not yet been setup
@brunobat
Copy link
Contributor Author

All good after all. Holly's native-tests.json reorg is not on main and I got confused.
@gsmet are you ok with this one being merged?

gsmet
gsmet reviewed Jan 22, 2025
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I added a few minor suggestions, questions, mostly to the doc.

I would recommend fixing all the easy stuff before merging and then we can discuss the remaining points after merge if they require some discussion.

@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from 47d5c7b to a6c8a69 Compare January 22, 2025 13:12
@brunobat
Copy link
Contributor Author

Addressed the review @gsmet

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 22, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit a6c8a69.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 22, 2025 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit a6c8a69.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/micrometer/deployment

✖ io.quarkus.micrometer.deployment.binder.VertxHttpClientMetricsTest.testWebClientMetrics - History

  • event executor terminated - java.util.concurrent.RejectedExecutionException
java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:934)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:353)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:346)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:836)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:827)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:817)
	at io.vertx.core.impl.EventLoopExecutor.execute(EventLoopExecutor.java:35)

@gsmet gsmet merged commit d63ac25 into quarkusio:main Jan 23, 2025
55 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.19 - main milestone Jan 23, 2025
@brunobat brunobat mentioned this pull request Jan 24, 2025
Merged
@gsmet
Copy link
Member

gsmet commented Jan 24, 2025

I just saw this had the backport label, I don't think we can backport it, it's too much of a change, it's going to be for 3.19.

@brunobat
Copy link
Contributor Author

That's fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkouba mkouba mkouba left review comments

@manovotn manovotn manovotn left review comments

@radcortez radcortez radcortez left review comments

@gsmet gsmet gsmet approved these changes

@geoand geoand geoand approved these changes

@alesj alesj Awaiting requested review from alesj

Assignees

No one assigned

Labels

area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/metrics area/tracing release/noteworthy-feature triage/flaky-test

Projects

Quarkus Roadmap/Planning
Status: Done
WG - Quarkus 3.20 LTS
Status: Done

Milestone

3.19.0.CR1

Development

Successfully merging this pull request may close these issues.

6 participants

@brunobat @geoand @gsmet @mkouba @manovotn @radcortez