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

Way to mark time spent in queue and handler as follows_from request span #144

Open
Restioson opened this issue Jul 18, 2022 · 4 comments
Open
Milestone

Comments

@Restioson
Copy link
Owner

Restioson commented Jul 18, 2022

When split_receiver is used and the receiver is discarded, the xtra_actor_request span will still include the message handling time, which will lead to the containing span being longer than it should actually be in some cases. This is because the work is done somewhere else, and, unlike without split_receiver, is not waited for. Therefore, there needs to be some way to disconnect the handler and waiting time in queue from the xtra_actor_request span.

Consider the following behaviour under master:

#[instrument]
async fn do_something_quickly(addr: Address<A>) {
    let _ = addr.send(LongMessage).split_receiver().await;
    tracing::info!("Done!");
}

"Done!" will print, and then the do_something_quickly span may live long past that, since it will only end when the grandchild xtra_message_handler span for LongMessage finishes too.

Ideally, this is would be the easiest and most natural thing to do in the case of let _ = addr.send(..).split_receiver().await;. However, some subscribers (e.g Jaeger, Zipkin, and Grafana Tempo1) do not support back-referencing spans linked to a given would-be parent (i.e one which the other spans follow from but are not children of). They only show this link on the would-be child span. This can make it hard to navigate traces, so in some instances it can be intended to make these spans children, even though they are semantically not, as a compromise in the favour of explorability.


Footnotes

  1. I'm pretty sure Grafana Tempo just re-exports Jaeger's view, though I don't know for certain.

@thomaseizinger
Copy link
Collaborator

I don't think I understand enough about spans to help here but if somehow possible, I'd be great to have a failing test case that we can work towards fixing.

@Restioson
Copy link
Owner Author

Maybe we should postpone this to 0.7 too?

@thomaseizinger
Copy link
Collaborator

I certainly don't know how to fix it :)

@Restioson Restioson modified the milestones: 0.6.0, 0.7.0 Feb 15, 2023
@Restioson
Copy link
Owner Author

Postponing to 0.7 as it is unclear how this should be handled. I know the principled solution here, but I feel like it is not that useful, as I've mentioned how this can lead to weird behaviour and degraded debuggability in the consuming software (grafana tempo, zipkin, jaeger, etc) after playing around with some different solutions on a real application.

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

2 participants