-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
tracing: Add spanID
field to access logs and http.vars.span_id
placeholder
#6646
Conversation
Signed-off-by: YifanYang6 <yifanyang6@link.cuhk.edu.cn>
Signed-off-by: YifanYang6 <yifanyang6@link.cuhk.edu.cn>
If needed, I can also update the related documentation for these changes in the Caddy website. |
Thanks for the PR! Just so I'm clear on this, the performance is only impacted if metrics are enabled, right? |
That's correct. The performance impact occurs only when tracing is enabled, as both However, it seemed that my previous benchmarks showed some impact even when tracing wasn't enabled. I reran the benchmarks several times, and the new results confirm that the performance is affected only when I've also updated the previous benchmark results. |
spanID
field to access logs and http.vars.span_id
placeholder when tracing is enabledspanID
field to access logs and http.vars.span_id
placeholder
yes if you mean |
Signed-off-by: YifanYang6 <yifanyang6@link.cuhk.edu.cn>
… binary replacement - Removed temporary workaround for `spanid` support, as our patch has been merged (caddyserver/caddy#6646). - Updated to `caddy:2.9` to use the official 2.9 Docker image, which now includes our patch (docker-library/official-images#17848). Signed-off-by: YifanYang6 <yifanyang6@link.cuhk.edu.cn>
Signed-off-by: YifanYang6 <yifanyang6@link.cuhk.edu.cn>
No docs for that... I want to use custom tags, any idea how i can send via caddy custom tags to the tracing? For example i am using this:
|
All this does is add the span ID to the logs automatically, when tracing is enabled. That's it. There's not really anything to document. Tagging is a totally different topic, it's not currently a supported feature. |
In PR #5507, the
traceid
was added to the logs. To further align with the OpenTelemetry Log Correlation specification and achieve complete log correlation using trace context, we have now included thespanid
alongside thetraceid
.Benchmark Comparison
I conducted benchmarks to assess the performance impact of adding the
spanid
. Sincetraceid
andspanid
are always used together, I modified the existing benchmarks fortraceid
to includespanid
. Below is the performance comparison before and after adding thespanid
:Before:
After:
Diff:
The addition of
spanid
to the logs led to a slight increase (~5%) in processing time. Memory usage increased in ~13%, as expected.I also added the
http.vars.span_id
placeholder, similar to what was done in PR #6308 for thehttp.vars.trace_id
placeholder.