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

Add http.route to the server span when ServerSpanNaming is updated #5086

Merged
merged 4 commits into from
Jan 14, 2022

Conversation

mateuszrzeszutek
Copy link
Member

Another part of #442 (which actually mostly resolves that issue, but this still needs a couple of refactorings to look good, so I won't close it until it's all done)

This PR adds http.route attribute to spans (no metrics yet; another PR) and introduces several TODOs - I'll track them in #442.

@mateuszrzeszutek mateuszrzeszutek requested a review from a team as a code owner January 12, 2022 12:21
@@ -100,6 +99,7 @@ class RestCamelTest extends AgentInstrumentationSpecification implements RetryOn
"$SemanticAttributes.NET_PEER_PORT" Long
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" String
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we assert the value for route?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added. Turns out camel isn't using ServerSpanNaming to update the server span name and the route is incorrect -- another TODO added to my list.

Comment on lines -39 to +43
assert value =~ expected
assert value =~ expected, "value '$value' does not match regex '$expected'"
} else if (expected instanceof Class) {
assert ((Class) expected).isInstance(value)
assert ((Class) expected).isInstance(value), "value '$value' is not an instance of $expected.name"
} else if (expected instanceof Closure) {
assert ((Closure) expected).call(value)
assert ((Closure) expected).call(value), "value '$value' fails the passed predicate"
Copy link
Member

Choose a reason for hiding this comment

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

👍

@trask trask merged commit 872c6c7 into open-telemetry:main Jan 14, 2022
@mateuszrzeszutek mateuszrzeszutek deleted the http-route branch January 17, 2022 08:50
RashmiRam pushed a commit to RashmiRam/opentelemetry-auto-instr-java that referenced this pull request May 23, 2022
open-telemetry#5086)

* Add `http.route` to the server span when `ServerSpanNaming` is updated

* fix camel tests

* fix test compilation failure

* assert route in camel instrumentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants