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

[OTel]: Spans produced by spin_sdk::key_value::Store::set don't have a parent assigned #2525

Open
ThorstenHans opened this issue May 22, 2024 · 11 comments

Comments

@ThorstenHans
Copy link
Contributor

ThorstenHans commented May 22, 2024

Spans produced as part of using the key-value store set function, don't have a parent assigned. This results in having two independent traces from a single HTTP API invocation

image

I've tested other actions in the context of key-value store (such as exists and `get) which are all represented correctly:

image

Versions

  • spin CLI: spin 2.5.1 (cba6773 2024-05-14)
  • spin-sdk crate: 3.0.1

Repro

See the following pseudo code for reproducing the behavior:

use spin_sdk::http::{IntoResponse, Request, Response};
use spin_sdk::http_component;
use spin_sdk::key_value::Store;

/// A simple Spin HTTP component.
#[http_component]
fn handle_api(req: Request) -> anyhow::Result<impl IntoResponse> {
    println!("Handling request to {:?}", req.header("spin-full-url"));
    let store = Store::open_default()?;
    let exists = store.exists("foo")?;
    if !exists {
        return Ok(Response::new(404, ()));
    }

    let Some(value) = store.get("foo")? else {
        return Ok(Response::new(204, ()));
    };

    store.set("bar", b"baz")?;

    Ok(Response::builder()
        .status(200)
        .header("content-type", "text/plain")
        .body(value)
        .build())
}
@lann
Copy link
Collaborator

lann commented May 24, 2024

Quick update on behalf of @calebschoepp: this seems to be a bug in tracing-opentelemetry. I believe he is still working on it.

@calebschoepp
Copy link
Contributor

@ThorstenHans please try setting OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=http://localhost:4318/v1/traces instead of OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318. This disables metrics which is the issue I believe.

@ThorstenHans
Copy link
Contributor Author

@calebschoepp I stumbled upon this as part of orchestrating a distributed application with .NET Aspire. Aspire specifies the OTEL_EXPORTER_OTLP_ENDPOINT automatically.

I think I could modify the overall Otel configuration manually, but it would be beneficial to get this fixes at some point without having users to modify standard behavior of Otel.

@ThorstenHans
Copy link
Contributor Author

@calebschoepp you can reproduce the behavior by running the sample at https://github.com/fermyon/Aspire.Spin

cd example/host

export ASPIRE_ALLOW_UNSECURED_TRANSPORT=true
dotnet run -lp http

curl http://localhost:3002

dotnet run will print the dashboard url, open that one and find the traces there

@calebschoepp
Copy link
Contributor

Thanks @ThorstenHans. Turns out we don't even need aspire to repro this. The following app repros the issue:

use spin_sdk::http::{IntoResponse, Request, Response};
use spin_sdk::http_component;
use spin_sdk::key_value::Store;

#[http_component]
fn handle_api(req: Request) -> anyhow::Result<impl IntoResponse> {
    let store = Store::open_default()?;

    store.get("foo")?;

    store.set("bar", b"baz")?;

    Ok(Response::builder()
        .status(200)
        .header("content-type", "text/plain")
        .build())
}

When OTEL_EXPORTER_OTLP_ENDPOINT is set it has the issue. When OTEL_EXPORTER_OTLP_TRACES_ENDPOINT is set it does not have the error. This is still the only workaround I know of.

Spent all of yesterday trying to hunt this bug down and still didn't have any luck.

@ThorstenHans
Copy link
Contributor Author

I remember times when I had similar problems with distributed tracing in dapr sdk for go... I'll check if I can find the corresponding issue... maybe we get some information or hint from that one

@ThorstenHans
Copy link
Contributor Author

Maybe it's worth having a look @calebschoepp dapr/go-sdk#355

@calebschoepp
Copy link
Contributor

Small update: This is not an issue on the old Go and JS SDKs. No clue why it isn't an issue there, but maybe this is a hint that can help us.

@lann
Copy link
Collaborator

lann commented Jun 18, 2024

This is not an issue on the old Go and JS SDKs.

How old?

@calebschoepp
Copy link
Contributor

This is not an issue on the old Go and JS SDKs.

How old?

@karthik2804 can you provide the versions of the SDKs we were testing with please?

@karthik2804
Copy link
Contributor

The go sdk - github.com/fermyon/spin-go-sdk v0.0.0-20240220234050-48ddef7a2617
The JS SDK - npm package spin-sdk 0.6.0 and plugin js2wasm 0.6.1

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

No branches or pull requests

4 participants