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

Opentelemetry server templating does not work with template in class Path annotation #18251

Closed
hacst opened this issue Jun 29, 2021 · 3 comments · Fixed by #19882
Closed

Opentelemetry server templating does not work with template in class Path annotation #18251

hacst opened this issue Jun 29, 2021 · 3 comments · Fixed by #19882
Assignees
Labels
area/tracing kind/bug Something isn't working
Milestone

Comments

@hacst
Copy link
Contributor

hacst commented Jun 29, 2021

Describe the bug

I'm not sure whether this should even be allowed, but when adding a Path annotation with a template parameter to a class the server span contains the actual path of the request instead of the templated version.

According to @kenfinnigan it is worth figuring out what is supposed to happen here.

Expected behavior

Either Quarkus should complain about the template parameter in that place or the template should be correctly replaced.

Actual behavior

The actual url without replacement is used as the name of the server span.

To Reproduce

The problem is reproducible with a resource similar to:

@Path("/tracing/filter/{dummy}")
public static class TestResource {
  @GET
  public String get(@PathParam("dummy") String dummy) {
    // ...
  }
}

The resulting name of the span has the full path of e.g. /tracing/filter/1234 instead of the template /tracing/filter/{dummy}.

in contrast templating is working fine with

@Path("/tracing")
public static class TestResource {
  @GET
  @Path("/filter/{dummy}")
  public String get(@PathParam("dummy") String dummy) {
    // ...
  }
}

Quarkus version or git rev

2.0.0.Final

@hacst hacst added the kind/bug Something isn't working label Jun 29, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 29, 2021

/cc @kenfinnigan

@kenfinnigan
Copy link
Member

From a brief look it's valid for @Path on a class to have templated values, so looks like a bug in determining templated paths.

@geoand
Copy link
Contributor

geoand commented Jul 1, 2021

From a brief look it's valid for @Path on a class to have templated values, so looks like a bug in determining templated paths.

Yes, it is valid - albeit not widely used :)

@kenfinnigan kenfinnigan self-assigned this Sep 2, 2021
kenfinnigan added a commit to kenfinnigan/quarkus that referenced this issue Sep 3, 2021
- Fixes quarkusio#18251
- Add test to Micrometer integration test. Verified not a problem with RESTEasy Reactive
- Add test to OpenTelemetry integration test to very fix for RESTEasy Classic
kenfinnigan added a commit to kenfinnigan/quarkus that referenced this issue Sep 3, 2021
- Fixes quarkusio#18251
- Add test to Micrometer integration test. Verified not a problem with RESTEasy Reactive
- Add test to OpenTelemetry integration test to very fix for RESTEasy Classic
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Sep 3, 2021
@gsmet gsmet modified the milestones: 2.3 - main, 2.2.2.Final Sep 6, 2021
gsmet pushed a commit to gsmet/quarkus that referenced this issue Sep 6, 2021
- Fixes quarkusio#18251
- Add test to Micrometer integration test. Verified not a problem with RESTEasy Reactive
- Add test to OpenTelemetry integration test to very fix for RESTEasy Classic

(cherry picked from commit d57ddba)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Sep 6, 2021
- Fixes quarkusio#18251
- Add test to Micrometer integration test. Verified not a problem with RESTEasy Reactive
- Add test to OpenTelemetry integration test to very fix for RESTEasy Classic

(cherry picked from commit d57ddba)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Sep 6, 2021
- Fixes quarkusio#18251
- Add test to Micrometer integration test. Verified not a problem with RESTEasy Reactive
- Add test to OpenTelemetry integration test to very fix for RESTEasy Classic

(cherry picked from commit d57ddba)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tracing kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants