-
Notifications
You must be signed in to change notification settings - Fork 896
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
Let SDKs export all the spans regardless of their sampled flag #2986
Comments
(Re-massaged the title to have a more succinct one) |
@wdengw Hey, would you mind elaborating on the scenario that requires this? Most sampling scenarios people use these days are either head based (on the SDKs) or tail based (Collector/backend), while it seems you want something in-between (taking the |
cc @jmacd |
@wdengw I've speculated in #2918 (comment) about the need for two new sampling decision return values:
I believe this would address the need you have in mind. |
@carlosalberto The main use case is the need to collect complete information, like the number of request, the number errors, the number of DB calls, etc. for all the incoming requests that an app has served in its sidecar OTel Agent, then the Agent can based on the sampling decision made in the SDK to decide whether to store the spans or not. This way, we can minimize the tracing overhead in the hosting application. @jmacd #2986 is more focus on "Conditionally-recorded" "Unsampled-exported" is only mentioned once. Where can I find the detail about "Unsampled-exported". |
@wdengw The so-called "unsampled-exported" span state is hypothetical because the current tracing SDK specification does not allow an SDK to export a span if it was not sampled (current definition, meaning part of a sampled trace). However, we have several known reasons to export a span that was not sampled by that definition:
I believe you are interested in the first of these, whereas #2918 discusses the second of these. Both are examples of why we might want to support a span that is exported but not sampled. |
@jmacd Based on your description, yes, we are looking for feature enhancement similar to your item 1. |
The need makes sense, but I wonder how we could do this in a way that doesn't make things very confusing for users. Feels like the kind of thing that may accidentally lead people astray - I envision questions from people like "why are my spans being exported even though they weren't sampled?" and then the answers being involved discussions around how to make sure you are using the right combination of samplers and exporters (or something) to get the effect they want. Not to say that the proposal isn't interesting or useful, just to highlight that if we aren't careful it could just be rather confusing. |
@ahayworth True it is a bit confusing. I am going to try to explain it this way: Sampling process can be divided into two steps: sampling decision making and sampling execution. Sampling decision must be made in the SDK. The smapling execution, on the other hand, would be perform by SDK (The current behavior) or defer to OTel Agent. Let's call the latter the deferred sampling execution. What we ask here is can we support deferred sampling execution. |
@wdengw That explanation actually does make it a little easier to reason about (and should we accept this proposal, I think that language would be good to use in the spec 😄 ) |
What are you trying to achieve?
SDKs allow all spans to be exported to OTel Agent/Collector with its Sampled flag, so that The OTel agent/collector can receive 100% of the spans created by apps. This opens the opportunity for OTel Agent/Collector's processors to gather information from all spans and it can decided to store the spans to backend store based the spans' sampled flag.
What did you expect to see?
Option in SDK to allow all spans to be exported to OTel Agent/Collector with the sample flag.
Additional context.
See open-telemetry/opentelemetry-java#4990
Add any other context about the problem here. If you followed an existing documentation, please share the link to it.
The text was updated successfully, but these errors were encountered: