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 OpenTelemetry tracing instrumentation support #110

Closed
wants to merge 3 commits into from

Conversation

corhere
Copy link

@corhere corhere commented Apr 14, 2022

Provide client and server interceptors which instrument RPC requests with OpenTracing telemetry spans, based on the gRPC tracing module go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc@v0.31.0.

Signed-off-by: Cory Snider <csnider@mirantis.com>
Make the remote peer's address available to client and server
interceptors, and to server handlers, by annoting the respective request
contexts with peer.Peer values. The same peer.Peer value is reused for
all requests on a connection to reduce GC pressure. This does run the
risk, however, of the value getting mutated in one request affecting the
value read back in other requests on the same connection.

Signed-off-by: Cory Snider <csnider@mirantis.com>
Provide client and server interceptors which instrument RPC requests
with OpenTracing telemetry spans, based on the gRPC tracing module
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc@v0.31.0

Signed-off-by: Cory Snider <csnider@mirantis.com>
@corhere corhere marked this pull request as ready for review April 18, 2022 17:11
@corhere corhere marked this pull request as draft April 21, 2022 16:27
@corhere
Copy link
Author

corhere commented Apr 21, 2022

The client sets the metadata on the request before calling the interceptor, and the method to set the metadata on the request is unexported, so interceptors cannot alter the request's metadata (other than by directly manipulating the request's Metadata []*KeyValue field and possibly getting it wrong by appending duplicate keys). The otelttrpc client interceptor therefore cannot propagate the trace context using the same tools available to application code. One possible solution would be to move the metadata.setRequest call into (*Client).dispatch so that metadata can be manipulated by the interceptor in the same manner as google.golang.org/grpc, but that would raise some hairy questions about the semantics of the Metadata field on the request struct passed into the interceptor. If the migration away from gogo/protobuf is going to a breaking change, perhaps a breaking change to the UnaryClientInterceptor type is in order to make those questions go away by not exposing the actual to-be-marshaled Request value to the interceptor?

@swagatbora90
Copy link

@corhere are you still working on this PR? I think this is useful to have and we have had a few requests to add ttrpc tracing in containerd.

if p, ok := message.(proto.Message); ok {
span.AddEvent("message", trace.WithAttributes(
attribute.KeyValue(m),
semconv.MessageUncompressedSizeKey.Int(proto.Size(p)),

Choose a reason for hiding this comment

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

This led to performance issue for otel grpc as the otlpreceiver consume lots of cpu for proto.Size. It has been removed since open-telemetry/opentelemetry-go-contrib#3168

@corhere
Copy link
Author

corhere commented Mar 1, 2023

@swagatbora90 no, I am not working on this PR. You are welcome to take it over.

@corhere corhere closed this Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants