Add gRPC request metadata instrumentation #7011

Tavh · 2022-10-31T15:29:30Z

Solves. #6991

This PR implements the request portion of the new gRPC metadata instrumentation spec:
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md#grpc-request-and-response-metadata

The changes include:

new CommonConfig entry for desired gRPC metadata values: 'otel.instrumentation.grpc.capture-metadata.request'
(Similar to http headers)
setting the desired metadata values in GrpcTelemetry
new property in GrpcAttributesExtractor that holds a reference to the GrpcRpcAttributesGetter
new property in GrpcAttributesExtractor that stores the desired values so it can iterate them and extract each one from the request
inject the GrpcRpcAttributesGetter to GrpcAttributesExtractor (in GrpcTelemetryBuilder)
logic in GrpcRpcAttributesGetter to safely extract the gRPC metadata value
A new test in GrpcTest that makes sure that when a certain metadata key name is inserted, it also ends up in the span attributes

** Doesn't take care of the response because gRPC response is not implemented in java-instrumentation yet. (This is absolutely necessary but out of scope for this PR)
** "metadataValue" is only implemented inside GrpcRpcAttributesGetter and not in RpcAttributesGetter to avoid providing implementations for every RpcAttributesGetter in the repo as this PR only focuses on gRPC.

linux-foundation-easycla · 2022-10-31T15:29:33Z

The committers listed above are authorized under a signed CLA.

✅ login: Tavh / name: Tav Herzlich (c6f544b)

…ATTRIBUTE_VALUE_PREFIX

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java

...agent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_6/GrpcSingletons.java

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcRpcAttributesGetter.java

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java

...-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/CommonConfig.java

...tion/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTest.java

moved rpcRequestMetadata definition to GrpcSingletons changed null to emptylist in GrpcRpcAttributesGetter Added caching to GrpcAttributesExtractor

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java

mateuszrzeszutek

Left one more minor comment, but overall it looks good to me 👍
Thanks!

...6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java

Tavh · 2022-11-08T16:42:27Z

@mateuszrzeszutek
There seems to be a CI issue, trying to understand if it's even related to my changes, as I made a very small change and it started failing in an unrelated area

mateuszrzeszutek · 2022-11-08T16:51:02Z

@mateuszrzeszutek There seems to be a CI issue, trying to understand if it's even related to my changes, as I made a very small change and it started failing in an unrelated area

No, it's related to our safety net mechanism (muzzle) and the latest reactor 3.5 release (which broke a few things, and removed a couple of methods). I'm currently working on a PR that fixes it.

...brary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcMetadataUtil.java

trask · 2022-11-15T03:01:39Z

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java

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


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

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

can they be different at those two times?

@trask Not sure what you mean by that

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

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

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

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

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java

...6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java

trask · 2022-11-22T23:49:28Z

it's probably worth waiting to merge this for open-telemetry/opentelemetry-specification#2981

trask · 2022-11-23T17:31:34Z

it's probably worth waiting to merge this for open-telemetry/opentelemetry-specification#2981

@Tavh this spec PR has been merged so you can go ahead and update the names to match 👍

trask · 2022-11-28T23:46:55Z

hey @Tavh, I took a look at the latest CI failures, and it looks related to changing from onEnd to onStart.

It looks a bit tricky to capture them onStart, so I'd suggest switching back to onEnd, add we can open an issue to track this, so that it doesn't need to block your PR.

Tavh · 2022-12-11T10:43:32Z

@trask Took me a while to get back to this. I reverted the changes that moved the capture to requestStart

trask · 2022-12-11T21:44:08Z

it's probably worth waiting to merge this for open-telemetry/opentelemetry-specification#2981

@Tavh this spec PR has been merged so you can go ahead and update the names to match 👍

@Tavh one last thing and we can merge this, thx!

trask · 2023-01-12T04:19:24Z

ping @Tavh

Tavh · 2023-01-13T16:42:05Z

@trask changed the naming according to the spec changes

trask

thanks @Tavh 🎉

Added gRPC request instrumentation

c6f544b

Tavh requested a review from a team October 31, 2022 15:29

spotless

7830c0f

This was referenced Oct 31, 2022

Implement gRPC request metadata #6991

Closed

Configurable span attributes based on gRPC Metadata keys #6979

Closed

Tavh added 2 commits October 31, 2022 17:39

applied spotless to grpc-1.6:javaagent

045a616

applied spotless to grpc-1.6:library

33f2232

Tavh changed the title ~~Added gRPC request instrumentation~~ Add gRPC request instrumentation Oct 31, 2022

Tavh changed the title ~~Add gRPC request instrumentation~~ Add gRPC request metadata instrumentation Oct 31, 2022

Tavh and others added 4 commits November 1, 2022 15:58

Changed GRPC_METADATA_ATTRIBUTE_VALUE_PREFIX to RPC_REQUEST_METADATA_…

bc79e30

…ATTRIBUTE_VALUE_PREFIX

grpcRequestMetadata -> rpcRequestMetadata

4c90987

grpcRequestMetadata -> rpcRequestMetadata

5c302bb

removed onEnd from comment

cdd31b2

arbiv reviewed Nov 2, 2022

View reviewed changes

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java Outdated Show resolved Hide resolved

mateuszrzeszutek reviewed Nov 2, 2022

View reviewed changes

Tavh added 4 commits November 2, 2022 21:11

changed requestMetadataValuesToCapture to capturedRequestMetadata

cf1649f

moved rpcRequestMetadata definition to GrpcSingletons changed null to emptylist in GrpcRpcAttributesGetter Added caching to GrpcAttributesExtractor

spotless

e0391fd

spotless

ae0783b

spotless

1b842ee

mateuszrzeszutek reviewed Nov 3, 2022

View reviewed changes

...ibrary/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java Outdated Show resolved Hide resolved

Tavh added 3 commits November 3, 2022 17:31

!= null to !empty

f8ed4e4

requestMetadataValuesToCapture -> capturedRequestMetadata

99e65a5

spotless & made METADATA_KEY protected

bd73355

mateuszrzeszutek linked an issue Nov 8, 2022 that may be closed by this pull request

Implement gRPC request metadata #6991

Closed

mateuszrzeszutek approved these changes Nov 8, 2022

View reviewed changes

...6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java Outdated Show resolved Hide resolved

Tavh added 2 commits November 8, 2022 14:26

Initialized capturedRequestMetadata as empty list

1b14f73

Removed . from comment

5112050

Tavh added 4 commits November 13, 2022 21:15

changed test values

45fa0e7

spotless

ba9cd2d

comment

2801919

comment

83d9a08

trask approved these changes Nov 15, 2022

View reviewed changes

Tavh and others added 3 commits November 16, 2022 23:30

Merge branch 'main' into 6991-implement-grpc-request-metadata

fcbae3c

removed null checks

a9d290c

removed nullcheck

5c75ecc

mateuszrzeszutek reviewed Nov 17, 2022

View reviewed changes

...6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java Outdated Show resolved Hide resolved

Tavh added 2 commits November 17, 2022 11:54

fixed compilation error

e2fbb6c

spotless

bb30c18

Tavh added 3 commits November 27, 2022 16:52

Moved metadata capture to request start

ffdb462

Moved metadata capture to request start

1b207c9

spotless

0026c0d

trask added this to the v1.21.0 milestone Dec 8, 2022

Tavh added 2 commits December 11, 2022 12:40

moved attributes capture back to request end

1ef28b6

revert comment

23a8daf

trask removed this from the v1.21.0 milestone Dec 12, 2022

renamed prefix

6e42047

spotless

bea9394

trask approved these changes Jan 18, 2023

View reviewed changes

trask merged commit b9c10c9 into open-telemetry:main Jan 18, 2023

joshcarp mentioned this pull request Jan 18, 2023

Add metadata attributes connectrpc/otelconnect-go#59

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gRPC request metadata instrumentation #7011

Add gRPC request metadata instrumentation #7011

Tavh commented Oct 31, 2022 •

edited

Loading

linux-foundation-easycla bot commented Oct 31, 2022 •

edited

Loading

mateuszrzeszutek left a comment

Tavh commented Nov 8, 2022 •

edited

Loading

mateuszrzeszutek commented Nov 8, 2022

trask Nov 15, 2022

Tavh Nov 16, 2022

trask Nov 16, 2022

Tavh Nov 17, 2022

trask Nov 17, 2022

Tavh Nov 23, 2022

trask Nov 23, 2022

Tavh Nov 27, 2022

trask commented Nov 22, 2022

trask commented Nov 23, 2022

trask commented Nov 28, 2022

Tavh commented Dec 11, 2022

trask commented Dec 11, 2022

trask commented Jan 12, 2023

Tavh commented Jan 13, 2023

trask left a comment

Add gRPC request metadata instrumentation #7011

Add gRPC request metadata instrumentation #7011

Conversation

Tavh commented Oct 31, 2022 • edited Loading

linux-foundation-easycla bot commented Oct 31, 2022 • edited Loading

mateuszrzeszutek left a comment

Choose a reason for hiding this comment

Tavh commented Nov 8, 2022 • edited Loading

mateuszrzeszutek commented Nov 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trask commented Nov 22, 2022

trask commented Nov 23, 2022

trask commented Nov 28, 2022

Tavh commented Dec 11, 2022

trask commented Dec 11, 2022

trask commented Jan 12, 2023

Tavh commented Jan 13, 2023

trask left a comment

Choose a reason for hiding this comment

Tavh commented Oct 31, 2022 •

edited

Loading

linux-foundation-easycla bot commented Oct 31, 2022 •

edited

Loading

Tavh commented Nov 8, 2022 •

edited

Loading