search

census-instrumentation / opencensus-specs

Apache License 2.0
188 stars 50 forks source link

Change sampling decision after starting a span. #218

Open g-easy opened 5 years ago

g-easy commented 5 years ago

Currently, a sampling decision is made at the start of a span and can't be changed afterwards.

Envoy sometimes un-samples a span, e.g. if the span belongs to a health-check, and this wasn't known at start time.

codefromthecrypt commented 5 years ago

in brave we call this "abandon" not sampling. because sampling is trace tier. literally we have a method span.abandon() with a hazard sign make sure you use this with extreme care. Often you don't have to create a span for reasons such as preventing health checks from becoming traced.. that's quite odd actually. span.abandon() was used for something harder to tell, for example if the message was a dupe.

bogdandrutu commented 5 years ago

I think this is similar with "abandon" from brave. So the feature that you want is not to turn on sampling, but to turn off sampling. Is that correct?

g-easy commented 5 years ago

Envoy only uses abandon (true->false) for now, but in #217 we talk about a false->true transition, which might also be useful for a form of retroactive tracing.

This issue is generally about: should we allow this transition and, if so, in which directions?

codefromthecrypt commented 5 years ago

provided instrumentation is written to not inspect noop state, you could possibly transition from false to true, but it is a bit dodgy. In any case, one child could inherit sampled false and another true.

I wouldn't recommend a false to true transition. abandon's opposite would be resurrect, and it wouldn't be possible to resurrect data never recorded.

g-easy commented 5 years ago

Chase pointer: Envoy discussion about this: https://github.com/envoyproxy/envoy/issues/7422

g-easy commented 5 years ago

How important is the "abandon" use-case?

Instead of abandon, could we satisfy this use-case by filtering out spans by name during export?

codefromthecrypt commented 5 years ago

Instrumentation can control if abandon is present. otherwise they would need a buddy to filter it out later.. since export is different than the tracer api maybe from config pov it could be a little messy

On Thu, Aug 8, 2019, 9:53 AM easy notifications@github.com wrote:

How important is the "abandon" use-case?

Instead of abandon, could we satisfy this use-case by filtering out spans by name during export?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/census-instrumentation/opencensus-specs/issues/218?email_source=notifications&email_token=AAAPVV3TMK6LQPZPAB6E2GDQDN4BPA5CNFSM4GJ3BX32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD32F5OY#issuecomment-519331515, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAPVV6LZ2NZTITSKHXCIMTQDN4BPANCNFSM4GJ3BX3Q .