Skip to content

Don't mark outbound http spans as errors. #3158

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

rylev
Copy link
Collaborator

@rylev rylev commented Jun 13, 2025

We log errors in all the cases that are legit (non-user) related errors and so there's no need to also mark the span as errored. This prevents user related issues (e.g., timeouts) from not being bubbled up to telemetry as errors.

Additionally, we stop renaming the span after the http method which isn't very helpful.

@rylev rylev requested a review from lann June 13, 2025 09:54
We log errors in all the cases that are legit (non-user) related errors
and so there's no need to also mark the span as errored. This prevents
user related issues (e.g., timeouts) from not being bubbled up to
telemetry as errors.

Additionally, we stop renaming the span after the http method which
isn't very helpful.

Signed-off-by: Ryan Levick <[email protected]>

use crate::intercept::InterceptOutcome;

impl spin_http::Host for crate::InstanceState {
#[instrument(name = "spin_outbound_http.send_request", skip_all, err(level = Level::INFO),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can never quite remember how all of these things translate into various outputs but I think the name override here helps to distinguish this from the wasi send_request.

...except that the wasi send_request has the same name as this one (oops! 😅)

name = "spin_outbound_http.send_request",

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I removed this is because it seems to be overwritten completely with the write to otel.name (at least when viewing in HoneyComb - I haven't checked in DataDog).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is likely completely replaced for any otel exporter but would still show up in the tracing_subscriber::fmt output(s) and be used by RUST_LOG filters. 🤷

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 this pull request may close these issues.

2 participants