Sample on error - change sampling decision during the flow
kfirs opened this issue · 5 comments
I know, everywhere is written that changing the sampling decision is not feasible.
BUT, if I created a span, until it is not finished, the export decision in the SpanProcessor is not yet happened.
So, the question is why I can't change the sampled decision for this span as long it is not ended?
I can see 2 options - first is directly in the SpanContext to be able to set the sampled value. For this, it would require enhancement.
The other option, which is less preferred, is to extend the SpanProcessor implementation and change the onEnd() method to add additional parameter for the decision if to export or not the Span, and by this, to workaround the sampled decision.
for example
public void onEnd(ReadableSpan span) {
boolean forcedSampling = samplingProvider.get()
if (span != null && (span.getSpanContext().isSampled() || forcedSampling ) {
this.worker.addSpan(span);
}
}
BTW, by that I'm getting only the child-parent relationship spans to be forced sampling. For me, this is exactly what is needed.
So, the questions are
- why it was decided not to enable the change of
SpanContext.sampled? - Why not to do what I'm suggesting here and override the sampling decision?
- Can it be considered as an enhancement?
So, the question is why I can't change the sampled decision for this span as long it is not ended?
Because of the distributed nature of Distributed Tracing (there are more than one apps) and because the sampling decision needs to be propagated (either the whole trace is sampled or not). Let's say you have two apps: A and B. A calls B sequentially, it waits for B to respond then continues with its job. In this scenario:
- A makes the sampling decision, let's say it's
false(no sampling) - A calls B and propagates the sampling decision (no sampling)
- B respects this decision, do its job and drops its spans (no sampling)
- A gets the response from B, finishes its job and at this point it decides to change the sampling decision
How exactly would B know about this? How would it know which spans need to be changed? How would it publish spans that were already dropped? It's even worse when B calls another app C and so on.
I hope this answers the question why you can't change the sampling decision on the app side (head sampling). On the other hand you can change the sampling decision once you collected these spans on the tracing store/collector/backend (tail sampling).
why it was decided not to enable the change of SpanContext.sampled?
Why not to do what I'm suggesting here and override the sampling decision?
Can it be considered as an enhancement?
I hope the above answers these questions too, if the sampling decision that happens upfront on the app side (head sampling) is not working for you, you should consider doing tail sampling (outside of your apps).
With this, let me close this issue, please let us know if I misunderstood something and we can reopen as needed.
Thank you for your response. I'll try to address your questions.
In general, as I mentioned, my use case is sample on error. Meaning, once there is an error, I would like to sample the current span and its parents. This will give me good idea what happened in this transaction that led to this error.
I don't need all spans participated in this transaction to be sampled, and I understand that there will be edge cases that I will not be able to achieve this.
In your example, B and C are not related and only A should be sampled as the decision was in A.. But lets say B failed - so B should be sampled and also A. C shouldn't be sampled because its not important. I, as application, need to make sure to pass the information from B to A. I'll have this logic.
I will provide you all the use cases that I can support if you will give me the ability, but in the end, this should be my decision and my logic how I want to sample the spans. I would expect to have this ability to control it. At least as far as I understand, there is no technical restriction to enable it.
Tail sampling is expensive and we want to eliminate of using it.
Here is my use cases that I see that will be supported if you will give me this ability (or if I'll extend the SpanProcessor. It requires application logic as well, but I guess that this is fine).
Prerequisite -
a. current trace is not sampled.
b. Some error occurred
d. Configuration of tracing:debug:sampleOnError is true
1. Current span should be sampled
2. New spans should be sampled on the same ms and same thread
3. New spans should be sampled on the same ms and different thread
4. New spans should be sampled on different ms
5. Opened spans on the current ms on current thread should be sampled (parent)
6. Opened spans on current ms on different thread should be sampled (parent)
7. Opened spans on different ms should be sampled
8. A calls B calls C. C returns error, B returns 200 --> all transaction should be sampled
9. A calls B. A calls C. error occurred in C --> B shouldn't be sampled
(ms = Microservice)
To sum it up:
- I don't think that if I can't have all the data in the transaction, it not good. I can get enough information from failed transaction to have an idea what happened. it shouldn't be all or nothing concept.
- As a framework, I believe you need to give the option to enhance the logic, specially when you don't have technical restriction for it.
Thank you for your time
In general, as I mentioned, my use case is sample on error. Meaning, once there is an error, I would like to sample the current span and its parents.
I think I understand your use-case, what I'm saying is that once there is an error you might not have access to all the parent spans since they might be in other applications. Also having an error somewhere does not mean that the error will propagate all the way up to the root because a component might handle it or there are ~"fire-and-forget" scenarios (kafka, amqp, rsocket, etc.).
I don't need all spans participated in this transaction to be sampled, and I understand that there will be edge cases that I will not be able to achieve this.
I think you described why tracing libraries don't do this. Here, what you call edge case might be the main use-case I think and there are edge-cases where this can work (e.g.: you have one app).
In your example, B and C are not related and only A should be sampled as the decision was in A.. But lets say B failed - so B should be sampled and also A. C shouldn't be sampled because its not important. I, as application, need to make sure to pass the information from B to A. I'll have this logic.
In my example they are related, both B and C are part of the same trace and play vital part of what happened. I'm not sure how you got to the conclusion that C is not important but randomly dropping data is not very fortunate when you are trying to figure out what happened, I don't recommend dropping spans like that since from that point traces are not showing what happened but showing what did not. Also, there are scenarios when you cannot pass this information (see above).
I will provide you all the use cases that I can support if you will give me the ability, but in the end, this should be my decision and my logic how I want to sample the spans. I would expect to have this ability to control it. At least as far as I understand, there is no technical restriction to enable it.
I'm not sure I need use-cases where this can work when there are many use cases when this will not. Please also notice that:
- This is the reason tracing libraries do not have this functionality.
- Micrometer Tracing does not do sampling, that's the job of the tracing libraries so if you want a separate feature for this, you should ask in the OTel and/or Brave issue trackers.
- It's still 100% your decision: you can do head or tail sampling or you can provide your own exporter/reporter where you should be able to decide which spans you want to publish (sampled) or drop (not-sampled) based on the span data right before sending the data, there is no change needed on Micrometer side to do this.
Tail sampling is expensive and we want to eliminate of using it.
I understand that but it is also the only way (at least what I'm aware of) to do this ~properly.
New spans should be sampled on the same ms and same thread
What does "ms" mean here?
A calls B calls C. C returns error, B returns 200 --> all transaction should be sampled
Again, I don't really know how A will know that C failed.
A calls B. A calls C. error occurred in C --> B shouldn't be sampled
I disagree, not sampling B is not showing what happened in the trace.
I don't think that if I can't have all the data in the transaction, it not good. I can get enough information from failed transaction to have an idea what happened. it shouldn't be all or nothing concept.
I disagree, I think most of the users would not trust the data that they are looking at if it would have missing pieces randomly.
As a framework, I believe you need to give the option to enhance the logic, specially when you don't have technical restriction for it.
As I mentioned above, Micrometer Tracing does not do sampling, that's the responsibility of the tracing library (Brave/OTel). The implemented features are user-driven in Micrometer Tracing but that does not mean that every user request will be implemented especially if no other users were asking for it and/or it does not serve a good purpose for other users.
fyi2: I asked about this in the CNCF (OTel) Slack workspace; there is an interest to implement some kind of "local tail sampling" which seems to be what you are asking, I recommend collaborating over this in the OTel issue tracker: open-telemetry/opentelemetry-specification#307