Add more Jaeger spans to Worker constructor #114
Conversation
Also, stop the globals_instantiation span before top_level_execution.
harrishancock
requested review from
kentonv,
mikea,
jclee,
a-robinson,
bretthoerner and
byule
as code owners
| @@ -1253,8 +1253,21 @@ Worker::Worker(kj::Own<const Script> scriptParam, | |||
| } | |||
| }); | |||
|
|
|||
| auto maybeMakeSpan = [&](auto operationName) -> MaybeSpan { | |||
| if (auto span = systemTracer.makeSpan(operationName)) { | |||
| span.setTag("truncated_script_id"_kj, truncateScriptId(script->getId())); | |||
There was a problem hiding this comment.
I guess all the spans created this way would have a common truncated_script_id tag... not sure if there is a point at which it becomes more efficient to just have it present once in a parent span. But I suppose there is also a convenience factor in being able to search for spans by the truncated_string_id tag, too.
There was a problem hiding this comment.
This is intentional, precisely for the convenience you mention. In theory, it should be perfectly possible to perform some sort of graph query that expresses, "find me all sub-spans of a span with a particular value for truncated_script_id", or "find me the truncated_script_id value of the ancestor span of a particular span that I have identified", but in practice this is difficult to express, and slow to execute, with the tools we have.
Also, stop the globals_instantiation span before top_level_execution.