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

Parent span not set properly #120

Closed
julianocosta89 opened this issue Mar 4, 2024 · 3 comments
Closed

Parent span not set properly #120

julianocosta89 opened this issue Mar 4, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@julianocosta89
Copy link

julianocosta89 commented Mar 4, 2024

In the OpenTelemetry Demo we have a Rust service using reqwest-middleware.
We have noticed that the Context Propagation is not working properly when using reqwest to call another service.
The issue is detailed in here: open-telemetry/opentelemetry-demo#1318

But to add a full context to this issue, here is where we are using it:

async fn request_quote(count: u32) -> Result<f64, Box<dyn std::error::Error>> {
    // TODO: better testing here and default quote_service_addr
    let quote_service_addr: String = format!(
        "{}{}",
        env::var("QUOTE_SERVICE_ADDR").expect("$QUOTE_SERVICE_ADDR is not set"),
        "/getquote"
    );

    let mut reqbody = HashMap::new();
    reqbody.insert("numberOfItems", count);

    let client = ClientBuilder::new(reqwest::Client::new())
        .with(TracingMiddleware::<SpanBackendWithUrl>::new())
        .build();

    let req = client.request(Method::POST, quote_service_addr);

    let mut headers = HeaderMap::new();

    let cx = Context::current();
    global::get_text_map_propagator(|propagator| {
        propagator.inject_context(&cx, &mut HeaderInjector(&mut headers))
    });

    let resp = req
        .json(&reqbody)
        .headers(headers)
        .send()
        .await?
        .text_with_charset("utf-8")
        .await?;

    debug!("{:?}", resp);

    match resp.parse::<f64>() {
        Ok(f) => Ok(f),
        Err(error) => Err(Box::new(error)),
    }
}

When shippingservice(Rust) calls quoteservice(PHP) the parent_id sent is not from the caller, but from the caller's parent.
The trace is currenly looking like this:

image

But it should be something like this:

checkoutservice oteldemo.ShippingService/GetQuote
    |--> shippingservice oteldemo.ShippingService/GetQuote
        |--> shippingservice POST
            |--> quoteservice POST /getquote
                |--> quoteservice {closure}
                    |--> quoteservice calculate-quote
@julianocosta89 julianocosta89 added the bug Something isn't working label Mar 4, 2024
@julianocosta89
Copy link
Author

Another thing that I wonder is why do we need to manually add the context to the headers?

I've removed the following and the trace context was totally gone from the headers.

let mut headers = HeaderMap::new();

    let cx = Context::current();
    global::get_text_map_propagator(|propagator| {
        propagator.inject_context(&cx, &mut HeaderInjector(&mut headers))
    });

Shouldn't the reqwest instrumentation handle that automatically?

@LukeMathWalker
Copy link
Collaborator

You're enabling opentelemetry_0_20 (https://github.com/open-telemetry/opentelemetry-demo/blob/d8ecad7b975e662d0ab9096d7952f7e14497787c/src/shippingservice/Cargo.toml#L30) while using opentelemetry = 0.21 in the same manifest (https://github.com/open-telemetry/opentelemetry-demo/blob/d8ecad7b975e662d0ab9096d7952f7e14497787c/src/shippingservice/Cargo.toml#L22).

If you switch to the correct feature everything should work as expected.
You shouldn't be setting the context yourself, the middleware does it for you (if configured correctly).

@julianocosta89
Copy link
Author

Thank you @LukeMathWalker!

I've just submitted a PR fixing it: open-telemetry/opentelemetry-demo#1433

Appreciate the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants