Optionally add baggage to span attributes#8023
Conversation
|
Did you consider using a |
|
@laurit good suggestion, I wasn't aware of that |
064765a to
6443b85
Compare
6443b85 to
4afc902
Compare
|
@laurit updated PR as per your suggestion. I've realised that there is probably an issue with my previous PR for logback MDC in that the "baggage." prefix should be on the key not the value |
|
@adamleantech thanks for reporting, I'll fix it |
| @AutoService(AutoConfigurationCustomizerProvider.class) | ||
| public class AgentTracerProviderConfigurer implements AutoConfigurationCustomizerProvider { | ||
| private static final String ADD_THREAD_DETAILS = "otel.javaagent.add-thread-details"; | ||
| private static final String ADD_BAGGAGE = "otel.javaagent.span.add-baggage"; |
There was a problem hiding this comment.
this would correspond with, e.g. otel.traces.exporter, otel.metrics.exporter, otel.logs.exporter
| private static final String ADD_BAGGAGE = "otel.javaagent.span.add-baggage"; | |
| private static final String ADD_BAGGAGE = "otel.javaagent.traces.add-baggage"; |
trask
left a comment
There was a problem hiding this comment.
I wonder if otel.javaagent.span.add-baggage should take a list of baggage to add (or * for all), this would match otel.instrumentation.logback-appender.experimental.capture-mdc-attributes
and maybe we should do the same for |
|
I'm interested in this same functionality but for use as an AutoConfigureSpi addition to a manually instrumented program (which uses the AutoConfigure SDK extension). Is there anything which ties Not for this PR obviously, but I wonder if this and AddThreadDetailsSpanProcessor could be relocated into the https://github.com/open-telemetry/opentelemetry-java repo as some sort of sdk-extension-customizer artifact? |
Yeah, this configurer is coupled to the agent through the
I am liking this idea. @jack-berg and @jkwatson what do you think? Is there room for add-on processors like this in the core repo, or would you prefer to see it as a contrib addition, or something else? |
|
Contrib is the appropriate place for this. If such a processor were to occur in the spec, we could promote that component the core repo. |
|
Has this been contributed to the contrib repo / exists on the java agent? |
not yet |
| span.setAttribute( | ||
| // add prefix to key to not override existing attributes | ||
| "baggage." + key, | ||
| value.getValue())); |
There was a problem hiding this comment.
Given the lack of spec around this topic, it would be good to keep the attribute naming flexible and have the baggage namespace prefix come in through the constructor as an optional parameter. The suggested whitelisting of baggage keys to attach would provide the necessary security to prevent unwanted overrides.
resolves #4520