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

Helidon tracing custom spans #8861

Open
boris-senapt opened this issue Jun 7, 2024 · 4 comments
Open

Helidon tracing custom spans #8861

boris-senapt opened this issue Jun 7, 2024 · 4 comments
Assignees
Projects

Comments

@boris-senapt
Copy link

boris-senapt commented Jun 7, 2024

What is the correct way to create custom spans in Helidon Tracing? I see different places use different approaches to getting the current span, so I end up having to use a utility like the below

private <T> T runInSpan(final Span span, final Callable<T> callable) {
    final var context = Context.builder()
            .update(it -> Contexts.context().ifPresent(it::parent))
            .build();
    // OpenTelemetryDataPropagationProvider uses the span from the context
    context.register(span);
    // WebClientTracing uses the SpanContext from the context
    context.register(span.context());
    // Not sure about this one, but TracingFilter adds it
    context.register(TracingConfig.class, span.context());
    return Contexts.runInContext(context, () -> {
        // enables propagation on the underlying OTEL framework
        try (final var scope = span.activate()) {
            return callable.call();
        } catch (RuntimeException ex) {
            span.end(ex);
            throw ex;
        } finally {
            span.end();
        }
    });
}

If I miss any of the 3 above steps, my custom scope is only propagated "sometimes" and I end up with a mess


Environment Details

  • Helidon Version: 4.0.9
  • Helidon SE
  • JDK version: GraalVM CE 22+36.1 (build 22+36-jvmci-b02)
  • OS: Sonoma 14.5
@github-actions github-actions bot added this to Triage in Backlog Jun 7, 2024
@tjquinno
Copy link
Member

The issue description talks about creating custom spans but the code deals quite a bit with context propagation.

A few thoughts...

  1. Helidon's tracing implementations should automatically propagate the current tracing information when a new Context is created, without your code having to do that. There are some changes in this area in 4.0.10 (releasing soon) which fix problems with propagation of tracing information via Context.
  2. Do you see the same problems if you do not use GraalVM?
  3. In general, and specifically for both of the above questions, it would be great if you could share a simple project that shows in specific coding examples what you expect to work that does not. Your utility avoids for you (and therefore hides) which specific problem or problems might be present in particular coding situations.
  4. I'd like to better understand what "sometimes" and "a mess" mean for you! ;-)

@tjquinno tjquinno self-assigned this Jun 12, 2024
@boris-senapt
Copy link
Author

The issue description talks about creating custom spans but the code deals quite a bit with context propagation.

A few thoughts...

  1. Helidon's tracing implementations should automatically propagate the current tracing information when a new Context is created, without your code having to do that. There are some changes in this area in 4.0.10 (releasing soon) which fix problems with propagation of tracing information via Context.
  2. Do you see the same problems if you do not use GraalVM?
  3. In general, and specifically for both of the above questions, it would be great if you could share a simple project that shows in specific coding examples what you expect to work that does not. Your utility avoids for you (and therefore hides) which specific problem or problems might be present in particular coding situations.
  4. I'd like to better understand what "sometimes" and "a mess" mean for you! ;-)

Yes, you are right - my main concern is the "correct" current span being propagated in various ways; and the propagation for

  • OTEL native stuff like OKHttp instrumentation
  • Helidon WebClient
  • across threads via context

is all different.

I'll find some time to build a demonstrator to show what I'm encountering - hopefully this week.

@boris-senapt
Copy link
Author

I've been working on a reproducer, and you're right @tjquinno that 4.0.10 fixes the main issue I was having in that the context propagation was looking for the current span in the Helidon Context not in the ThreadLocal that OTEL uses.

I did find weirdness though - the code in io.helidon.common.context.ContextAwareExecutorImpl check for an active context before it propagates anything

Optional<Context> context = Contexts.context();
if (context.isPresent()) {

But OpenTelemetryDataPropagationProvider actually makes no use of the context - it propagates the Helidon Tracing active Scope - it doesn't really care about or reference the Helidon Context.

But what that means is that if you don't have a Helidon Context no propagation happens - even though one is not required.


Interestingly Log4jMdcPropagator has the same behaviour.

So I wonder if the problem isn't tracing at all, but the ContextAwareExecutorImpl?

@m0mus m0mus moved this from Triage to Sprint Scope in Backlog Jun 21, 2024
@tjquinno
Copy link
Member

tjquinno commented Jun 25, 2024

TL;DR: As you mentioned, there are several types of "propagation" possibly in play in this discussion, some involving tracing specifically, some using Context and some not, etc. All types of propagation should "just work."

But if--even with the recent tracing improvements--you are still seeing specific problems, please report them here (or in other issues) so we can reproduce and track each one...preferably with a reproducer.


Longer discussion...

You pasted a bit of code from ContextAwareExecutorImpl (which actually appears in both wrap methods in that class). Do you observe a specific problem that you think is caused by that code?

As you point out, that code propagates nothing if there is no current context to propagate into. But I think--perhaps @tomas-langer can confirm--that there is almost always--perhaps always always--a current context, the global context if nothing else:

/**
* Global context is always present and statically shared in this JVM.
* This is similar to Singleton scope when using an injection engine.
* Global context is also used as a parent for newly created contexts, unless you specify a parent using
* {@link Context.Builder#parent(Context)}.
* Registering any instance in this context will make it available to any component in this JVM.
*
* @return global context instance, never null
*/
public static Context globalContext() {
return GLOBAL_CONTEXT.get();
}
So propagation should happen all the time as contexts come and go.

You mentioned that the Helidon OTel propagation provider does not use the context, but it does not really need to. The ContextAwareExecutorImpl#wrap methods do the following:

  • Invoke each propagation provider's data method (which gathers the current values of whatever data that propagator manages) and stores it in a private data structure.
  • Before the user's task executes (likely in a new thread), uses that private data structure to invoke each propagator with the data it previously provided.
  • After the user's task completes, invokes the propagator's clearData method so it can clean up anything necessary.

Any code can register data in a Context but it is the propagation providers that actually capture, set, and restore their own particular states in the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Backlog
  
Sprint Scope
Development

No branches or pull requests

3 participants