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

Audit and update google/golang/org/grpc instrumentation to comply with RPC semantic conventions #172

Open
MrAlias opened this issue May 16, 2023 · 6 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@MrAlias MrAlias added the help wanted Extra attention is needed label May 16, 2023
@damemi damemi self-assigned this May 21, 2024
@damemi
Copy link
Contributor

damemi commented Jul 18, 2024

Latest RPC conventions: https://github.com/open-telemetry/semantic-conventions/blob/02ecf0c71e9fa74d09d81c48e04a132db2b7060b/docs/rpc/rpc-spans.md and specifically the gRPC conventions (which extend and override RPC): https://github.com/open-telemetry/semantic-conventions/blob/02ecf0c71e9fa74d09d81c48e04a132db2b7060b/docs/rpc/grpc.md

Common remote procedure call conventions

For outgoing requests, the SpanKind MUST be set to CLIENT and for incoming requests to SERVER.

Correct - client
Correct - server

Remote procedure calls can only be represented with these semantic conventions, when the names of the called service and method are known and available.

Span name

The span name MUST be the full RPC method name formatted as:

$package.$service/$method

(where $service MUST NOT contain dots and $method MUST NOT contain slashes)

We don’t seem to currently fit this in our example, see the gRPC test spans, which for example set name to /helloworld.Greeter/SayHello which has a leading /. However, this is coming directly from the upstream example code which sets the full method name to that exact string.

We are grabbing this value directly from the grpc.Invoke() arguments (client, server) so it’s possible that this is just one case where /helloworld is $service as set by the go gRPC library’s generated code.

If there is no package name or if it is unknown, the $package. part (including the period) is omitted.

Service name

On the server process receiving and handling the remote procedure call, the service name provided in rpc.service does not necessarily have to match the [service.name][] resource attribute.

One process can expose multiple RPC endpoints and thus have multiple RPC service names. From a deployment perspective, as expressed by the service.* resource attributes, it will be treated as one deployed service with one service.name.

Likewise, on clients sending RPC requests to a server, the service name provided in rpc.service does not have to match the [peer.service][] span attribute.

As an example, given a process deployed as QuoteService, this would be the name that goes into the service.name resource attribute which applies to the entire process.
This process could expose two RPC endpoints, one called CurrencyQuotes (= rpc.service) with a method called getMeanRate (= rpc.method) and the other endpoint called StockQuotes (= rpc.service) with two methods getCurrentBid and getLastClose (= rpc.method).
In this example, spans representing client request should have their peer.service attribute set to QuoteService as well to match the server's service.name resource attribute.
Generally, a user SHOULD NOT set peer.service to a fully qualified RPC service name.

Client attributes

The following attributes are required:

  • rpc.system (set)
  • server.address (set)
  • server.port (conditionally, “if the port is supported by the network transport used for communication”) (not set)

rpc.system has the following list of well-known values. If one of them applies, then the respective value MUST be used; otherwise, a custom value MAY be used.

Only one that applies to us is grpc, which we set in both server and client

Server attributes

The following are required:

  • rpc.system (set)
  • server.address (not set)
  • server.port (conditionally, “if the port is supported by the network transport used for communication”) (not set)

gRPC specific conventions

See https://github.com/open-telemetry/semantic-conventions/blob/02ecf0c71e9fa74d09d81c48e04a132db2b7060b/docs/rpc/grpc.md

The following attributes are required on both server and client:

  • rpc.grpc.status_code (not set)

Updates needed

  • Confirm service.name is inherited from the Go gRPC library, which in theory should be compliant with the standard by default
  • Set server.port in client
  • Set server.address in server
  • Set server.port in server
  • Set rpc.grpc.status_code in client and server

@damemi
Copy link
Contributor

damemi commented Aug 8, 2024

For rcp.grpc.status_code, I think if we update our return probe instrumentation for Invoke to read the returned error object, that should get us what we need. gRPC seems to return these as a Status struct with another embedded `*spb.Status struct inside it. From there, the code is just an int which we can map to an actual code in Go. So we should just be able to generate the appropriate offsets and read the int value from those.

@RonFed do we have any way to read the return values for an instrumented function?

@RonFed
Copy link
Contributor

RonFed commented Aug 9, 2024

@damemi The returned error is an interface. Assuming we know the concrete type of the error returned, we can add that type and its relevant fields to offsetgen and read them in the return probe. If the concrete type is not always the same it will be more challenging since we'll have to understand what struct is being returned in the error.

@damemi
Copy link
Contributor

damemi commented Aug 9, 2024

@RonFed yeah, that's what I'm saying is that gRPC should always return the error's concrete type as a Status struct. Because I think it's expected that you can use the gRPC status package to parse this response code, I'm pretty sure that's a safe assumption.

Do we have any examples of reading return values? I know we read the method arguments

@RonFed
Copy link
Contributor

RonFed commented Aug 9, 2024

@damemi Yes,

Here we are reading the returned pointer from a register:

// Get the ** returned ** Span (concrete type of the interfaces)
void *span_ptr_val = get_argument(ctx, 4);

Here we are reading the returned value of the stack:

// The message returned on the stack since it returned as a struct and not a pointer
void *message = (void *)(PT_REGS_SP(ctx) + 8);

In the error case it will be returned as a pointer in a register, we can read the pointer and then the fields.

@damemi
Copy link
Contributor

damemi commented Aug 9, 2024

Awesome, thanks! I'll try to follow that and implement it for grpc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
Development

No branches or pull requests

3 participants