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

Vertx http client 3 instrumentation populating relative URL in http.url #4731

Closed
anuraaga opened this issue Nov 29, 2021 · 0 comments · Fixed by #4739
Closed

Vertx http client 3 instrumentation populating relative URL in http.url #4731

anuraaga opened this issue Nov 29, 2021 · 0 comments · Fixed by #4739
Labels
bug Something isn't working

Comments

@anuraaga
Copy link
Contributor

anuraaga commented Nov 29, 2021

We currently populating the confusingly named relative URI field as http.url

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v3_0/client/Vertx3HttpAttributesExtractor.java#L17

We hadn't noticed it because this line is incorrect.

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/vertx-http-client/vertx-http-client-3.0/javaagent/src/test/groovy/client/VertxHttpClientTest.groovy#L34

It should be httpClient.requestAbs(HttpMethod.valueOf(method), "$uri")

It seems like Jetty and older versions of Armeria handled an absolute URL as a path leniently in a way that worked. When trying to update Armeria I found this test failing with HTTP 400. It did make clear that when using the methods correctly, the instrumentation populates a relative path into that attribute.

Unfortunately there doesn't seem to be a way to access hostname publicly. Should we instrument the constructor to store it in a virtual field? Or any other tips appreciated.

@anuraaga anuraaga added the bug Something isn't working label Nov 29, 2021
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
Development

Successfully merging a pull request may close this issue.

1 participant