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

Use URL path template when tracing REST clients where possible #39534

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

luneo7
Copy link
Contributor

@luneo7 luneo7 commented Mar 18, 2024

Use URL path template in the REST client span names where possible.

Using raw Vert.x web client is not possible right now since all the instrumentation happens after UriTemplate is applied so we have only the full URI... this would make is have infinite cardinality since a path variable can lead to infinite URLs. Ideally the Vert.x web client could expose the used UriTemplate during tracing or for metrics (as we have with VertxRoute right now) so we could extract it and build the span name with the template as well.

Now RestEasy clients using Vert.x or not will be with a span name of {method} {urlPathTemplate} as we had before =]

Copy link

quarkus-bot bot commented Mar 18, 2024

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

@geoand geoand requested a review from brunobat March 19, 2024 06:13

This comment has been minimized.

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

@luneo7 The IT tests using the client will need to be updated as well.

@luneo7 luneo7 force-pushed the rest-client-otel branch from 1b61652 to b78e585 Compare March 19, 2024 15:59
@luneo7 luneo7 force-pushed the rest-client-otel branch from b78e585 to d7b1ca5 Compare March 19, 2024 16:00
@luneo7
Copy link
Contributor Author

luneo7 commented Mar 19, 2024

@luneo7 The IT tests using the client will need to be updated as well.

Done =]

Copy link

quarkus-bot bot commented Mar 19, 2024

Status for workflow Quarkus CI

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

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

@geoand geoand requested a review from brunobat March 20, 2024 12:12
Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Thanks @luneo7!

@luneo7
Copy link
Contributor Author

luneo7 commented Mar 21, 2024

@brunobat @geoand do we need anything else to get this merged? Thanks 😊

@geoand
Copy link
Contributor

geoand commented Mar 21, 2024

@brunobat can merge it at his discression :)

@brunobat brunobat merged commit 3da6a27 into quarkusio:main Mar 21, 2024
31 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.10 - main milestone Mar 21, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Mar 21, 2024
@luneo7
Copy link
Contributor Author

luneo7 commented Mar 21, 2024

Thank you! Could this be backported as well?

@luneo7 luneo7 deleted the rest-client-otel branch March 22, 2024 16:30
@brunobat
Copy link
Contributor

Added the label.

@michalvavrik
Copy link
Member

This is breaking change and not exactly fun to look for what changed. Please add a note to the migration guide for people like me, thank you.

@michalvavrik
Copy link
Member

michalvavrik commented Mar 23, 2024

My point is that operationNames has changed, so every historical data on which you execute aggregation needs a rule that for period between x and y they should aggregate operation names xyz etc. So it is something users need to know.

@brunobat
Copy link
Contributor

Added a wiki entry. @michalvavrik

@michalvavrik
Copy link
Member

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tracing kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quarkus OpenTelemetry Rest Client Span Name with Route (URL Path Template)
6 participants