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

Let SDKs export all the spans regardless of their sampled flag #2986

Open
wdengw opened this issue Nov 26, 2022 · 10 comments
Open

Let SDKs export all the spans regardless of their sampled flag #2986

wdengw opened this issue Nov 26, 2022 · 10 comments
Assignees
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK [label deprecated] triaged-needmoreinfo [label deprecated] The issue is triaged - the OTel community needs more information to decide spec:trace Related to the specification/trace directory

Comments

@wdengw
Copy link

wdengw commented Nov 26, 2022

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.

@wdengw wdengw added the spec:trace Related to the specification/trace directory label Nov 26, 2022
@carlosalberto carlosalberto changed the title Is there a way to let trace exporter to export all the spans to OTel agent/collector regardless of the sampled flag in the spans' are 0/1 and let OTel agent/collector to make the decision based on the spans' sampled flag? Let SDKs export all the spans regardless of their sampled flag Nov 28, 2022
@carlosalberto
Copy link
Contributor

(Re-massaged the title to have a more succinct one)

@carlosalberto carlosalberto added the [label deprecated] triaged-needmoreinfo [label deprecated] The issue is triaged - the OTel community needs more information to decide label Nov 28, 2022
@carlosalberto
Copy link
Contributor

@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 sampled flag as a mere hint).

@carlosalberto
Copy link
Contributor

cc @jmacd

@jmacd
Copy link
Contributor

jmacd commented Nov 28, 2022

@wdengw I've speculated in #2918 (comment) about the need for two new sampling decision return values:

  1. "Unsampled-exported" (which is what you're asking for)
  2. "Conditionally-recorded" (which is an in-between state that allows the SDK's exporter to make the decision between unrecorded and unsampled-exported.

I believe this would address the need you have in mind.

@wdengw
Copy link
Author

wdengw commented Nov 28, 2022

@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".

@arminru arminru added the area:sdk Related to the SDK label Nov 28, 2022
@jmacd
Copy link
Contributor

jmacd commented Nov 28, 2022

@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:

  1. For counting spans in a span-to-metrics pipeline, we may wish to export a span independently of whether its trace context is sampled.
  2. For completing traces that are linked-from by spans in other traces, we may wish to export a span independently of whether its trace context is sampled.

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.

@wdengw
Copy link
Author

wdengw commented Nov 30, 2022

@jmacd Based on your description, yes, we are looking for feature enhancement similar to your item 1.

@ahayworth
Copy link
Contributor

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.

@wdengw
Copy link
Author

wdengw commented Dec 1, 2022

@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.

@ahayworth
Copy link
Contributor

@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 😄 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK [label deprecated] triaged-needmoreinfo [label deprecated] The issue is triaged - the OTel community needs more information to decide spec:trace Related to the specification/trace directory
Projects
None yet
Development

No branches or pull requests

6 participants