In io.opentelemetry.instrumentation.grpc.v1_6 the Setter duplicates instead of replacing

[cleverchuk]

Describe the bug

In this code block the carrier happens to be implemented as a multi-value map. This makes adding the same key with new value a none replacing operation. This behavior will cause the metadata transported across multiple services to keep growing as they update the traceparent or tracestate or any key.

Steps to reproduce

Setup three grpc service that communicate in linear fashion i.e A -> B -> C each instrumented with the agent. Log the carrier to console.

Expected behavior

For example if the agent is updating tracestate with the name of the service then the metadata log line in each service should like
A: [tracestate=name=A]
B: [tracestate=name=B]
C: [tracestate=name=C]

Actual behavior

A: [tracestate=name=A]
B: [tracestate=name=B, tracestate=name=A]
C: [tracestate=name=C, tracestate=name=B, tracestate=name=A]

Javaagent or library instrumentation version

v2.3.0

Environment

JDK: openjdk:17
OS: MacOS 14.4.1

Additional context

No response

[wgy035]

Lifecycle of Metadata is a single RPC call. Maybe the better behavior is removeAll the key before adding new value.

[steverao]

I saw you created relevant PR. So I assigned this to you now. @wgy035

[laurit]

@cleverchuk would it be possible to provide a sample application that reproduces the issue. We need to build a test for this before #11308 can be merged.

[cleverchuk]

@laurit Reproduction needs to create a custom propagator. So when there's a custom propagator that injects let's say a vendor specific key into the Metadata object, the key is propagated further downstream and duplicates when it passes through propagators that inject the same key. The propagator is unable to replace this key with the new data because of how the Metadata object is implemented and there's no api to remove duplicates at the propagator level. It isn't as straightforward as providing an application because a custom propagator needs to be made as well. How about a unit test that shows that when a set is called multiple times with the same key and different/same values during inject then a getAll call on the Metadata for all values for the same key shows that an iterable of size > 1 is returned? So something like below

 class GrpcPropagator implements TextMapPropagator {

    @Override
    public Collection<String> fields() {
      return null;
    }

    @Override
    public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
      setter.set(carrier, "key", "value");
      setter.set(carrier, "key", "value");
    }

    @Override
    public <C> Context extract(Context context, C carrier, TextMapGetter<C> getter) {
      return null;
    }
  }

  @Test
  void checkThatIterableSizeIsGreaterThanOne() {
    GrpcPropagator tested = new GrpcPropagator();
    Metadata metadata = new Metadata();
    tested.inject(Context.current(), metadata, MetadataSetter.INSTANCE);

    int size = 0;
    for (String s : metadata.getAll(
        Metadata.Key.of("key", Metadata.ASCII_STRING_MARSHALLER))) {
      size++;
    }
    assertTrue(size > 1);
  }