Remove custom labels hash and write directly to Trace

Author: gnurizen

support/ebpf/go_labels.ebpf.c

@@ -2,7 +2,6 @@
 
 #include "bpfdefs.h"
 #include "util.h"
-#include "hash.h"
 #include "kernel.h"
 #include "tracemgmt.h"
 #include "tsd.h"
@@ -16,25 +15,25 @@ void process_value(GoMapBucket *map_value, CustomLabelsArray *out, unsigned i) {
         return;
     CustomLabel *lbl = &out->labels[out->len];
     if (map_value->keys[i].str != NULL) {
-        long res = bpf_probe_read_user(lbl->key.key_bytes, CUSTOM_LABEL_MAX_KEY_LEN, map_value->keys[i].str);
+        unsigned klen = MIN(map_value->keys[i].len, CUSTOM_LABEL_MAX_KEY_LEN-1);
+        long res = bpf_probe_read_user(lbl->key, klen, map_value->keys[i].str);
         if (res) {
             DEBUG_PRINT("cl: failed to read key for custom label (%lx): %ld", (unsigned long) map_value->keys[i].str, res);
             return;
         }
-        res = bpf_probe_read_user(lbl->val.val_bytes, CUSTOM_LABEL_MAX_VAL_LEN, map_value->values[i].str);
+        unsigned vlen = MIN(map_value->values[i].len, CUSTOM_LABEL_MAX_VAL_LEN-1);
+        res = bpf_probe_read_user(lbl->val, vlen, map_value->values[i].str);
         if (res) {
             DEBUG_PRINT("cl: failed to read value for custom label: %ld", res);
             return;
         }
-        lbl->key_len = map_value->keys[i].len;
-        lbl->val_len = map_value->values[i].len;
     }
     out->len++;
 }
 
 static inline __attribute__((__always_inline__))
 bool process_bucket(PerCPURecord *record, void *label_buckets, int j) {
-    CustomLabelsArray *out = &record->customLabelsState.cla;
+    CustomLabelsArray *out = &record->trace.custom_labels;
     GoMapBucket *map_value = &record->goMapBucket;
     long res = bpf_probe_read(map_value, sizeof(GoMapBucket), label_buckets + (j * sizeof(GoMapBucket)));
     if (res < 0) {

support/ebpf/hash.h

@@ -4,7 +4,6 @@
 
 #include "bpfdefs.h"
 #include "util.h"
-#include "hash.h"
 #include "kernel.h"
 #include "tracemgmt.h"
 #include "tsd.h"
@@ -142,32 +141,6 @@ bpf_map_def SEC("maps") cl_procs = {
   .max_entries = 128,
 };
 
-#define MAX_BUCKETS 8
-#define MAX_CUSTOM_LABELS_ENTRIES 1000
-
-bpf_map_def SEC("maps") custom_labels = {
-  .type = BPF_MAP_TYPE_HASH,
-  .key_size = sizeof(u64),
-  .value_size = sizeof(CustomLabelsArray),
-  .max_entries = MAX_CUSTOM_LABELS_ENTRIES,
-};
-
-static inline __attribute__((__always_inline__))
-bool add_custom_labels(PerCPURecord *record) {
-  CustomLabelsArray *lbls = &record->customLabelsState.cla;
-  DEBUG_PRINT("cl: got %d custom labels", lbls->len);
-  u64 hash = hash_custom_labels(lbls);
-  int err = bpf_map_update_elem(&custom_labels, &hash, lbls, BPF_ANY);
-  if (err) {
-    DEBUG_PRINT("cl: failed to update custom labels with error %d\n", err);
-    return false;
-  } else {
-    record->trace.custom_labels_hash = hash;
-    DEBUG_PRINT("cl: successfully computed hash 0x%llx for %d custom labels", hash, lbls->len);
-    return true;
-  }
-}
-
 static inline __attribute__((__always_inline__))
 void *get_m_ptr(struct GoCustomLabelsOffsets *offs, UnwindState *state) {
     long res;
@@ -224,15 +197,8 @@ void maybe_add_go_custom_labels(struct pt_regs *ctx, PerCPURecord *record) {
 
     DEBUG_PRINT("cl: trace is within a process with Go custom labels enabled");
     increment_metric(metricID_UnwindGoCustomLabelsAttempts);
-    clear_custom_labels(&record->customLabelsState.cla);
     record->state.processed_go_labels = true;
     tail_call(ctx, PROG_GO_LABELS);
-  } else {
-    if (record->customLabelsState.cla.len > 0) {
-      if (!add_custom_labels(record)) {
-        increment_metric(metricID_UnwindGoCustomLabelsFailures);
-      }
-    }
   }
 }
 
@@ -260,7 +226,7 @@ bool get_native_custom_labels(PerCPURecord *record, NativeCustomLabelsProcInfo *
 
   unsigned ct = 0;
   NativeCustomLabel *lbl_ptr;
-  CustomLabelsArray *out = &record->customLabelsState.cla;
+  CustomLabelsArray *out = &record->trace.custom_labels;
 
 #pragma unroll
   for (int i = 0; i < MAX_CUSTOM_LABELS; i++) {
@@ -276,16 +242,14 @@ bool get_native_custom_labels(PerCPURecord *record, NativeCustomLabelsProcInfo *
     if (!lbl->key.buf)
       continue;
     CustomLabel *out_lbl = &out->labels[ct];
-    unsigned len = MIN(lbl->key.len, CUSTOM_LABEL_MAX_KEY_LEN);
-    out_lbl->key_len = len;
-    if ((err = bpf_probe_read_user(out_lbl->key.key_bytes, len, (void *)lbl->key.buf))) {
+    unsigned klen = MIN(lbl->key.len, CUSTOM_LABEL_MAX_KEY_LEN-1);
+    if ((err = bpf_probe_read_user(out_lbl->key, klen, (void *)lbl->key.buf))) {
       increment_metric(metricID_UnwindNativeCustomLabelsErrReadKey);
       DEBUG_PRINT("cl: failed to read label key: %d", err);
       goto exit;
     }
-    len = MIN(lbl->value.len, CUSTOM_LABEL_MAX_VAL_LEN);
-    out_lbl->val_len = len;
-    if ((err = bpf_probe_read_user(out_lbl->val.val_bytes, len, (void *)lbl->value.buf))) {
+    unsigned vlen = MIN(lbl->value.len, CUSTOM_LABEL_MAX_VAL_LEN-1);
+    if ((err = bpf_probe_read_user(out_lbl->val, vlen, (void *)lbl->value.buf))) {
       increment_metric(metricID_UnwindNativeCustomLabelsErrReadValue);
       DEBUG_PRINT("cl: failed to read label value: %d", err);
       goto exit;
@@ -306,12 +270,8 @@ void maybe_add_native_custom_labels(PerCPURecord *record) {
     DEBUG_PRINT("cl: %d does not support native custom labels", pid);
     return;
   }
-  clear_custom_labels(&record->customLabelsState.cla);
   DEBUG_PRINT("cl: trace is within a process with native custom labels enabled");
   bool success = get_native_custom_labels(record, proc);
-  if (success) {
-    success = add_custom_labels(record);
-  }
   if (success)
     increment_metric(metricID_UnwindNativeCustomLabelsAddSuccesses);
   else

support/ebpf/interpreter_dispatcher.ebpf.c

@@ -223,11 +223,9 @@ static inline PerCPURecord *get_pristine_per_cpu_record()
   record->tailCalls = 0;
   record->ratelimitAction = RATELIMIT_ACTION_DEFAULT;
   record->customLabelsState.go_m_ptr = NULL;
-  // customLabelsState.cla is reset right before use.
 
   Trace *trace = &record->trace;
   trace->kernel_stack_id = -1;
-  trace->custom_labels_hash = 0;
   trace->stack_len = 0;
   trace->pid = 0;
   trace->tid = 0;
@@ -236,6 +234,12 @@ static inline PerCPURecord *get_pristine_per_cpu_record()
   trace->apm_trace_id.as_int.lo = 0;
   trace->apm_transaction_id.as_int = 0;
 
+  u64 *labels_space = (u64*)&trace->custom_labels;
+#pragma unroll
+  for (int i=0; i < sizeof(CustomLabelsArray)/8; i++) {
+    labels_space[i] = 0;
+  }
+
   return record;
 }
 

support/ebpf/tracemgmt.h

@@ -565,20 +565,21 @@ typedef struct __attribute__((packed)) ApmCorrelationBuf {
   ApmSpanID transaction_id;
 } ApmCorrelationBuf;
 
-typedef struct NativeCustomLabelsString {
-  size_t len;
-  const unsigned char *buf;
-} NativeCustomLabelsString;
+#define CUSTOM_LABEL_MAX_KEY_LEN COMM_LEN
+// Big enough to hold UUIDs, etc.
+#define CUSTOM_LABEL_MAX_VAL_LEN 48
 
-typedef struct NativeCustomLabel {
-  NativeCustomLabelsString key;
-  NativeCustomLabelsString value;
-} NativeCustomLabel;
+typedef struct CustomLabel {
+  char key[CUSTOM_LABEL_MAX_KEY_LEN];
+  char val[CUSTOM_LABEL_MAX_VAL_LEN];
+} CustomLabel;
 
-typedef struct NativeCustomLabelsThreadLocalData {
-  NativeCustomLabel *storage;
-  size_t count;
-} NativeCustomLabelsThreadLocalData;
+#define MAX_CUSTOM_LABELS 10
+
+typedef struct CustomLabelsArray {
+    unsigned len;
+    CustomLabel labels[MAX_CUSTOM_LABELS];
+} CustomLabelsArray;
 
 // Container for a stack trace
 typedef struct Trace {
@@ -596,8 +597,8 @@ typedef struct Trace {
   ApmSpanID apm_transaction_id;
   // APM trace ID or all-zero if not present.
   ApmTraceID apm_trace_id;
-  // custom labels hash or zero if not present
-  u64 custom_labels_hash;
+  // Custom Labels
+  CustomLabelsArray custom_labels;
   // The kernel stack ID.
   s32 kernel_stack_id;
   // The number of frames in the stack.
@@ -791,7 +792,7 @@ typedef struct PythonUnwindScratchSpace {
 
 struct GoString {
     char *str;
-    s64 len;
+    u64 len;
 };
 
 struct GoSlice {
@@ -807,38 +808,25 @@ typedef struct GoMapBucket {
     void *overflow;
 } GoMapBucket;
 
-
-// These must be divisible by 8 and a power of 2
-#define CUSTOM_LABEL_MAX_KEY_LEN 64
-#define CUSTOM_LABEL_MAX_VAL_LEN 64
-
-typedef struct CustomLabel {
-    unsigned key_len;
-    unsigned val_len;
-    // If we use unaligned `unsigned char` instead of `u64`
-    // buffers, the hash function becomes too complex to verify.
-    union {
-      u64 key_u64[CUSTOM_LABEL_MAX_KEY_LEN / 8];
-      unsigned char key_bytes[CUSTOM_LABEL_MAX_KEY_LEN];
-    } key;
-    union {
-      u64 val_u64[CUSTOM_LABEL_MAX_VAL_LEN / 8];
-      unsigned char val_bytes[CUSTOM_LABEL_MAX_VAL_LEN];
-    } val;
-} CustomLabel;
-
-#define MAX_CUSTOM_LABELS 14
-
-typedef struct CustomLabelsArray {
-    unsigned len;
-    struct CustomLabel labels[MAX_CUSTOM_LABELS];
-} CustomLabelsArray;
-
 typedef struct CustomLabelsState {
   void *go_m_ptr;
-  CustomLabelsArray cla;
 } CustomLabelsState;
 
+typedef struct NativeCustomLabelsString {
+  size_t len;
+  const unsigned char *buf;
+} NativeCustomLabelsString;
+
+typedef struct NativeCustomLabel {
+  NativeCustomLabelsString key;
+  NativeCustomLabelsString value;
+} NativeCustomLabel;
+
+typedef struct NativeCustomLabelsThreadLocalData {
+  NativeCustomLabel *storage;
+  size_t count;
+} NativeCustomLabelsThreadLocalData;
+
 // Per-CPU info for the stack being built. This contains the stack as well as
 // meta-data on the number of eBPF tail-calls used so far to construct it.
 typedef struct PerCPURecord {

support/ebpf/tracer.ebpf.debug.amd64

@@ -901,22 +901,12 @@ func (t *Tracer) loadBpfTrace(raw []byte, cpu int) *host.Trace {
 		}
 	}
 
-	if ptr.custom_labels_hash != 0 {
-		var lbls C.CustomLabelsArray
-
-		if err := t.ebpfMaps["custom_labels"].Lookup(
-			unsafe.Pointer(&ptr.custom_labels_hash), unsafe.Pointer(&lbls),
-		); err != nil {
-			log.Warnf("Failed to read custom labels: %v", err)
-		}
-
-		trace.CustomLabels = make(map[string]string, int(lbls.len))
-		for i := 0; i < int(lbls.len); i++ {
-			lbl := lbls.labels[i]
-			klen := min(int(lbl.key_len), 64)
-			key := string(lbl.key[0:klen])
-			vlen := min(int(lbl.val_len), 64)
-			val := string(lbl.val[0:vlen])
+	if ptr.custom_labels.len > 0 {
+		trace.CustomLabels = make(map[string]string, int(ptr.custom_labels.len))
+		for i := 0; i < int(ptr.custom_labels.len); i++ {
+			lbl := ptr.custom_labels.labels[i]
+			key := C.GoString((*C.char)(unsafe.Pointer(&lbl.key)))
+			val := C.GoString((*C.char)(unsafe.Pointer(&lbl.val)))
 			trace.CustomLabels[key] = val
 		}
 	}