Some improvements about names of span logs#47667
Conversation
robot-ch-test-poll
added
the
pr-improvement
nickitat
added
the
can be tested
| if (trace_processors) | ||
| { | ||
| span = std::make_unique<OpenTelemetry::SpanHolder>("ExecutionThreadContext::executeTask() " + node->processor->getName()); | ||
| span = std::make_unique<OpenTelemetry::SpanHolder>(node->processor->getName()); |
There was a problem hiding this comment.
Maybe leave some small prefix like "P:" for processors?
There was a problem hiding this comment.
The names in span in most cases are function names. So 'P:' seems a little bit wired for users to understand. I think using the processor name itself is clearly.
If we really want to give a clear information that this span is processor, how about using the function name style by adding prefix 'Processor::' ?
|
I checked log of the failed test, seems that it has nothing to do with the changes in the PR |
|
Stateless tests (debug) [4/5] - |
|
Thank you @nickitat |
4 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Improvement name of some span logs
Description
1. use type of job name as span name
Thread name is used as name of the span that is first created on a thread. If a Job does not set the thread name by calling
setThreadNameinside it, the default name 'ThreadPool::worker' is used.We often see a lot of logs with this default name, and such default name provides nothing to understand the execution flow.
So, there's an improvement on this part that if the thread name is not changed by the job, the type name of the job will be used as span log. This gives a very clear information that what the thread is doing.
2. use name of IProcessors only as span name
Previously,
ExecutionThreadContext::executeTask()is added as prefix of the span name, this makes the log not so intuitive.This prefix is eliminated and only the name of processor itself is used.
3. add a span for PipelineExecutor.execute to give a clear info that how many threads are forked during the execution of pipeline.
These improvements are illustrated on the pic below
Since these names are related to the class name, and they're might change over time, it does not make any sense to write test cases to cover the changes in this PR.