baggage propagation

[AgraVator]

No description provided.

[ejona86]

The issue was that it didn't propagate to metrics... but you don't test metrics. I'd expect changes to

grpc-java/opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java

Line 131 in 0567531

private static final class ClientTracer extends ClientStreamTracer {

This can be considered a good step, as it is now propagated to the application. Basically, when you say "propagation" you need to say what you are propagating to. But this would only be one part of what had been discussed earlier. (And this can go in before the rest of the metrics is working.)

[ejona86]

How can this be null?

[AgraVator]

If it was never set...

[ejona86]

How could it be never set?

[AgraVator]

not present in the header...

[ejona86]

https://www.javadoc.io/static/io.opentelemetry/opentelemetry-api/1.55.0/io/opentelemetry/api/baggage/Baggage.html#fromContext(io.opentelemetry.context.Context)

[AgraVator]

hmm, so we are getting it from the grpc context which uses

    /**
     * Get the value from the specified context for this key.
     */
    @SuppressWarnings("unchecked")
    public T get(Context context) {
      T value = (T) PersistentHashArrayMappedTrie.get(context.keyValueEntries, this);
      return value == null ? defaultValue : value;
    }

and can return null

[ejona86]

But it is added unconditionally to the context earlier, so it shouldn't be missing. If it is missing, that seems like a bug, and something we'd want to know about. Seems we should do something, other than ignore it.

[ejona86]

When merging, fix the commit title to start with "opentelemetry:" and include any relevant details in the description. For example, there is an internal bug for this, so that should be included in the commit message.