Implement InvocationSpanContext for user tracing / streaming tail workers #3028
Conversation
|
Don't merge just yet – will review today |
Yep, figured as much. Want to make sure this is going to be good with the work you're doing |
| // If the traceId or invocationId are invalid, then we'll ignore them. | ||
| if (!sc->getTraceId() || !sc->getInvocationId()) return kj::none; |
There was a problem hiding this comment.
At this point, maybe we should just have a helper method to check validness.
| // If the traceId or invocationId are invalid, then we'll ignore them. | |
| if (!sc->getTraceId() || !sc->getInvocationId()) return kj::none; | |
| // If the traceId or invocationId are invalid, then we'll ignore them. | |
| if (!sc->hasTraceId() || !sc->hasInvocationId()) return kj::none; |
There was a problem hiding this comment.
Are you suggesting renaming these because there is no hasTraceId() or hasInvocationId(). These both return const TraceId& which implements an operator bool(), which means these work as is.
Co-authored-by: Yagiz Nizipli <yagiz@cloudflare.com>
| // monotically increasing number. Every InvocationSpanContext has a root span | ||
| // whose ID is zero. Every child span context created within that context will | ||
| // have a span id that is one greater than the previously created one. | ||
| class SpanIdCounter final: public kj::Refcounted { |
There was a problem hiding this comment.
SpanIdCounter felt overengineered at a first glance – using an increasing unsigned counter value correctly should not be difficult as long as it only needs to be done within the span implementation itself. I take that this is being used for counter memory management so that it is easier to acquire increasing, unique span IDs across several "branches" of the trace?
There was a problem hiding this comment.
Yes, that's exactly it. I didn't put much effort into designing this bit and just Did Something That Works. If you have an alternative suggested approach here I'm happy to change this part. The key requirement is that every descendent span should be assigned the next monotonically increasing value without those having to be aware of each other in any other way.
e.g.
root span (0)
create child span off root (1)
create child span off root (2)
create child span off 1 (3)
create child span off 3 (4)
create child span off 2 (5)
....
|
|
||
| inline kj::uint next() { | ||
| static constexpr kj::uint kMax = kj::maxValue; | ||
| KJ_ASSERT(id < kMax, "max number of spans exceeded"); |
There was a problem hiding this comment.
Not objecting to this assert, but if 2^32 spans are reached we have bigger problems 😄
There was a problem hiding this comment.
Oh, I meant for this to be a KJ_DASSERT but this is just me being way over paranoid lol.
| traceId.toCapnp(writer.initTraceId()); | ||
| invocationId.toCapnp(writer.initInvocationId()); | ||
| writer.setSpanId(spanId); | ||
| kj::mv(getParent()); |
There was a problem hiding this comment.
Where does the parent context move, or is this just used to invalidate it?
This implements the
InvocationSpanContextclass. This type holds a tuple of trace ID, invocation ID, and span ID and will be used in support for the updated streaming tail workers implementation. The basic idea is that every top-level request receives a trace ID which is shared across every invocation the occurs within that request. Every individual invocation has a unique invocation ID, and every span within that invocation receives a monotonically increasing span ID.While this PR does not currently introduce uses of this type, the idea is for all invocations to always have a root
InvocationSpanContextand for every child user span created to branch off this root. The current user span then would always have a currentInvocationSpanContextthat would be propagated outward for all traceable sub requests (generally any subrequest that is not an public internet fetch). A follow up PR will start to introduce the uses and propagation of the span context. This PR only introduces the class impl an the basic test for it. (I'm intentionally breaking this work into smaller incremental PRs to make it easier to review individual chunks).