Add trace-ID integration testing

Consolidate probe.bpf.c and go_types.h into go_traceparent.h, define magic numbers.
grafana · Aug 31, 2023 · 0f886c6 · 0f886c6
1 parent 3ad1d38
commit 0f886c6
Show file tree

Hide file tree

Showing 10 changed files with 90 additions and 65 deletions.
diff --git a/bpf/go_nethttp.c b/bpf/go_nethttp.c
@@ -16,7 +16,7 @@
 #include "bpf_dbg.h"
 #include "go_common.h"
 #include "go_nethttp.h"
-#include "probe.bpf.c"
+#include "go_traceparent.h"
 
 struct {
     __uint(type, BPF_MAP_TYPE_HASH);
@@ -150,7 +150,7 @@ int uprobe_WriteHeader(struct pt_regs *ctx) {
     }
     bpf_probe_read(&trace->content_length, sizeof(trace->content_length), (void *)(req_ptr + content_length_ptr_pos));
 
-    void *traceparent_ptr = extract_context_from_req_headers((void*)(req_ptr + req_header_ptr_pos));
+    void *traceparent_ptr = extract_traceparent_from_req_headers((void*)(req_ptr + req_header_ptr_pos));
     if (traceparent_ptr != NULL) {
         long res = bpf_probe_read(trace->traceparent, sizeof(trace->traceparent), traceparent_ptr);
         if (res < 0) {

diff --git a/bpf/probe.bpf.c → bpf/go_traceparent.h b/bpf/probe.bpf.c → bpf/go_traceparent.h
@@ -15,11 +15,51 @@
 #include "utils.h"
 #include "stdbool.h"
 #include "bpf_dbg.h"
-#include "go_types.h"
+#include "bpf_helpers.h"
+
 #define MAX_BUCKETS 8
 #define W3C_KEY_LENGTH 11
 #define W3C_VAL_LENGTH 55
 
+#define MAX_REALLOCATION 400
+#define MAX_DATA_SIZE 400
+
+#define OFFSET_OF_GO_RUNTIME_HMAP_FIELD_B 9
+#define OFFSET_OF_GO_RUNTIME_HMAP_FIELD_BUCKETS 16
+
+struct go_string
+{
+    char *str;
+    s64 len;
+};
+
+struct go_slice
+{
+    void *array;
+    s64 len;
+    s64 cap;
+};
+
+struct go_slice_user_ptr
+{
+    void *array;
+    void *len;
+    void *cap;
+};
+
+struct go_iface
+{
+    void *tab;
+    void *data;
+};
+
+struct map_bucket {
+    char tophash[8];
+    struct go_string keys[8];
+    struct go_slice values[8];
+    void *overflow;
+};
+
 struct
 {
     __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
@@ -41,7 +81,7 @@ static __always_inline _Bool bpf_memcmp(char *s1, char *s2, s32 size)
     return true;
 }
 
-static __always_inline void *extract_context_from_req_headers(void *headers_ptr_ptr)
+static __always_inline void *extract_traceparent_from_req_headers(void *headers_ptr_ptr)
 {
     void *headers_ptr;
     long res;
@@ -61,14 +101,14 @@ static __always_inline void *extract_context_from_req_headers(void *headers_ptr_
         return NULL;
     }
     unsigned char log_2_bucket_count;
-    res = bpf_probe_read(&log_2_bucket_count, sizeof(log_2_bucket_count), headers_ptr + 9);
+    res = bpf_probe_read(&log_2_bucket_count, sizeof(log_2_bucket_count), headers_ptr + OFFSET_OF_GO_RUNTIME_HMAP_FIELD_B);
     if (res < 0)
     {
         return NULL;
     }
     u64 bucket_count = 1 << log_2_bucket_count;
     void *header_buckets;
-    res = bpf_probe_read(&header_buckets, sizeof(header_buckets), headers_ptr + 16);
+    res = bpf_probe_read(&header_buckets, sizeof(header_buckets), headers_ptr + OFFSET_OF_GO_RUNTIME_HMAP_FIELD_BUCKETS);
     if (res < 0)
     {
         return NULL;

diff --git a/bpf/headers/go_types.h b/bpf/headers/go_types.h
diff --git a/pkg/internal/ebpf/nethttp/bpf_bpfel_arm64.o b/pkg/internal/ebpf/nethttp/bpf_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/nethttp/bpf_bpfel_x86.o b/pkg/internal/ebpf/nethttp/bpf_bpfel_x86.o
diff --git a/pkg/internal/ebpf/nethttp/bpf_debug_bpfel_arm64.o b/pkg/internal/ebpf/nethttp/bpf_debug_bpfel_arm64.o
diff --git a/pkg/internal/ebpf/nethttp/bpf_debug_bpfel_x86.o b/pkg/internal/ebpf/nethttp/bpf_debug_bpfel_x86.o
diff --git a/pkg/internal/transform/spanner.go b/pkg/internal/transform/spanner.go
@@ -108,12 +108,14 @@ func extractIP(b []uint8, size int) string {
 }
 
 func extractTraceID(traceparent [55]byte) string {
-	// If traceparent was not set in eBPF, entire field should be all zeroes.
+	// If traceparent was not set in eBPF, entire field should be zeroed bytes.
 	if traceparent[0] == 0 {
 		return ""
 	}
+
+	// It is assumed that eBPF code has already verified the length is exactly 55
 	// See https://www.w3.org/TR/trace-context/#traceparent-header-field-values for format.
-	// Traceparent starts with version, e.g. "00-". TraceID is the following 32 hex digits
+	// 2 hex version + dash + 32 hex traceID + dash + 16 hex parent + dash + 2 hex flags
 	return string(traceparent[3:35])
 }
 

diff --git a/test/integration/suites_test.go b/test/integration/suites_test.go
@@ -18,6 +18,7 @@ func TestSuite(t *testing.T) {
 	t.Run("RED metrics", testREDMetricsHTTP)
 	t.Run("HTTP traces", testHTTPTraces)
 	t.Run("HTTP traces (no traceID)", testHTTPTracesNoTraceID)
+	t.Run("HTTP traces (bad traceID)", testHTTPTracesBadTraceID)
 	t.Run("GRPC traces", testGRPCTraces)
 	t.Run("GRPC RED metrics", testREDMetricsGRPC)
 	t.Run("Internal Prometheus metrics", testInternalPrometheusExport)

diff --git a/test/integration/traces_test.go b/test/integration/traces_test.go
@@ -23,7 +23,6 @@ func testHTTPTracesNoTraceID(t *testing.T) {
 func testHTTPTraces(t *testing.T) {
 	testHTTPTracesCommon(t, true)
 }
-
 func testHTTPTracesCommon(t *testing.T, doTraceID bool) {
 	var traceID string
 	slug := "create-trace"
@@ -126,6 +125,45 @@ func testHTTPTracesCommon(t *testing.T, doTraceID bool) {
 	}), "not all tags matched in %+v", process.Tags)
 }
 
+func testHTTPTracesBadTraceID(t *testing.T) {
+	slugToParent := map[string]string{
+		// Valid traceparent example:
+		//		valid: "00-5fe865607da112abd799ea8108c38bcb-4c59e9a913c480a3-01"
+		// Examples of INVALID traceIDs in traceparent:  Note: eBPF rejects when len != 55
+		"invalid-trace-id1": "00-Zfe865607da112abd799ea8108c38bcb-4c59e9a913c480a3-01",
+		"invalid-trace-id2": "00-5fe865607da112abd799ea8108c38bcL-4c59e9a913c480a3-01",
+		"invalid-trace-id3": "00-5fe865607Ra112abd799ea8108c38bcb-4c59e9a913c480a3-01",
+		"invalid-trace-id4": "00-0x5fe865607da112abd799ea8108c3cb-4c59e9a913c480a3-01",
+		"invalid-trace-id5": "00-5FE865607DA112ABD799EA8108C38BCB-4c59e9a913c480a3-01",
+	}
+	for slug, traceparent := range slugToParent {
+		t.Log("Testing bad traceid. traceparent:", traceparent, "slug:", slug)
+		doHTTPGetWithTraceparent(t, instrumentedServiceStdURL+"/"+slug+"?delay=10ms", 200, traceparent)
+
+		var trace jaeger.Trace
+		test.Eventually(t, testTimeout, func(t require.TestingT) {
+			resp, err := http.Get(jaegerQueryURL + "?service=testserver&operation=GET%20%2F" + slug)
+			require.NoError(t, err)
+			if resp == nil {
+				return
+			}
+			require.Equal(t, http.StatusOK, resp.StatusCode)
+			var tq jaeger.TracesQuery
+			require.NoError(t, json.NewDecoder(resp.Body).Decode(&tq))
+			traces := tq.FindBySpan(jaeger.Tag{Key: "http.target", Type: "string", Value: "/" + slug})
+			require.Len(t, traces, 1)
+			trace = traces[0]
+		}, test.Interval(100*time.Millisecond))
+
+		// Check the information of the parent span
+		res := trace.FindByOperationName("GET /" + slug)
+		require.Len(t, res, 1)
+		parent := res[0]
+		require.NotEmpty(t, parent.TraceID)
+		require.NotEqual(t, traceparent[3:35], parent.TraceID)
+	}
+}
+
 func testGRPCTraces(t *testing.T) {
 	require.Error(t, grpcclient.Debug(10*time.Millisecond, true))