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

HTTP error raised out of controller results in bad transaction.name mapping - quarkus rest reactive client #8873

Closed
bcoquell opened this issue Jul 5, 2023 · 7 comments · Fixed by #8891 or #8998
Labels
bug Something isn't working

Comments

@bcoquell
Copy link

bcoquell commented Jul 5, 2023

Describe the bug
When an http 400 error for example is generated outside of the controller of a Quarkus application (client rest reactive in my case) the transaction name is / instead of the /api/name (/v1/profileData). The mapping error comes for example when the error is raised in a method with this kind of priority :
@priority(Priorities.AUTHORIZATION)
public class CustomFilter implements ContainerRequestFilter

Steps to reproduce
As I made for bug #8319 you will find a sample app here :
https://github.com/bcoquell/quarkus-sampleapp-otel-400-badRequest

sample app :
https://github.com/bcoquell/quarkus-sampleapp-otel-400-badRequest/blob/main/quarkus-app.zip

Details of observation :
https://github.com/bcoquell/quarkus-sampleapp-otel-400-badRequest/blob/main/400-details-and-test-results.pdf

Details to reproduce :
https://github.com/bcoquell/quarkus-sampleapp-otel-400-badRequest/blob/main/howto-launch.txt

What did you expect to see?
A good mappinf of transaction.name in both cases (400 generated inside/outside the controller)

What did you see instead?
/ instead of /v1/profileData in the transaction.name

What version are you using?
agent opentelemetry 1.27.0

Environment
Java / Quarkus 2.7.5 final / Openshift / Elastic APM Server as collector
Compiler: (temurin 17.0.6)
OS: (Openshift cluster - image built with eclipse-temurin:17)

Additional context
Thanks for your help !

@bcoquell
Copy link
Author

bcoquell commented Jul 13, 2023

Hi, I just made a test with 1.28 version of agent, but I still have the problem describe in the ticket when I use my sample app + agent.

image

image

Thanks for your help @laurit @mateuszrzeszutek and all ;)

BR,

Baptiste

@laurit
Copy link
Contributor

laurit commented Jul 20, 2023

@bcoquell could you try the latest snapshot from https://oss.sonatype.org/content/repositories/snapshots/io/opentelemetry/javaagent/opentelemetry-javaagent/1.29.0-SNAPSHOT/ and let us know whether it works now

@bcoquell
Copy link
Author

bcoquell commented Jul 21, 2023 via email

@bcoquell
Copy link
Author

bcoquell commented Jul 21, 2023 via email

@trask
Copy link
Member

trask commented Jul 21, 2023

It would be nice to have correct mapping in such cases, for example to identify easily bad client requests for example.

I think this would violate the low-cardinality requirement of span names?

Is-it possible to check this in the context of the ticket please ? or would you prefer I raise an other one.

yes, please open a new issue if you'd like to discuss since this is a different (underlying) issue

@laurit
Copy link
Contributor

laurit commented Jul 22, 2023

@bcoquell not implemented endpoints having a route of / is the intended behavior because the specification https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#name requires route to have low cardinality.

@bcoquell
Copy link
Author

bcoquell commented Jul 24, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants