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

OpenTelemetry and tagged baggage does not create a tag #375

Closed
mhalbritter opened this issue Oct 2, 2023 · 6 comments
Closed

OpenTelemetry and tagged baggage does not create a tag #375

mhalbritter opened this issue Oct 2, 2023 · 6 comments
Labels
bug A general bug
Milestone

Comments

@mhalbritter
Copy link
Contributor

I have this code:

    @GetMapping("/call1")
    public String call1() {
        ScopedSpan span = this.tracer.startScopedSpan("call1");
        try {
            try (BaggageInScope scope7 = this.tracer.createBaggageInScope("t1", "t1-v1")) {
// ...
            }
        } catch (RuntimeException | Error ex) {
            span.error(ex);
            throw ex;
        } finally {
            span.end();
        }
    }

and t1 configured as tag fields on the OtelBaggageManager.

While it creates the span, the t1 tag is missing.

If I add

this.tracer.getBaggage("t1").makeCurrent("t1-v1").close();

to the try-with-resources, the tag appears.

@marcingrzejszczak marcingrzejszczak added the bug A general bug label Dec 8, 2023
@marcingrzejszczak marcingrzejszczak added this to the 1.0.13 milestone Dec 8, 2023
@marcingrzejszczak
Copy link
Contributor

marcingrzejszczak commented Dec 8, 2023

@mhalbritter so in Micrometer 1.0.x I fixed the bug in OTel and that got propagated forward.

However with Brave it works with 1.0.x if you add the separate tag setting handler, but it won't work with 1.1.x onwards. That's because we've aligned that the Brave's baggage should work like OTel one which means that it has a value only within the scope. The handler however is called after the span is finished so it's way after the baggage scope gets closed. So what I did is I added a constructor for BraveBaggageManager that requires to pass the list of tags to it (it's a non-breaking change cause the default constructor assumes that there are no tag keys) and whenever a baggage is created I will tag the span (instead of doing it at the end of span I do it at the beginning of baggage). That means that Boot will need to align here to change the way the BBM bean is being created.

The only potential problem I see is that someone creates a span but doesn't it put it in scope and wants to set a baggage on it, but baggage as such must live within a scope so I can't imagine someone creating a span not in scope and then a baggage in scope. If someone wants baggage they need to have a span in scope too 🤷

@mhalbritter
Copy link
Contributor Author

Thanks. I've opened spring-projects/spring-boot#38724.

@nikakis
Copy link

nikakis commented Sep 4, 2024

@marcingrzejszczak We have the following issue after upgrading spring boot to 3.1.4 version.

At the code snipet below we invoke the set method for example with x-user-id as key and user-id as value. When we try to fetch the x-user-id from the baggage it returns a null value.

@Override
public void set(String key, @Nullable String value) {
    Baggage baggage = tracer.getBaggage(key);
    TraceContext currentContext = currentTraceContext.context();

    // The default is that baggage set on a child span doesn't
    // bubble up to the parent spans. That means if the child span is ended, the baggage is no longer available.
    // This sets the baggage on every span up to the root. This way it doesn't matter in which child span the
    // baggage is set, it's available on the whole trace.
    while (currentContext != null) {
        baggage.makeCurrent(currentContext, value).close();

        currentContext = traceContextLookup.find(currentContext.traceId(), currentContext.parentId());
    }
}

If we don't invoke the close() method after makeCurrent() everything works properly.

Any idea?

Thanks in advance!

@mhalbritter
Copy link
Contributor Author

FYI: This is a cross-post from here.

@jonatan-ivanov
Copy link
Member

Neither Boot 3.1 nor Micrometer Tracing 1.1 are supported, see: https://spring.io/projects/spring-boot#support
Please try the latest version: Boot 3.3 and Micrometer-Tracing 1.3.

@shakuzen
Copy link
Member

shakuzen commented Sep 6, 2024

Per spring-projects/spring-boot#38724 (comment) I think they tried with Boot 3.3.3 and had the same issue. I think a new issue with a minimal reproducer or code for a unit test would be best to continue the investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants