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

Default span propagation for tracing in builders #156

Closed
xd009642 opened this issue Nov 26, 2019 · 7 comments
Closed

Default span propagation for tracing in builders #156

xd009642 opened this issue Nov 26, 2019 · 7 comments

Comments

@xd009642
Copy link
Contributor

Following on from jtescher/opentelemetry-rust#17

It would be useful if the server and client builders provided a default implementing for propagating parent spans via the grpc-trace-bin header. Currently I have something similar to this for a service I'm working on using https://github.com/census-instrumentation/opencensus-go/blob/17d7955af9d42886455ce010dd46878208041a58/plugin/ocgrpc/trace_common.go as a reference for how it's typically done in other languages:

Server::builder()
    .interceptor_fn(|svc, req| {
        let grpc_trace = &req.headers()["grpc-trace-bin"];
        let bp = BinaryPropagator::new();
        let span_state = bp.from_bytes(grpc_trace.as_bytes().into().span_id();
        let parent = match span_state {
            0 => None,
            e @ _ => Some(Id::from_u64(e)),
        };
        let root = match req.uri().path() {
            "/package.service/method" => span!(parent: parent, Level::INFO, "package.service.method"),
            _ => span!(parent: parent, Level::INFO, "package.service.unknown"),
        };
        let _enter = root.enter();
        svc.call(req)
    })
    .add_service(service).serve(addr).await?;

The ugly match is because the span name needs a static lifetime so if there was also a static string representing the package.service.name identifier in the generated grpc code that would be useful as well.

@xd009642
Copy link
Contributor Author

I'm also getting BinaryPropagator from the opentelemetry so not sure about what to do with that...

@jtescher
Copy link

@xd009642 you can also look at the the actix-web impl if that helps https://github.com/OutThereLabs/actix-web-opentelemetry/blob/master/src/middleware/trace.rs#L152

@xd009642
Copy link
Contributor Author

slightly, though I kinda thought it would be possible to do this with just tracing and what tonic already has? I'm currently reading up some docs to try and figure it all out in my mind before asking a bunch of stupid questions 😄

@xd009642
Copy link
Contributor Author

Oh I see the grpc-trace-bin format is defined by different tracing services like opencensus etc. So I suppose I could take a Fn that takes the bytes from the trace header and decodes them to return an option of the span parent id. That would minimise the boilerplate but not solve my span name issue.

And of course whether the span name is actually standardised in any way between the different tracing things...

@LucioFranco
Copy link
Member

This was done in #175

@flokli
Copy link

flokli commented May 23, 2023

Sorry for the necrobump, but I'm trying to wrap my head around #175.

I get that PR adds a .trace_fn(…) to a Server::builder(), but does this actually extract the trace ID and sets the parent span if headers in the request metadata are present?

And similarly, how do I get a tonic client to (always?) inject its current trace ID as a trace ID into the metadata?

Is this something for tonic to provide? Or are there other crates that do this, and I'm just missing the link?

@flokli
Copy link

flokli commented May 23, 2023

Hmmh, #1184 is probably the better place to discuss this. Sorry for the noise!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants