Trace context propagation docs and enable distributed traces by default #1557

grcevski · 2025-01-21T22:18:52Z

Add documentation for the new context propagation support and enable it by default.

codecov · 2025-01-21T22:26:35Z

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 71.13%. Comparing base (1dad6ff) to head (c6ed7e6).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/internal/ebpf/tctracer/tctracer.go	20.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1557      +/-   ##
==========================================
- Coverage   71.17%   71.13%   -0.04%     
==========================================
  Files         197      197              
  Lines       19668    19671       +3     
==========================================
- Hits        13998    13993       -5     
- Misses       4993     4997       +4     
- Partials      677      681       +4

Flag	Coverage Δ
integration-test	`53.32% <42.85%> (+0.62%)`	⬆️
k8s-integration-test	`53.45% <42.85%> (-0.09%)`	⬇️
oats-test	`34.43% <28.57%> (+1.71%)`	⬆️
unittests	`47.30% <0.00%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rafaelroquetto

Looks good to me. Will leave it for the @grafsean to approve

rafaelroquetto · 2025-01-21T23:21:08Z

docs/sources/distributed-traces.md


-Beyla will read any incoming trace context header values, track the Go program execution flow and propagate the trace context by automatically adding the `traceparent` field in outgoing HTTP/gRPC requests. If an application already adds the `taceparent` field in outgoing requests, Beyla will use that value for tracing instead its own generated trace context. If Beyla cannot find an incoming `traceparent` context value, it will generate one according to the W3C specification.
+Beyla will read any incoming trace context header values, track the program execution flow and propagate the trace context by automatically adding the `traceparent` field in outgoing HTTP/gRPC requests. If an application already adds the `taceparent` field in outgoing requests, Beyla will use that value for tracing instead its own generated trace context. If Beyla cannot find an incoming `traceparent` context value, it will generate one according to the W3C specification.


typo taceparent

rafaelroquetto · 2025-01-21T23:23:57Z

docs/sources/distributed-traces.md

+
+For TLS encrypted traffic (HTTPS), Beyla is unable to inject the trace information in the outgoing HTTP headers and instead it injects the information
+at TCP/IP packet level. Because of this limitation, Beyla is only able to send the trace information to other Beyla instrumented services. L7 proxies
+and load balancers will also disrupt the TCP/IP context propagation, becuase the original packets are discarded and replayed downstream.


typo: becuase

grcevski · 2025-01-22T00:32:48Z

Looks good to me. Will leave it for the @grafsean to approve

I made some code changes to fix things, do you mind looking over the code changes? I found a bug that our TC code was overriding a manually set TP in an outgoing request. NodeClient tests for this.

marctc · 2025-01-22T09:39:25Z

docs/sources/configure/metrics-traces-attributes.md

+| ---------------------------- | -------------------------------------- | ------- | ------- |
+| `enable_context_propagation` | `BEYLA_BPF_ENABLE_CONTEXT_PROPAGATION` | boolean | (true)  |
+
+Enables injecting of the `Traceparent` header value for outgoing HTTP requests, allowing


Do we want to link somewhere here to docs/sources/distributed-traces.md?

Good idea, done!

docs/sources/distributed-traces.md

marctc

Aren't we forgetting to mention limitations with Java? or we already overcame those?

grafsean

Thanks for the docs. I'll copy edit when I do a pass over these files.

grcevski · 2025-01-22T14:50:24Z

Aren't we forgetting to mention limitations with Java? or we already overcame those?

I think we need to separately document those in limitations.md, since they are not related to the ability to write outgoing header values, but more related to internal thread handoff.

rafaelroquetto · 2025-01-22T15:48:30Z

bpf/protocol_http.h

@@ -134,6 +137,11 @@ static __always_inline void http_get_or_create_trace_info(http_connection_metada
    if (meta) {
        u32 type = trace_type_from_meta(meta);
        set_trace_info_for_connection(conn, type, tp_p);
+        // If the user code setup traceparent manually, don't interfere and add


(Non-rhetorical question) why does this matter? For (outbound) client calls, wouldn't we want to update the outgoing_trace_map (done in server_or_client_trace()) so that the tc egress program can refer back to it and write the traceparent?

So I think we shouldn't mess with the user application code. For example, imagine a user has done this in their code:

const options = { headers: { "traceparent": ... } } http.get(url, options, (res) => { ... do something });

They expect that their downstream service will get this traceparent, I don't know why would they do this, but they can. If we don't check for this condition and setup the outgoing trace map, then Beyla will overwrite the span_id of the outgoing request. The trace_id component will be respected, but the span_id will get generated and updated. This span_id manipulation happens because we use the TCP seq/ack for it.

At the same time, this is the user sending the traceparent on their own, all the TC code would do in this case is just duplicate the header, which will have no effect.

rafaelroquetto · 2025-01-22T15:51:28Z

pkg/internal/ebpf/tctracer/tctracer.go

@@ -138,11 +138,16 @@ func (p *Tracer) startTC(ctx context.Context) {
 		return
 	}

+	if p.cfg.EBPF.UseTCForL7CP {


I wonder if instead of trying to enforce this at the level where the tracers are created, we should enforce this at the config validation level, not letting it propagate....

I guess it's true, we'll need to change the tests to reflect that, because we now have to explicitly disable ContextPropagation if TCL7 is used. Do you think it's worth the effort since we don't actually support this option?

marctc · 2025-01-22T16:07:42Z

Aren't we forgetting to mention limitations with Java? or we already overcame those?

I think we need to separately document those in limitations.md, since they are not related to the ability to write outgoing header values, but more related to internal thread handoff.

ah sorry, I thought this is the same document

Docs and enable distributed traces by default

c188eaa

grcevski requested a review from a team as a code owner January 21, 2025 22:18

grcevski mentioned this pull request Jan 21, 2025

[Docs] Tracing: What works and what doesn't #1407

Closed

rafaelroquetto reviewed Jan 21, 2025

View reviewed changes

grcevski added 3 commits January 21, 2025 19:04

add checks for overlap with TCL7

adbce8a

Spelling and language usage

78ea9f1

fix issue with overriding user code tp

2deb0fd

grcevski added 2 commits January 21, 2025 19:35

more docs fixes

eebe433

more doc fixes

ef25e40

mariomac approved these changes Jan 22, 2025

View reviewed changes

marctc reviewed Jan 22, 2025

View reviewed changes

docs/sources/distributed-traces.md Outdated Show resolved Hide resolved

marctc reviewed Jan 22, 2025

View reviewed changes

docs/sources/distributed-traces.md Show resolved Hide resolved

marctc reviewed Jan 22, 2025

View reviewed changes

marctc requested a review from grafsean January 22, 2025 09:59

grafsean approved these changes Jan 22, 2025

View reviewed changes

grafsean mentioned this pull request Jan 22, 2025

Docs: copy edit for PR 1554 #1558

Merged

update based on PR feedback

c6ed7e6

rafaelroquetto reviewed Jan 22, 2025

View reviewed changes

marctc approved these changes Jan 22, 2025

View reviewed changes

grcevski merged commit fa8b83a into grafana:main Jan 22, 2025
17 checks passed

grcevski deleted the update_tracing_docs branch January 22, 2025 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trace context propagation docs and enable distributed traces by default #1557

Trace context propagation docs and enable distributed traces by default #1557

grcevski commented Jan 21, 2025 •

edited

Loading

codecov bot commented Jan 21, 2025 •

edited

Loading

rafaelroquetto left a comment

rafaelroquetto Jan 21, 2025

rafaelroquetto Jan 21, 2025

grcevski commented Jan 22, 2025

marctc Jan 22, 2025

grcevski Jan 22, 2025

marctc left a comment

grafsean left a comment

grcevski commented Jan 22, 2025

rafaelroquetto Jan 22, 2025

grcevski Jan 22, 2025

grcevski Jan 22, 2025

rafaelroquetto Jan 22, 2025

grcevski Jan 22, 2025

marctc commented Jan 22, 2025


		Beyla will read any incoming trace context header values, track the Go program execution flow and propagate the trace context by automatically adding the `traceparent` field in outgoing HTTP/gRPC requests. If an application already adds the `taceparent` field in outgoing requests, Beyla will use that value for tracing instead its own generated trace context. If Beyla cannot find an incoming `traceparent` context value, it will generate one according to the W3C specification.
		Beyla will read any incoming trace context header values, track the program execution flow and propagate the trace context by automatically adding the `traceparent` field in outgoing HTTP/gRPC requests. If an application already adds the `taceparent` field in outgoing requests, Beyla will use that value for tracing instead its own generated trace context. If Beyla cannot find an incoming `traceparent` context value, it will generate one according to the W3C specification.

Trace context propagation docs and enable distributed traces by default #1557

Trace context propagation docs and enable distributed traces by default #1557

Conversation

grcevski commented Jan 21, 2025 • edited Loading

codecov bot commented Jan 21, 2025 • edited Loading

Codecov Report

rafaelroquetto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grcevski commented Jan 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marctc left a comment

Choose a reason for hiding this comment

grafsean left a comment

Choose a reason for hiding this comment

grcevski commented Jan 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marctc commented Jan 22, 2025

grcevski commented Jan 21, 2025 •

edited

Loading

codecov bot commented Jan 21, 2025 •

edited

Loading