Add metadata attributes

[joshcarp]

Adds metadata attributes to all types.
Metadata attributes are only defined in the spec for trace span attributes.
The ones defined are:

• rpc.<protocol>.request.metadata.<normalised key>
• rpc.<protocol>.response.metadata.<normalised key>
  Where;
• The <normalised key> are specified via the WithTraceMetadataAttributes(requestKeys, responseKeys []string) Option.
• protocol is one of grpc, grpc_web or buf_connect
• "normalised" is <key> being the normalized gRPC Metadata key (lowercase, with - characters replaced by _), the value being the metadata values. as per the spec. This is implemented here
• The value formatting is in the format rpc.request.metadata.my_custom_metadata_attribute=["1.2.3.4", "1.2.3.5"], which is implemented here

Closes: https://github.com/bufbuild/connect-opentelemetry-go/issues/52

[akshayjshah]

Generally seems like a good idea, especially if the Java & JS otel-grpc integrations do this. A few suggestions:

• Let's make sure we structure the options roughly the same as Java & JS. I'd expect both an allowlist and a denylist, but please do a little research and comment here with how the other implementations do it.
• No matter how we address the previous point, we need space for the allow/deny/etc config to evolve without breaking back compat. Let's separate WithTraceRequestMetadata and WithTraceResponseMetadata and have each take a config struct (so we have space to add more config struct fields over time).
• Please model the allowlists as sets instead of slices, so we have O(1) membership checks.

[akshayjshah]

• No matter how we address the previous point, we need space for the allow/deny/etc config to evolve without breaking back compat. Let's separate WithTraceRequestMetadata and WithTraceResponseMetadata and have each take a config struct (so we have space to add more config struct fields over time).

On further thought, mixing config structs and functional options seems bad. LMK what your research into the OTel-gRPC integrations in Java and JS turns up and we can decide how to proceed.

[joshcarp]

java

[joshcarp]

on grpc java it's done as follows:

  /** Sets which metadata request values should be captured as span attributes on client spans. */
  @CanIgnoreReturnValue
  public GrpcTelemetryBuilder setCapturedClientRequestMetadata(
      List<String> capturedClientRequestMetadata) {
    this.capturedClientRequestMetadata = capturedClientRequestMetadata;
    return this;
  }

  /** Sets which metadata request values should be captured as span attributes on server spans. */
  @CanIgnoreReturnValue
  public GrpcTelemetryBuilder setCapturedServerRequestMetadata(
      List<String> capturedServerRequestMetadata) {
    this.capturedServerRequestMetadata = capturedServerRequestMetadata;
    return this;
  }

and constructing this:

  @Override
  protected ManagedChannelBuilder<?> configureClient(ManagedChannelBuilder<?> client) {
    return client.intercept(
        GrpcTelemetry.builder(testing.getOpenTelemetry())
            .setCapturedClientRequestMetadata(
                Collections.singletonList(CLIENT_REQUEST_METADATA_KEY))
            .build()
            .newClientInterceptor());

getting the metadata:

  List<String> metadataValue(GrpcRequest request, String key) {
    if (request.getMetadata() == null) {
      return Collections.emptyList();
    }

    if (key == null || key.isEmpty()) {
      return Collections.emptyList();
    }

    Iterable<String> values =
        request.getMetadata().getAll(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER));

    if (values == null) {
      return Collections.emptyList();
    }

    return StreamSupport.stream(values.spliterator(), false).collect(Collectors.toList());
  }

calling the metadata function:

    for (String key : capturedRequestMetadata) {
      List<String> value = getter.metadataValue(request, key);
      if (!value.isEmpty()) {
        attributes.put(requestAttributeKey(key), value);
      }
    }

This is basically the same way that this PR implements it:

• slices as keys for allowed request and allowed response metadata
• set them as a field on the interceptor
• in the request if there are any matches between the allowlist then add them + format the keys/values

[joshcarp]

Please model the allowlists as sets instead of slices, so we have O(1) membership checks.

Question about this; there is never a case where a single lookup is done; in each request the keys of metadataReqKeys and metadataResKeys are checked against http.Header, because every key needs to be looked up it's the same as if it were a map.

The only benefit of having a map is that duplicate keys aren't logged, but that could be done in the option, keeping the option parameter a []string, then remove duplicates, then continue with a []string

EDIT: plus it also makes testing awkward because it's non deterministic to iterate through a map

[akshayjshah]

Seems like we ought to move span creation to the first call to Send or the first call to Close, whichever happens first? That'll also make the RPC duration match network activity more closely.

[joshcarp]

Completed through createSpanOnce that gets called in both close and send: 1f41e4c

[akshayjshah]

It doesn't look to me like this is thread-safe. onClose is called from CloseResponse, which must be safe to call concurrently with Send. (This is documented on the StreamingClientConn interface.) Using a sync.Once guarantees that createSpan is only called once, but it does not guarantee that the span has actually been created - it's possible for a call to CloseResponse to be in the midst of creating the span while a concurrent call to Send skips over the Once and attempts to use the nonexistent span.

Let's pair program this afternoon and try to fix this up!