Skip to content

Conversation

trask
Copy link
Member

@trask trask commented Oct 3, 2025

Part of #7345

@trask trask marked this pull request as ready for review October 3, 2025 21:41
@trask trask requested a review from a team as a code owner October 3, 2025 21:41
Comment on lines +319 to +323
span.hasName(spanName)
.hasKind(SpanKind.INTERNAL)
.hasAttributesSatisfyingExactly(
codeFunctionAssertions(
"io.opentelemetry.javaagent.instrumentation.ratpack.TracingHandler", "handle"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember we had a similar discussion a while ago (in the context of #7345), but do you think we could capture the user/application code that implements the handler instead of the ratpack implementation class ?

While it is not written explicitly in semantic conventions, giving priority to classes/methods of the application is expected to provide "more value" in the sense that the consumer of this data has more leverage on the application implementation than on the underlying framework.

We could also implement a few alternatives:

  • capture two spans: one for the framework, one for the "user/application code", the downside is that they would always have similar duration, but it could allow to compare usages of the same framework feature across applications for example
  • extend semantic conventions to provide a clear split between "technical" or "framework" code attributes and the "application".

In think that having a common strategy to capture it would probably be beneficial, especially if we want to move forward the work on #7345.

Copy link
Contributor

Choose a reason for hiding this comment

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

I totally agree that adding TracingHandler here isn't useful and using the actual user class that handles the request would be much better. The only problem is that getting that class isn't easy.

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.

3 participants