Skip to content
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

Add gRPC request metadata instrumentation #7011

Merged
merged 45 commits into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
c6f544b
Added gRPC request instrumentation
Tavh Oct 31, 2022
7830c0f
spotless
Tavh Oct 31, 2022
045a616
applied spotless to grpc-1.6:javaagent
Tavh Oct 31, 2022
33f2232
applied spotless to grpc-1.6:library
Tavh Oct 31, 2022
bc79e30
Changed GRPC_METADATA_ATTRIBUTE_VALUE_PREFIX to RPC_REQUEST_METADATA_…
Tavh Nov 1, 2022
4c90987
grpcRequestMetadata -> rpcRequestMetadata
Tavh Nov 1, 2022
5c302bb
grpcRequestMetadata -> rpcRequestMetadata
Tavh Nov 1, 2022
cdd31b2
removed onEnd from comment
Tavh Nov 2, 2022
cf1649f
changed requestMetadataValuesToCapture to capturedRequestMetadata
Tavh Nov 2, 2022
e0391fd
spotless
Tavh Nov 2, 2022
ae0783b
spotless
Tavh Nov 2, 2022
1b842ee
spotless
Tavh Nov 2, 2022
f8ed4e4
!= null to !empty
Tavh Nov 3, 2022
99e65a5
requestMetadataValuesToCapture -> capturedRequestMetadata
Tavh Nov 3, 2022
bd73355
spotless & made METADATA_KEY protected
Tavh Nov 3, 2022
1b14f73
Initialized capturedRequestMetadata as empty list
Tavh Nov 8, 2022
5112050
Removed . from comment
Tavh Nov 8, 2022
3699771
Added . to comment
Tavh Nov 9, 2022
577d98a
capture metadata config rpc -> grpc
Tavh Nov 9, 2022
0809704
dot
Tavh Nov 9, 2022
92e98ac
Split into client/server config
trask Nov 11, 2022
0135af8
Merge pull request #1 from trask/6991-implement-grpc-request-metadata
Tavh Nov 11, 2022
80a8e69
Update build.gradle.kts
Tavh Nov 11, 2022
ad399ce
fix test
Tavh Nov 11, 2022
095e7e9
fix test
Tavh Nov 11, 2022
50776df
modified grpc test
Tavh Nov 13, 2022
a9122fd
spotless
Tavh Nov 13, 2022
1300621
Fixed test
Tavh Nov 13, 2022
e8ef646
spotless
Tavh Nov 13, 2022
45fa0e7
changed test values
Tavh Nov 13, 2022
ba9cd2d
spotless
Tavh Nov 13, 2022
2801919
comment
Tavh Nov 13, 2022
83d9a08
comment
Tavh Nov 14, 2022
fcbae3c
Merge branch 'main' into 6991-implement-grpc-request-metadata
Tavh Nov 16, 2022
a9d290c
removed null checks
Tavh Nov 16, 2022
5c75ecc
removed nullcheck
Tavh Nov 16, 2022
e2fbb6c
fixed compilation error
Tavh Nov 17, 2022
bb30c18
spotless
Tavh Nov 17, 2022
ffdb462
Moved metadata capture to request start
Tavh Nov 27, 2022
1b207c9
Moved metadata capture to request start
Tavh Nov 27, 2022
0026c0d
spotless
Tavh Nov 27, 2022
1ef28b6
moved attributes capture back to request end
Tavh Dec 11, 2022
23a8daf
revert comment
Tavh Dec 11, 2022
6e42047
renamed prefix
Tavh Jan 13, 2023
bea9394
spotless
Tavh Jan 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions instrumentation/grpc-1.6/javaagent/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,7 @@ tasks {
// The agent context debug mechanism isn't compatible with the bridge approach which may add a
// gRPC context to the root.
jvmArgs("-Dotel.javaagent.experimental.thread-propagation-debugger.enabled=false")
jvmArgs("-Dotel.instrumentation.grpc.capture-metadata.client.request=some-client-key")
jvmArgs("-Dotel.instrumentation.grpc.capture-metadata.server.request=some-server-key")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@

package io.opentelemetry.javaagent.instrumentation.grpc.v1_6;

import static java.util.Collections.emptyList;

import io.grpc.ClientInterceptor;
import io.grpc.Context;
import io.grpc.ServerInterceptor;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.grpc.v1_6.GrpcTelemetry;
import io.opentelemetry.instrumentation.grpc.v1_6.internal.ContextStorageBridge;
import io.opentelemetry.javaagent.bootstrap.internal.InstrumentationConfig;
import java.util.List;

// Holds singleton references.
public final class GrpcSingletons {
Expand All @@ -27,9 +30,18 @@ public final class GrpcSingletons {
InstrumentationConfig.get()
.getBoolean("otel.instrumentation.grpc.experimental-span-attributes", false);

List<String> clientRequestMetadata =
InstrumentationConfig.get()
.getList("otel.instrumentation.grpc.capture-metadata.client.request", emptyList());
List<String> serverRequestMetadata =
InstrumentationConfig.get()
.getList("otel.instrumentation.grpc.capture-metadata.server.request", emptyList());

GrpcTelemetry telemetry =
GrpcTelemetry.builder(GlobalOpenTelemetry.get())
.setCaptureExperimentalSpanAttributes(experimentalSpanAttributes)
.setCapturedClientRequestMetadata(clientRequestMetadata)
.setCapturedServerRequestMetadata(serverRequestMetadata)
.build();

CLIENT_INTERCEPTOR = telemetry.newClientInterceptor();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.grpc.v1_6;

import static java.util.Collections.unmodifiableList;

import io.opentelemetry.api.common.AttributeKey;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;

final class CapturedGrpcMetadataUtil {
private static final String RPC_REQUEST_METADATA_KEY_ATTRIBUTE_PREFIX = "rpc.request.metadata.";
private static final ConcurrentMap<String, AttributeKey<List<String>>> requestKeysCache =
new ConcurrentHashMap<>();

static List<String> lowercase(List<String> names) {
if (names == null) {
return Collections.emptyList();
}

Tavh marked this conversation as resolved.
Show resolved Hide resolved
return unmodifiableList(
names.stream().map(s -> s.toLowerCase(Locale.ROOT)).collect(Collectors.toList()));
}

static AttributeKey<List<String>> requestAttributeKey(String metadataKey) {
return requestKeysCache.computeIfAbsent(
metadataKey, CapturedGrpcMetadataUtil::createRequestKey);
}

private static AttributeKey<List<String>> createRequestKey(String metadataKey) {
return AttributeKey.stringArrayKey(RPC_REQUEST_METADATA_KEY_ATTRIBUTE_PREFIX + metadataKey);
}

private CapturedGrpcMetadataUtil() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,31 @@

package io.opentelemetry.instrumentation.grpc.v1_6;

import static io.opentelemetry.instrumentation.grpc.v1_6.CapturedGrpcMetadataUtil.lowercase;
import static io.opentelemetry.instrumentation.grpc.v1_6.CapturedGrpcMetadataUtil.requestAttributeKey;

import io.grpc.Status;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.List;
import javax.annotation.Nullable;

final class GrpcAttributesExtractor implements AttributesExtractor<GrpcRequest, Status> {
private final GrpcRpcAttributesGetter getter;
private final List<String> capturedRequestMetadata;

GrpcAttributesExtractor(
GrpcRpcAttributesGetter getter, List<String> requestMetadataValuesToCapture) {
this.getter = getter;
this.capturedRequestMetadata = lowercase(requestMetadataValuesToCapture);
}

@Override
public void onStart(
AttributesBuilder attributes, Context parentContext, GrpcRequest grpcRequest) {
// No request attributes
// Request attributes captured on request end.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to capture attributes on start so they can be used for sampling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trask Capture them only at start or at both start and end?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can they be different at those two times?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trask Not sure what you mean by that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I mean, if you capture the request metadata in onStart, is there a reason to capture them in onEnd also?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trask So do you think it should only be on start?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we don't know of any reason to also be on end, then let's start with "only on start"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trask
Status capture has to be on end because it doesn't exist at start.
I moved the metadata capture to start though

}

@Override
Expand All @@ -29,5 +42,14 @@ public void onEnd(
if (status != null) {
attributes.put(SemanticAttributes.RPC_GRPC_STATUS_CODE, status.getCode().value());
}

if (capturedRequestMetadata != null) {
Tavh marked this conversation as resolved.
Show resolved Hide resolved
for (String key : capturedRequestMetadata) {
List<String> value = getter.metadataValue(request, key);
if (!value.isEmpty()) {
attributes.put(requestAttributeKey(key), value);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@

package io.opentelemetry.instrumentation.grpc.v1_6;

import io.grpc.Metadata;
import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesGetter;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import javax.annotation.Nullable;

enum GrpcRpcAttributesGetter implements RpcAttributesGetter<GrpcRequest> {
Expand Down Expand Up @@ -37,4 +42,23 @@ public String method(GrpcRequest request) {
}
return fullMethodName.substring(slashIndex + 1);
}

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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.opentelemetry.instrumentation.grpc.v1_6.internal.GrpcNetServerAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.function.Function;
import java.util.stream.Stream;
Expand Down Expand Up @@ -50,6 +51,8 @@ public final class GrpcTelemetryBuilder {
additionalClientExtractors = new ArrayList<>();

private boolean captureExperimentalSpanAttributes;
private List<String> capturedClientRequestMetadata = Collections.emptyList();
private List<String> capturedServerRequestMetadata = Collections.emptyList();

GrpcTelemetryBuilder(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
Expand Down Expand Up @@ -115,6 +118,22 @@ public GrpcTelemetryBuilder setCaptureExperimentalSpanAttributes(
return this;
}

/** 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;
}

/** Returns a new {@link GrpcTelemetry} with the settings of this {@link GrpcTelemetryBuilder}. */
public GrpcTelemetry build() {
SpanNameExtractor<GrpcRequest> originalSpanNameExtractor = new GrpcSpanNameExtractor();
Expand All @@ -139,7 +158,6 @@ public GrpcTelemetry build() {
instrumenter ->
instrumenter
.setSpanStatusExtractor(new GrpcSpanStatusExtractor())
.addAttributesExtractor(new GrpcAttributesExtractor())
.addAttributesExtractors(additionalExtractors));

GrpcNetClientAttributesGetter netClientAttributesGetter = new GrpcNetClientAttributesGetter();
Expand All @@ -149,11 +167,17 @@ public GrpcTelemetry build() {
.addAttributesExtractor(RpcClientAttributesExtractor.create(rpcAttributesGetter))
.addAttributesExtractor(NetClientAttributesExtractor.create(netClientAttributesGetter))
.addAttributesExtractors(additionalClientExtractors)
.addAttributesExtractor(
new GrpcAttributesExtractor(
GrpcRpcAttributesGetter.INSTANCE, capturedClientRequestMetadata))
.addOperationMetrics(RpcClientMetrics.get());
serverInstrumenterBuilder
.addAttributesExtractor(RpcServerAttributesExtractor.create(rpcAttributesGetter))
.addAttributesExtractor(
NetServerAttributesExtractor.create(new GrpcNetServerAttributesGetter()))
.addAttributesExtractor(
new GrpcAttributesExtractor(
GrpcRpcAttributesGetter.INSTANCE, capturedServerRequestMetadata))
.addOperationMetrics(RpcServerMetrics.get());

if (peerService != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
import java.util.Collections;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
import org.junit.jupiter.api.Test;
Expand All @@ -42,13 +43,21 @@ class GrpcTest extends AbstractGrpcTest {
@Override
protected ServerBuilder<?> configureServer(ServerBuilder<?> server) {
return server.intercept(
GrpcTelemetry.create(testing.getOpenTelemetry()).newServerInterceptor());
GrpcTelemetry.builder(testing.getOpenTelemetry())
.setCapturedServerRequestMetadata(
Collections.singletonList(SERVER_REQUEST_METADATA_KEY))
.build()
.newServerInterceptor());
}

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

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,18 @@
import io.grpc.reflection.v1alpha.ServerReflectionGrpc;
import io.grpc.reflection.v1alpha.ServerReflectionRequest;
import io.grpc.reflection.v1alpha.ServerReflectionResponse;
import io.grpc.stub.MetadataUtils;
import io.grpc.stub.StreamObserver;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.util.ThrowingRunnable;
import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.Collections;
import java.util.List;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.CountDownLatch;
Expand All @@ -66,6 +71,9 @@

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
public abstract class AbstractGrpcTest {
protected static final String CLIENT_REQUEST_METADATA_KEY = "some-client-key";

protected static final String SERVER_REQUEST_METADATA_KEY = "some-server-key";

protected abstract ServerBuilder<?> configureServer(ServerBuilder<?> server);

Expand Down Expand Up @@ -1669,6 +1677,72 @@ public void sayHello(
assertThat(error).hasValue(null);
}

@Test
void setCapturedRequestMetadata() throws Exception {
String metadataAttributePrefix = "rpc.request.metadata.";
AttributeKey<List<String>> clientAttributeKey =
AttributeKey.stringArrayKey(metadataAttributePrefix + CLIENT_REQUEST_METADATA_KEY);
AttributeKey<List<String>> serverAttributeKey =
AttributeKey.stringArrayKey(metadataAttributePrefix + SERVER_REQUEST_METADATA_KEY);
String serverMetadataValue = "server-value";
String clientMetadataValue = "client-value";

BindableService greeter =
new GreeterGrpc.GreeterImplBase() {
@Override
public void sayHello(
Helloworld.Request req, StreamObserver<Helloworld.Response> responseObserver) {
Helloworld.Response reply =
Helloworld.Response.newBuilder().setMessage("Hello " + req.getName()).build();
responseObserver.onNext(reply);
responseObserver.onCompleted();
}
};

Server server = configureServer(ServerBuilder.forPort(0).addService(greeter)).build().start();

ManagedChannel channel = createChannel(server);

Metadata extraMetadata = new Metadata();
extraMetadata.put(
Metadata.Key.of(SERVER_REQUEST_METADATA_KEY, Metadata.ASCII_STRING_MARSHALLER),
serverMetadataValue);
extraMetadata.put(
Metadata.Key.of(CLIENT_REQUEST_METADATA_KEY, Metadata.ASCII_STRING_MARSHALLER),
clientMetadataValue);

GreeterGrpc.GreeterBlockingStub client =
GreeterGrpc.newBlockingStub(channel)
.withInterceptors(MetadataUtils.newAttachHeadersInterceptor(extraMetadata));

Helloworld.Response response =
testing()
.runWithSpan(
"parent",
() -> client.sayHello(Helloworld.Request.newBuilder().setName("test").build()));

OpenTelemetryAssertions.assertThat(response.getMessage()).isEqualTo("Hello test");

testing()
.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(),
span ->
span.hasName("example.Greeter/SayHello")
.hasKind(SpanKind.CLIENT)
.hasParent(trace.getSpan(0))
.hasAttribute(
clientAttributeKey, Collections.singletonList(clientMetadataValue)),
span ->
span.hasName("example.Greeter/SayHello")
.hasKind(SpanKind.SERVER)
.hasParent(trace.getSpan(1))
.hasAttribute(
serverAttributeKey,
Collections.singletonList(serverMetadataValue))));
}

private ManagedChannel createChannel(Server server) throws Exception {
ManagedChannelBuilder<?> channelBuilder =
configureClient(ManagedChannelBuilder.forAddress("localhost", server.getPort()));
Expand Down