Propagate HTTP trace context in Go #491
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #491 +/- ##
===========================================
- Coverage 77.25% 66.23% -11.02%
===========================================
Files 67 64 -3
Lines 5302 5228 -74
===========================================
- Hits 4096 3463 -633
- Misses 981 1489 +508
- Partials 225 276 +51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -61,6 +61,8 @@ int uprobe_ServeHTTP(struct pt_regs *ctx) { | |||
|
|
|||
| if (req) { | |||
| server_trace_parent(goroutine_addr, &invocation.tp, (void*)(req + req_header_ptr_pos)); | |||
| // TODO: if context propagation is supported, overwrite the header value in the map with the | |||
| // new span context and the same thread id. | |||
There was a problem hiding this comment.
What happens if we don't do it now? Is the feature incomplete?
There was a problem hiding this comment.
I don't see it as incomplete in terms of Beyla instrumentation, but it would be nice to make Beyla work well for instrumented applications with the Go SDK. As it stands now, Beyla will correctly propagate the context, but if there's SDK instrumentation as well, the two spans will look parallel, just as it was before. If we overwrite the header value, we'll manage to nest them. So it's an extension to the feature to make it work with auto and manual instrumentation.
| @@ -86,6 +86,7 @@ services: | |||
| - --config=/configs/beyla-config.yml | |||
| volumes: | |||
| - ./configs/:/configs | |||
| - ./system/sys/kernel/security:/sys/kernel/security | |||
There was a problem hiding this comment.
Will we have to document this e.g. to allow running Beyla in Kubernetes?
There was a problem hiding this comment.
Oh this is a very good point. I need to document this. As it stands now, if security isn't there we'll assume we can propagate the context, since the security file is missing. But it could be because the users didn't mount it and the host machine doesn't allow it. I'll follow-up with a PR on this.
There was a problem hiding this comment.
I opened an issue so I don't forget #502
| //go:generate $BPF2GO -cc $BPF_CLANG -cflags $BPF_CFLAGS -target amd64,arm64 bpf ../../../../bpf/go_nethttp.c -- -I../../../../bpf/headers -DNO_HEADER_PROPAGATION | ||
| //go:generate $BPF2GO -cc $BPF_CLANG -cflags $BPF_CFLAGS -target amd64,arm64 bpf_debug ../../../../bpf/go_nethttp.c -- -I../../../../bpf/headers -DBPF_DEBUG -DNO_HEADER_PROPAGATION | ||
| //go:generate $BPF2GO -cc $BPF_CLANG -cflags $BPF_CFLAGS -target amd64,arm64 bpf_tp ../../../../bpf/go_nethttp.c -- -I../../../../bpf/headers | ||
| //go:generate $BPF2GO -cc $BPF_CLANG -cflags $BPF_CFLAGS -target amd64,arm64 bpf_tp_debug ../../../../bpf/go_nethttp.c -- -I../../../../bpf/headers -DBPF_DEBUG |
There was a problem hiding this comment.
This is going to grow exponentially 😅
There was a problem hiding this comment.
I know, I've been thinking about this, but I can't find a good solution that allows us to support various configurations :(.
This PR effectively finishes the work to propagate the trace context for HTTP Go applications (no gRPC), by writing the discovered trace id and context id in the HTTP headers. Few details on how this works:
TODO: