-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Implement ContextStorageOverride for opentelemetry context bridge #11523
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only concern is the name.
contextstorage/src/main/java/io/grpc/override/OpenTelemetryContextStorage.java
Outdated
Show resolved
Hide resolved
contextstorage/src/main/java/io/grpc/override/OpenTelemetryContextStorage.java
Show resolved
Hide resolved
settings.gradle
Outdated
@@ -76,6 +76,7 @@ include ":grpc-istio-interop-testing" | |||
include ":grpc-inprocess" | |||
include ":grpc-util" | |||
include ":grpc-opentelemetry" | |||
include ":grpc-context-storage-override" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the package is clearly io.grpc.context-related. However, nothing speaks to "otel" in the name. context-opentelementry-storage
(+override?) as the name? (context-storage-override-opentelemetry
?)
@@ -111,6 +112,7 @@ project(':grpc-istio-interop-testing').projectDir = "$rootDir/istio-interop-test | |||
project(':grpc-inprocess').projectDir = "$rootDir/inprocess" as File | |||
project(':grpc-util').projectDir = "$rootDir/util" as File | |||
project(':grpc-opentelemetry').projectDir = "$rootDir/opentelemetry" as File | |||
project(':grpc-opentelemetry-context-storage-override').projectDir = "$rootDir/contextstorage" as File |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API discussion:
grpc-opentelemetry-context-override
[x]grpc-context-override-opentelemetry
grpc-context-opentelemetry-override
b/357892637
follow up with #11500
opencensus shim will find opentelmetry context when using opencensus {@link io.opencensus.trace.unsafe.ContextManagerImpl}, but does not solve the problem of bridging io.grpc.context and io.opentelemetry.context
otel implementation does not have otel over grpc contxt:
https://github.com/open-telemetry/opentelemetry-java/blob/18d192d1fadc6f6dd4dfbff01c0c139eaf93c604/context/src/grpcInOtelTest/java/io/grpc/override/ContextStorageOverride.java
other implementation:
https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/grpc-1.6/library#library-instrumentation-for-grpc-160