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

Setting status on span does nothing #3515

Open
Meemaw opened this issue Jul 30, 2023 · 3 comments
Open

Setting status on span does nothing #3515

Meemaw opened this issue Jul 30, 2023 · 3 comments
Assignees

Comments

@Meemaw
Copy link
Contributor

Meemaw commented Jul 30, 2023

Describe the bug
Calling the set_status method on SpanRef doesn't do anything. The status that ends up in Datadog is still Unset.

use opentelemetry::{self as otel, global, trace::TraceContextExt};

otel::Context::current().span().set_status(otel::trace::Status::Error {
    description: "my error".into(),
});

Expected behavior
Should be able to set status on span.

@garypen
Copy link
Contributor

garypen commented Jul 31, 2023

@Meemaw Is this really an issue in the opentelemetry crate?

@Meemaw
Copy link
Contributor Author

Meemaw commented Aug 7, 2023

Not sure, might be due to custom visitor?

impl<N> Visit for CustomVisitor<N>

@garypen
Copy link
Contributor

garypen commented Aug 14, 2023

I looked into this earlier today and I think this is not supported because we are using the tracing-opentelemetry crate to bridge between normal tracing spans and our opentelemetry support. (To be clear: You might be able to make this work by using the opentelemetry crate directly, but I couldn't figure out how to do it.)

If you read the tracing-opentelemetry documentation, you'll find some comments there about special fields treatment for OTEL fields and these snippets should help you see what you could do to make this work...

        let current = tracing::span::Span::current();
        if current.is_disabled() {
            println!("current span is disabled");
        } else {
            current.record("otel.status_code", "error");
            current.record("otel.status_message", "my status message");
        }

HOWEVER: This won't work right now for three reasons:

  • You can't set otel.status_message because we don't initialise it to empty in our integration. (that's easy to fix)
  • We explicitly set the otel status based on the result of the request. So, even if you set it manually we overwrite it.
  • The current span, when handling supergraph_service always appears to be the router span.

We could fix the second issue by adding a check to see if this has been set manually before we set it, but I don't think that's a good choice since I think our automated handling of Errors is the best approach for this.

(This is all happening in apollo-router/src/plugins/telemetry/mod.rs)

The third issue is a bit of a puzzle to me right now. If you call current() in the supergraph_service (for instance) the current span is identified as name: "router", where I would expect it to be name: "supergraph". I'm going to need to dig into that a little further. It might be that my understanding of how we name spans is off, so I'll look into that further.

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

No branches or pull requests

2 participants