Detect trace conflicts in Kprobe context propagation (#557)

grafana · Jan 18, 2024 · 72a0ed9 · 72a0ed9
1 parent 1e8f70c
commit 72a0ed9
Show file tree

Hide file tree

Showing 35 changed files with 66 additions and 42 deletions.
diff --git a/bpf/http_sock.h b/bpf/http_sock.h
@@ -285,7 +285,6 @@ static __always_inline void handle_buf_with_connection(pid_connection_info_t *pi
             http_connection_metadata_t *meta = bpf_map_lookup_elem(&filtered_connections, pid_conn);
             get_or_create_trace_info(meta, pid_conn->pid, &pid_conn->conn, u_buf, bytes_len, capture_header_buffer);
 
-#if 0 // disabled for now, until we have better way to detect when it's safe to assume same thread
             if (meta) {            
                 tp_info_pid_t *tp_p = trace_info_for_connection(&pid_conn->conn);
                 if (tp_p) {
@@ -294,11 +293,11 @@ static __always_inline void handle_buf_with_connection(pid_connection_info_t *pi
                     if (meta->type == EVENT_HTTP_CLIENT && !valid_span(tp_p->tp.parent_id)) {
                         bpf_dbg_printk("Looking for trace id of a client span");
                         u64 pid_tid = bpf_get_current_pid_tgid();
-                        tp_info_t *server_tp = bpf_map_lookup_elem(&server_traces, &pid_tid);
-                        if (server_tp) {
+                        tp_info_pid_t *server_tp = bpf_map_lookup_elem(&server_traces, &pid_tid);
+                        if (server_tp && server_tp->valid) {
                             bpf_dbg_printk("Found existing server span for id=%llx", pid_tid);
-                            bpf_memcpy(info->tp.trace_id, server_tp->trace_id, sizeof(info->tp.trace_id));
-                            bpf_memcpy(info->tp.parent_id, server_tp->span_id, sizeof(info->tp.parent_id));
+                            bpf_memcpy(info->tp.trace_id, server_tp->tp.trace_id, sizeof(info->tp.trace_id));
+                            bpf_memcpy(info->tp.parent_id, server_tp->tp.span_id, sizeof(info->tp.parent_id));
                         } else {
                             bpf_dbg_printk("Cannot find server span for id=%llx", pid_tid);
                         }
@@ -309,7 +308,7 @@ static __always_inline void handle_buf_with_connection(pid_connection_info_t *pi
             } else {
                 bpf_dbg_printk("No META!");
             }
-#endif            
+
             // we copy some small part of the buffer to the info trace event, so that we can process an event even with
             // incomplete trace info in user space.
             bpf_probe_read(info->buf, FULL_BUF_SIZE, u_buf);

diff --git a/bpf/trace_common.h b/bpf/trace_common.h
@@ -10,6 +10,7 @@
 typedef struct tp_info_pid {
     tp_info_t tp;
     u32 pid;
+    u8  valid;
 } tp_info_pid_t;
 
 struct {
@@ -118,6 +119,15 @@ static __always_inline void server_or_client_trace(http_connection_metadata_t *m
     }
     if (meta->type == EVENT_HTTP_REQUEST) {
         u64 pid_tid = bpf_get_current_pid_tgid();
+
+        tp_info_pid_t *existing = bpf_map_lookup_elem(&server_traces, &pid_tid);
+        // we have a conflict, mark this invalid and do nothing
+        if (existing) {
+            bpf_dbg_printk("Found conflicting server span, marking as invalid, id=%llx", pid_tid);
+            existing->valid = 0;
+            return;
+        }
+
         bpf_dbg_printk("Saving server span for id=%llx", pid_tid);
         bpf_map_update_elem(&server_traces, &pid_tid, tp_p, BPF_ANY);
     }
@@ -155,19 +165,19 @@ static __always_inline void get_or_create_trace_info(http_connection_metadata_t
 
     tp_p->tp.ts = bpf_ktime_get_ns();
     tp_p->tp.flags = 1;
+    tp_p->valid = 1;
     tp_p->pid = pid; // used for avoiding finding stale server requests with client port reuse
     urand_bytes(tp_p->tp.span_id, SPAN_ID_SIZE_BYTES);
 
     u8 found_tp = 0;
 
-#if 0 // disabled for now, until we have better way to detect when it's safe to assume same thread
     if (meta) {
         if (meta->type == EVENT_HTTP_CLIENT) {
             tp_p->pid = -1; // we only want to prevent correlation of duplicate server calls by PID
             u64 pid_tid = bpf_get_current_pid_tgid();
             tp_info_pid_t *server_tp = bpf_map_lookup_elem(&server_traces, &pid_tid);
 
-            if (server_tp) {
+            if (server_tp && server_tp->valid) {
                 found_tp = 1;
                 bpf_dbg_printk("Found existing server tp for client call");
                 bpf_memcpy(tp_p->tp.trace_id, server_tp->tp.trace_id, sizeof(tp_p->tp.trace_id));
@@ -184,7 +194,6 @@ static __always_inline void get_or_create_trace_info(http_connection_metadata_t
             } 
         }
     }
-#endif    
 
     if (!found_tp) {
         bpf_dbg_printk("Generating new traceparent id");

diff --git a/pkg/internal/ebpf/httpfltr/bpf_bpfel_arm64.go b/pkg/internal/ebpf/httpfltr/bpf_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_bpfel_arm64.o b/pkg/internal/ebpf/httpfltr/bpf_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpfltr/bpf_bpfel_x86.go b/pkg/internal/ebpf/httpfltr/bpf_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_bpfel_x86.o b/pkg/internal/ebpf/httpfltr/bpf_bpfel_x86.o
diff --git a/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_arm64.go b/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_arm64.o b/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_x86.go b/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_x86.o b/pkg/internal/ebpf/httpfltr/bpf_debug_bpfel_x86.o
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_arm64.go b/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_arm64.o b/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_x86.go b/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_x86.o b/pkg/internal/ebpf/httpfltr/bpf_tp_bpfel_x86.o
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_arm64.go b/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_arm64.o b/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_x86.go b/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_x86.o b/pkg/internal/ebpf/httpfltr/bpf_tp_debug_bpfel_x86.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_bpfel_arm64.go b/pkg/internal/ebpf/httpssl/bpf_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_bpfel_arm64.o b/pkg/internal/ebpf/httpssl/bpf_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_bpfel_x86.go b/pkg/internal/ebpf/httpssl/bpf_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_bpfel_x86.o b/pkg/internal/ebpf/httpssl/bpf_bpfel_x86.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_arm64.go b/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_arm64.o b/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_x86.go b/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_x86.o b/pkg/internal/ebpf/httpssl/bpf_debug_bpfel_x86.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_arm64.go b/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_arm64.o b/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_x86.go b/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_x86.o b/pkg/internal/ebpf/httpssl/bpf_tp_bpfel_x86.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_arm64.go b/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_arm64.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_arm64.o b/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_x86.go b/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_x86.go
diff --git a/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_x86.o b/pkg/internal/ebpf/httpssl/bpf_tp_debug_bpfel_x86.o
diff --git a/test/integration/suites_test.go b/test/integration/suites_test.go
@@ -14,7 +14,7 @@ import (
 	"github.com/grafana/beyla/test/integration/components/docker"
 )
 
-var kprobeTraces = false // disabled for now until we figure a safe way to determine server client relationships
+var kprobeTraces = true // allow tests to run distributed traces tests
 
 func TestSuite(t *testing.T) {
 	compose, err := docker.ComposeSuite("docker-compose.yml", path.Join(pathOutput, "test-suite.log"))