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

All samplers are ignored as the default span context is force enabled #298

Closed
oszi opened this issue Sep 6, 2018 · 2 comments · Fixed by #474
Closed

All samplers are ignored as the default span context is force enabled #298

oszi opened this issue Sep 6, 2018 · 2 comments · Fixed by #474

Comments

@oszi
Copy link

oszi commented Sep 6, 2018

Since #159 was merged, sampler decisions are ignored (even AlwaysOffSampler) as the default span context is force enabled. It intended to solve a valid use-case but now every span context is force enabled. Unless you manually send a span context with o=0.

A quick and dirty solution could be changing trace_options.DEFAULT and span_context.DEFAULT_OPTIONS to '0'. Better yet, the default context should set o to int(sampler.should_sample(...)). However, neither solves that o=0 does not disable tracing.

@oszi
Copy link
Author

oszi commented Sep 18, 2018

Furthermore, GoogleCloudFormatPropagator's to_header wrongfully serializes the span context when the span_id is None (it happens) which breaks propagation of tracing options.
A fix would be something like this:

    def to_header(self, span_context: SpanContext) -> str:
        if span_context.span_id:
            trace_span_id = f"{span_context.trace_id}/{span_context.span_id}"
        else:
            trace_span_id = f"{span_context.trace_id}"

        return f"{trace_span_id};o={span_context.trace_options.trace_options_byte}"

I still have no better solution for the original issue than passing the sampler to the propagator and setting the tracing options to self.sampler.should_sample(...) by default.

@falenn
Copy link

falenn commented Nov 29, 2018

Totally agree - the default setting DEFAULT_OPTIONS = 1 keeps custom-supplied samplers (just as other included samplers) from executing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants