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

Micrometer to OpenTelemetry Bridge #43831

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

brunobat
Copy link
Contributor

@brunobat brunobat commented Oct 11, 2024

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

Copy link

quarkus-bot bot commented Oct 11, 2024

/cc @ebullient (micrometer)

Copy link

github-actions bot commented Oct 11, 2024

🙈 The PR is closed and the preview is expired.

@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
// // 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

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

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

This comment has been minimized.

This comment has been minimized.

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

This comment has been minimized.

This comment has been minimized.

@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

This comment has been minimized.

@brunobat brunobat force-pushed the micrometer-to-otel-bridge branch from 3bfddac to 02659ec Compare January 21, 2025 09:38

This comment has been minimized.

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

This comment has been minimized.

@brunobat
Copy link
Contributor Author

Thanks @geoand :)

@geoand
Copy link
Contributor

geoand commented Jan 21, 2025

🙏🏽

This comment has been minimized.

@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

This comment has been minimized.

@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.

This comment has been minimized.

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?

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.

integration-tests/micrometer-opentelemetry/pom.xml Outdated Show resolved Hide resolved
integration-tests/micrometer-opentelemetry/pom.xml Outdated Show resolved Hide resolved
@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

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.

Copy link

quarkus-bot bot commented Jan 22, 2025

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
@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
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
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants