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

OTel metrics for Microprofile Telemetry 2.0 #43678

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

brunobat
Copy link
Contributor

@brunobat brunobat commented Oct 3, 2024

Copy link

quarkus-bot bot commented Oct 3, 2024

/cc @radcortez (opentelemetry)

@brunobat brunobat changed the title OTel metrics for Microprofile 2.0 OTel metrics for Microprofile Telemetry 2.0 Oct 3, 2024
@brunobat brunobat marked this pull request as draft October 3, 2024 11:07
@brunobat
Copy link
Contributor Author

brunobat commented Oct 3, 2024

Will include the TCK updates as well... Set to draft until then.

Copy link

github-actions bot commented Oct 3, 2024

🙈 The PR is closed and the preview is expired.

@Ladicek
Copy link
Contributor

Ladicek commented Oct 3, 2024

Off topic (but not all that much I guess): are you planning to support MetricsCapabilityBuildItem? (And the related SPIs too, probably?)

@brunobat
Copy link
Contributor Author

brunobat commented Oct 3, 2024

Off topic (but not all that much I guess): are you planning to support MetricsCapabilityBuildItem? (And the related SPIs too, probably?)

I'll decide that after evaluating the future Micrometer to OpenTelemetry bridge.
It it goes well, there might be no more automatic metrics for OTel.

@brunobat brunobat force-pushed the otel-jdk-metrics branch 2 times, most recently from c071e72 to cd37073 Compare October 8, 2024 16:36
@brunobat
Copy link
Contributor Author

brunobat commented Oct 8, 2024

#43752 is needed for the native image tests to pass because it includes an OTel side instrumentation fix.

This comment has been minimized.

This comment has been minimized.

@brunobat brunobat self-assigned this Oct 10, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

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.

The changes look good to me except for some minor nitpicking in the doc - but I might have misunderstood the sentence.

As for the failing test, should we raise an issue in the TCK and for now ignore this test?

docs/src/main/asciidoc/opentelemetry-metrics.adoc Outdated Show resolved Hide resolved
@brunobat
Copy link
Contributor Author

Created eclipse/microprofile-telemetry#241

@radcortez
Copy link
Member

Created eclipse/microprofile-telemetry#241

You can also exclude the failing test (because of the challenge) so we can proceed with the PR. Check:
https://github.com/smallrye/smallrye-opentelemetry/blob/a750353e55ccede3fe498bbc693be21621dac7a5/testsuite/tck/pom.xml#L97-L105

Copy link

quarkus-bot bot commented Oct 15, 2024

Status for workflow Quarkus Documentation CI

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

✅ 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 Oct 15, 2024

Status for workflow Quarkus CI

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

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

@gsmet
Copy link
Member

gsmet commented Oct 16, 2024

@radcortez I will let you review this one. I could be convinced to shoe it in 3.16.0 post CR1 if we think it has high value for 3.16.

@radcortez
Copy link
Member

We want this for MP 7.0 compatibility. Let me check what is the state of the missing pieces. If the other pieces are not ready, we don't need to rush this.

@brunobat brunobat merged commit f998b73 into quarkusio:main Oct 17, 2024
32 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants