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

An alternative idea of how to suppress client spans per instrumentati… #3734

Conversation

mateuszrzeszutek
Copy link
Member

…on type

An alternative proposition of #3691

This PR solves 2 problems that I found in #3691 (and introduces a few new ones... 😅 )

  • ease of context key bridging - I've used static final ContextKey constants that should make bridging much easier. The resulting implementations is a bit more complex, but on the other hand - most users don't need to care about that.
  • duplication of logic between ServerSpan/ClientSpan and InstrumentationType - well it's just a few first steps to remove that duplication, but I guess you can see the idea of how to do that in this PR.

I didn't even touch the configuration part, which is bound to make the whole thing a tad more complex. And I've enclosed a few questions about how we should handle ServerSpan/ConsumerSpan - maybe it's not really worth it to try to categorize them? Maybe we should just introduce a new "local root span" just for those two. That'd probably leave us with categories just for client/outgoing calls.

Oh, and I used the name InstrumentationCategory instead of InstrumentationType: my initial idea was to have instrumenters "tagged" with 0+ categories (e.g. db and client), but it turned out to be very problematic (trying to determine if a span of all given categories is already in the context would be a major pain) so I went back to exactly 1 category.

Anyway, please take a look at it; the main purpose of this PR is the discussion around this topic.
@lmolkova feel free to take any part of this PR that you find useful.

@mateuszrzeszutek
Copy link
Member Author

Closing as it's probably not needed anymore

@mateuszrzeszutek mateuszrzeszutek deleted the instrumentation-category-poc branch November 18, 2022 10:26
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

1 participant