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

Respect events' parent in tracing-opentelemetry #2295

Closed
wprzytula opened this issue Sep 2, 2022 · 0 comments · Fixed by #2296
Closed

Respect events' parent in tracing-opentelemetry #2295

wprzytula opened this issue Sep 2, 2022 · 0 comments · Fixed by #2296
Labels
crate/opentelemetry Related to the `tracing-opentelemetry` crate.

Comments

@wprzytula
Copy link
Contributor

Bug Report

Version

tracing-opentelemetry = "0.17.4"

Platform

[Irrelevant]

Crates

tracing-opentelemetry

Description

When creating events in asynchronous code, it is valid to add one to a span that is not current (right after awaiting the instrumented future - see below). However, OpenTelemetryLayer::on_event() uses current span for adding events no matter if the event has its parent explicitly set.

This code:

let outer_span = trace_span!("Outer");
async {
	let inner_span = trace_span!("Inner");
	some_async_function()
		.instrument(inner_span.clone())
		.await
		.map_err(|err| {
			error!(parent: &inner_span, "Error occured!");
	}.instrument(outer_span).await;

results in "Error occured!" being attached to outer span instead of inner span.

Proposed solution

Change OpenTelemetryLayer::on_event() to respect event's parent if set, falling back to current span if not set.

@jtescher jtescher added the crate/opentelemetry Related to the `tracing-opentelemetry` crate. label Sep 2, 2022
hawkw pushed a commit that referenced this issue Sep 7, 2022
…#2296)

## Motivation

One can want to have an event added from outside context of a span,
especially in an async code (e.g. between polls of the future, or
between polling multiple futures instrumented with that span). Then it
is expected that the event will be attached indeed to the span specified
as the parent and not the contextual one.

Fixes: #2295 

## Solution

See #2295
davidbarsky pushed a commit that referenced this issue Sep 8, 2022
…#2296)

One can want to have an event added from outside context of a span,
especially in an async code (e.g. between polls of the future, or
between polling multiple futures instrumented with that span). Then it
is expected that the event will be attached indeed to the span specified
as the parent and not the contextual one.

Fixes: #2295

See #2295
hawkw pushed a commit that referenced this issue Sep 19, 2022
…#2296)

One can want to have an event added from outside context of a span,
especially in an async code (e.g. between polls of the future, or
between polling multiple futures instrumented with that span). Then it
is expected that the event will be attached indeed to the span specified
as the parent and not the contextual one.

Fixes: #2295

See #2295
hawkw pushed a commit that referenced this issue Sep 19, 2022
…#2296)

One can want to have an event added from outside context of a span,
especially in an async code (e.g. between polls of the future, or
between polling multiple futures instrumented with that span). Then it
is expected that the event will be attached indeed to the span specified
as the parent and not the contextual one.

Fixes: #2295

See #2295
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
…tokio-rs#2296)

One can want to have an event added from outside context of a span,
especially in an async code (e.g. between polls of the future, or
between polling multiple futures instrumented with that span). Then it
is expected that the event will be attached indeed to the span specified
as the parent and not the contextual one.

Fixes: tokio-rs#2295

See tokio-rs#2295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/opentelemetry Related to the `tracing-opentelemetry` crate.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants