-
Notifications
You must be signed in to change notification settings - Fork 625
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
Align Sampling API with specs #906
Comments
I can look into this if help is still needed. |
@lzchen, can Aravin take this or did you already start on it? |
@aravinsiva |
To summarize what needs to be done:
Correct? Just want to be sure before I get working |
@aravinsiva |
@aravinsiva are you still interested in taking this on? Just checking that you'll have enough time to work on it. |
Hi @codeboten I have done some work on it. However I have a time sensitive project being worked on right now. So if this is high priority perhaps reassigning it is best. |
open-telemetry/opentelemetry-specification#611 EDIT: Done. |
the The actual Samplers, ability to set samplers - part of SDK. |
Closed and opening individual issues in the future. |
…en-telemetry#906) * chore: fixing documentation for web tracer provider, fixing examples for web, enable manager when registering * chore: fixing span extraction from zone after context updates * chore: bump
Looks like Sampling "API' is actually defined as part of the SDK.
As well, there are some implementation details that are possibly deviating from the spec.
For example, spans set with
is_recording_events
to False are currently not even started/ended. According to the spec, it seems the spans should still be passed onto the processor but not the exporter (ifis_sampled
flag is set).Also, the
is_recording_events
field seems to be always returningTrue
as part of the SDK. We probably need this to be an actual property of the span.The text was updated successfully, but these errors were encountered: