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

Adds support for ObservationAwareSpanThreadLocalAccessor #211

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

marcingrzejszczak
Copy link
Contributor

without this change you can't propagate just micrometer tracing spans with context propagation

with this change we're introducing an ObservationAwareSpanThreadLocalAccessor that is a ThreadLocalAccessor that knows about the existence of the ObservationThreadLocalAccessor. If OTLA started a span, the OASTLA will back off. If there was no observation and no span there, OASTLA will return an instance of a span. Since ordering matters, OAWSTLA will not be registered in the SPI mechanism, the user will need to set the ContextPropagation registry manually to ensure that OASTLA is registered AFTER OTLA.

additionally I've enhanced the SimpleTracer span and trace id generation so that the tests for OASTLA could have been easily written.

fixes gh-206

without this change you can't propagate just micrometer tracing spans with context propagation

with this change we're introducing an ObservationAwareSpanThreadLocalAccessor that is a ThreadLocalAccessor that knows about the existence of the ObservationThreadLocalAccessor. If OTLA started a span, the OASTLA will back off. If there was no observation and no span there, OASTLA will return an instance of a span. Since ordering matters, OAWSTLA will not be registered in the SPI mechanism, the user will need to set the ContextPropagation registry manually to ensure that OASTLA is registered AFTER OTLA.

fixes gh-206
// User created child spans manually and scoped them
// the current span is not the same as the one from observation
return currentSpan;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 use-case: a Span is already created via the Tracing API and somebody creates a nested Observation (and a Span through it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already checked in a test that it works fine

@marcingrzejszczak
Copy link
Contributor Author

@wilkinsona after we merge this, in Boot 3.0.x and 3.1.x we would have to make such a call (or a similar one)

@Configuration(proxyBeanMethods = false)
class SpanThreadLocalAccessorConfig {

        @Autowired
        Tracer tracer;

        @PostConstruct
        void setupThreadLocalAccessor() {
             ContextRegistry.getInstance().registerThreadLocalAccessor(new ObservationAwareSpanThreadLocalAccessor(tracer));
        }
}

@wilkinsona
Copy link

How will that work in tests where there may be multiple contexts each with their own Tracer? Won't the static-based configuration result in clashes? We could see a similar problem at runtime in an application with multiple contexts.

@marcingrzejszczak
Copy link
Contributor Author

How will that work in tests where there may be multiple contexts each with their own Tracer? Won't the static-based configuration result in clashes? We could see a similar problem at runtime in an application with multiple contexts.

Yeah, that will be a problem. But I haven't heard anyone using different tracers in different contexts until now. We would have this problem now already cause AFAIR everywhere where we're using the Context Propagation API we are referencing the global instance.

The Context Propagation API has the ability to pass a concrete ContextRegistry so if the user needs to differentiate between tracers and contexts and they use the Context Propagation API then they can pass a concrete instance of that registry.

@marcingrzejszczak marcingrzejszczak merged commit 504266a into 1.0.x Mar 22, 2023
@jonatan-ivanov jonatan-ivanov deleted the spanthreadlocalaccessor branch March 22, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants