grpc-swift icon indicating copy to clipboard operation
grpc-swift copied to clipboard

[DO NOT MERGE] Add tracing instrumentation

Open slashmo opened this issue 5 years ago • 7 comments

This is a WIP for tracing gRPC services. It's meant as a playground for experimenting with how such instrumentation will look like.

slashmo avatar Aug 16 '20 17:08 slashmo

CLA Check
The committers are authorized under a signed CLA.

  • :white_check_mark: Moritz Lang (7db7f80058a3e55102397539b86bc1cde512ef1f)

I'm currently finishing up the server span attributes. Per Otel spec, we should either add HTTP attributes to the same Span or create a child Span of the gRPC span for the HTTP transport:

HTTP calls can generally be represented using just HTTP spans. If they address a particular remote service and method known to the caller, i.e., when it is a remote procedure call transported over HTTP, the rpc.* attributes might be added additionally on that span, or in a separate RPC span that is a parent of the transporting HTTP call. Note that method in this context is about the called remote procedure and not the HTTP verb (GET, POST, etc.).

@glbrntt / @ktoso Which option would you prefer here?

https://github.com/open-telemetry/opentelemetry-specification/blob/b70565d5a8a13d26c91fb692879dc874d22c3ac8/specification/trace/semantic_conventions/rpc.md#distinction-from-http-spans

slashmo avatar Aug 20 '20 12:08 slashmo

Try what is simpler for now (so just one span I guess) and let's try doing the client as well so we can show two grpc services talking to one another, and then we continue improving the span quality perhaps?

I can use some tracer I have to see how it works in action once we have an end-to-end :)

ktoso avatar Aug 20 '20 12:08 ktoso

Try what is simpler for now (so just one span I guess) and let's try doing the client as well so we can show two grpc services talking to one another, and then we continue improving the span quality perhaps?

Sounds good 👌

slashmo avatar Aug 20 '20 12:08 slashmo

I added these HTTP attributes on the server span:

  • http.method
  • http.flavor
  • http.target
  • http.status_code
  • http.status_text
  • http.user_agent

I'm not sure where to extract values for the following recommended attributes though:

  • http.host
  • http.server_name
  • net.host.port
  • http.scheme
  • http.route
  • http.client_ip
  • net.peer.ip

@glbrntt Do you have an idea for some of those?

For reference, here's a list of the attributes Otel recommends for HTTP spans: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#http-client-server-example

slashmo avatar Aug 21 '20 11:08 slashmo

I'm not sure where to extract values for the following recommended attributes though:

  • http.host

We should be able to get this from the headers in the routing handler, I think.

  • net.host.port

We can get this from the Channel on the ChannelHandlerContext: context.channel.localAddress?.port

  • http.scheme

Hmm. This is unfortunate, this is in the request headers but it gets stripped by the HTTP2 to HTTP1 codec. We drop down to using HTTP/1 concepts in the pipeline so that supporting gRPC-Web (i.e. gRPC-ish over HTTP/1) is easier. We were thinking of changing this so that we always speak HTTP/2 in the pipeline and then only convert back to HTTP/1 if we absolutely have to for gRPC-Web.

  • http.route

Isn't this just rpc.service/rpc.method?

  • http.client_ip

This suggests that we'd have to pull this from the request headers in the routing handler, most likely.

  • net.peer.ip

I think this is another one we can pull from the ChannelHandlerContext: context.remoteAddress?.ipAddress

glbrntt avatar Aug 21 '20 12:08 glbrntt

Do we really need all the http attributes? A grpc call, wouldn’t the focus be on getting those in: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/rpc.md ?

In any case, as much as we have right now is okey and would be nice to continue to the complete client+server instrumentation. We can always add more attributes later, wdyt?

ktoso avatar Aug 21 '20 12:08 ktoso

We won't adopt tracing out-of-the-box in v1, instead this will come in v2 via interceptors.

glbrntt avatar Jan 15 '24 10:01 glbrntt