-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Baggage not included w/ OpenTelemetry #164
Comments
My understanding was that the OTEL API was just a wrapper around |
Looks like that's not not the case. There's not much documentation around it but: open-telemetry/opentelemetry-dotnet#1842 and
I think the idea here is that in OTEL API Baggage isn't tied to an Activity (Span) context; it's a separate/standalone thing with its own propagator and whatnot. It's all pretty confusing, because OTEL Baggage is propagated using standard W3C headers (as long you're using the OTEL propagators). And a receiving app using ASP.NET Core's built in diagnostics (without OTEL) will see those headers and set them on Activity.Baggage, where this Serilog Enricher will find them. But the limitation there is that OTEL Baggage isn't copied into Activity.Baggage until that propagation has occurred. So for an app that does Maybe this is an issue for OTEL .NET to address -- maybe |
The key lines in the second link:
It sounds like you have to copy baggage manually? |
I think the idea there is that OTEL telemetry data (Traces, Metrics, Logs) is not automatically enriched with Baggage. If you want it, you have to be explicit. This also underscores how, in the OTEL specification, Baggage is a standalone thing and not tied to Traces/Activities, which is probably why OTEL .NET is trying to move away from Activity.Baggage. Since this project is a Serilog Enricher, that's why I raised the issue here. But I realize Serilog != OTEL Logging, and I don't know what it'd look like to bring those things together. For now, I can certainly add a tiny OtelBaggageEnricher to Serilog to solve this problem. |
This is certainly confusing. I'm not sure this project should do anything about this though. We'd also need to add a reference to the OTEL package if we decide to. Should we close for now? |
Confusing indeed. Especially when OTEL acts as if Baggage is a separate thing from Trace, but recommends using "W3C format for distributed trace" to propagate it. But these things are in various levels of draft/experimental/beta, so probably premature to decide how Serilog (and this Enricher) should participate in that world, and maybe another package someday to avoid the extra dependency. I'll go ahead and close. Hope this issue / bread crumb helps someone else! |
Describe the bug
OpenTelemetry .NET recommends:
I'm new to OpenTelemetry and don't grok why they needed this specialized Baggage API and couldn't use the Activity API. But this Serilog enricher does not find baggage that is set in the OpenTelemetry API.
Not sure if this a use case this Serilog enricher intends to support. Or maybe OpenTelemetry should be copying its Baggage data onto the Activity context (where it can be found by this library), and I should raise the issue over there? It's a little confusing!
Steps to reproduce
Expected behaviour
Serilog Span enricher with IncludeBaggage option will include baggage set by the OpenTelemetry Baggage API.
The text was updated successfully, but these errors were encountered: