search

elixir-grpc / grpc

An Elixir implementation of gRPC
https://hex.pm/packages/grpc
Apache License 2.0
1.36k stars 210 forks source link

Expose Server Address via gRPC Stream #346

Closed matthewberends closed 5 months ago

matthewberends commented 5 months ago

We are currently in the process of updating services to follow opentelemetry semantics and a required attribute is server.address. With the current exposed APIs we could not find a way to access the server address details or the underlying request object to get this information.

matthewberends commented 5 months ago

Did you try to use set_headers to return the necessary metadata through that? I think that's a better path for OTel because then you can also pass an OTel trace that continues through from the client to the server and back.

Hey @polvalente

I looked briefly at using headers to propagate the server details but ultimately was hesitant since it seemed like the info should be available from the request on the server side and so adding it headers may be a bit hacky. One other concern we was if the client side did not use our instrumentation library it would not have the headers injected and races would potentially be missing information (but this situation is not common and probably acceptable).

This being said using headers would definitely work if thats whats recommended as best practice here?

polvalente commented 5 months ago

Did you try to use set_headers to return the necessary metadata through that? I think that's a better path for OTel because then you can also pass an OTel trace that continues through from the client to the server and back.

Hey @polvalente

I looked briefly at using headers to propagate the server details but ultimately was hesitant since it seemed like the info should be available from the request on the server side and so adding it headers may be a bit hacky. One other concern we was if the client side did not use our instrumentation library it would not have the headers injected and races would potentially be missing information (but this situation is not common and probably acceptable).

This being said using headers would definitely work if thats whats recommended as best practice here?

As I see it, if you need the client side to be agnostic to how the server is transmitting the OTel data, then you should be including this as metadata in your proto definitions.

Generally, OTel will transmit trace info using headers anyway, so you'd be looking at those to complete end-to-end traces anyway.

matthewberends commented 5 months ago

As I see it, if you need the client side to be agnostic to how the server is transmitting the OTel data, then you should be including this as metadata in your proto definitions.

Generally, OTel will transmit trace info using headers anyway, so you'd be looking at those to complete end-to-end traces anyway.

Sounds good. Thanks for the input! Closing this PR and opt for header propagation