rfc(informational): Achieving order between Pageload and SRR spans #138
Conversation
lforst
force-pushed
the
rfc/achieving-order-between-pageload-and-srr-spans
branch
from
e5dfea6
to
872021f
Compare
There was a problem hiding this comment.
I am +1 on doing the UI approach, it requires the least amount of eng investment and will allow us to validate the idea.
There are some unknowns that I think we should write down:
- Do we know or expect to ever have to query by such a relationship? This would mean that we would need to store this and would require flipping the order in the SDK
- How do other observability providers tackle this? We are probably not the only ones with this issue so lets see if there are other approaches we are not considering
| The hierarchy between Pageload Spans and SSR Spans is currently very confusing. | ||
| SSR Spans are the ancestors of Pageload Spans, even though in a real-world scenario the Pageload is always the first thing that happens. | ||
| This document discusses how this came to be and how it could be improved. |
There was a problem hiding this comment.
This sounds like a tradeoff between correctness and what someone might intuitively expect. My initial thought seeing that screenshot was that pageload would be the parent span, but when you think about it, it makes perfect sense that it is not.
There was a problem hiding this comment.
In my opinion, it is not entirely just a tradeoff between correctness and intuition.
Just by looking at the order of execution, currently, the trace has the wrong topology: First the browser starts requesting the page, then the server responds. The trace shows it the other way around.
SDK-wise the trace is currently correct: The server SDK initializes before the browser SDK initializes.
The thing is, users likely don't care about the SDK, they care about the timings and topology of their application.
There was a problem hiding this comment.
The thing is, users likely don't care about the SDK, they care about the timings and topology of their application.
Agree. Do we know how other providers tackle this problem, do they just ignore it?
| The drawbacks of flipping the order in the SDK is that we would make the data incorrect: | ||
| By optimistically setting the `parent_span_id` of the SSR Span, we may run into situations where we unintentionally orphan that span, for example when somebody requests the page with a non-browser client that will never run the Browser SDK. | ||
| Span orphaning could however be mitigated, if we let the SDK set an additional attribute that informs Sentry about the fact that the `parent_span_id` on the SSR transaction is optimistic (e.g. `sentry.has_optimistic_parent_span_id`). |
There was a problem hiding this comment.
This sounds brittle so I would try and avoid this.
There was a problem hiding this comment.
IMO not super brittle, but yes, I would also like to avoid this.
|
|
||
| Flipping at read time is hard because you would need all the spans of the entire trace to make an informed flipping decision. It is not enough to just look at transaction hierarchy. | ||
|
|
||
| Flipping the order when querying (ie. when loading the trace view or even purely having custom logic in the UI) might be another viable option. |
There was a problem hiding this comment.
I'm +1 on doing this as the first approach and reevaluating if we need to.
|
@lforst It just occurred to me, but I think the only reason this doesnt make sense to me visually is because the start time of a child ends up being before the start time of its parent, breaking the "children are contained inside parent" rule. In that case, instead of reparenting, could we consider an alternative approach where we would shift the start of the SSR span to match the start of the pageload span?
In this case, it would then look like the SSR span started at 0, same as the pageload span below it and visually containing it |
@JonasBa That would make the timings wrong. The pageload span starts when the browser starts requesting the page which is always before the server will start handling the request. Moving the server span to the left is entirely wrong and therefore not an option. |
…r request span (#74675) Implements required changes for getsentry/rfcs#138
This is an RFC to discuss how to flip the order between Pageload and SSR spans to improve user experience.
Rendered RFC
Just some slack convos for posterity: