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.

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

Micrometer exemplars on HTTP #44016

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Micrometer exemplars on HTTP #44016

merged 1 commit into from
Nov 13, 2024

Conversation

brunobat
Copy link
Contributor

Fixes #43845

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Oct 22, 2024

/cc @ebullient (micrometer)

@brunobat brunobat requested a review from tsegismont October 22, 2024 09:44
@brunobat
Copy link
Contributor Author

CC @ahus1

@ahus1
Copy link
Contributor

ahus1 commented Oct 22, 2024

Thank you for this PR, @brunobat! These code changes are beyond my Vert.x/Quarkus skills, thank you for looking into this! I see what the test case is doing, and that part I can understand and it looks good to me.

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat brunobat force-pushed the micrometer-exemplars-on-http branch from 43b6f07 to 04ab215 Compare October 23, 2024 11:30
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat brunobat requested a review from ebullient October 23, 2024 14:13
tsegismont
tsegismont approved these changes Oct 25, 2024
View reviewed changes
Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

LGTM with one question about the unwrapper implementation.

...va/io/quarkus/micrometer/runtime/export/exemplars/OpenTelemetryExemplarContextUnwrapper.java Outdated Show resolved Hide resolved
@brunobat
Copy link
Contributor Author

Thanks for the reviews. I'm hold in other issues, will come back to this soon.

@brunobat brunobat force-pushed the micrometer-exemplars-on-http branch from 04ab215 to fffb6cb Compare November 13, 2024 09:54
@brunobat brunobat requested a review from cescoffier November 13, 2024 09:55
@brunobat
Copy link
Contributor Author

@cescoffier should be fixed now.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Nov 13, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

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

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

@brunobat brunobat merged commit f02d1ad into quarkusio:main Nov 13, 2024
28 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 13, 2024
@tsegismont
Copy link
Contributor

Thank you @brunobat !

@ahus1
Copy link
Contributor

ahus1 commented Nov 14, 2024

@brunobat - thank's a lot, looking forward to try it out!

@brunobat
Copy link
Contributor Author

brunobat commented Nov 22, 2024 •
edited
Loading

@gsmet There are request to backport this PR to 3.15.

@gsmet gsmet modified the milestones: 3.18 - main, 3.17.1 Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsegismont tsegismont tsegismont approved these changes

@cescoffier cescoffier cescoffier approved these changes

@ebullient ebullient Awaiting requested review from ebullient

Assignees
No one assigned
Labels
area/metrics kind/bugfix triage/backport-3.15 triage/flaky-test
Projects
None yet
Milestone
3.17.2
Development

Successfully merging this pull request may close these issues.

Exemplars are not recorded for metric http_server_requests*
5 participants
@brunobat @ahus1 @tsegismont @cescoffier @gsmet