Bump key size to 25 (#162)

Author: umanwizard

interpreter/golabels/integrationtests/golabels_integration_test.go

@@ -141,7 +141,10 @@ func Test_Golabels(t *testing.T) {
 							require.True(t, strings.HasPrefix(v, "label2"))
 							hits |= (1 << 1)
 						} else if strings.HasPrefix(k, "l3") {
-							require.Len(t, v, 47)
+							// The maximum. Bump this, and
+							// the value in pprof_test.go, if
+							// CUSTOM_LABEL_MAX_VAL_LEN changes.
+							require.Len(t, v, 53)
 							require.True(t, strings.HasPrefix(v, "label3"))
 							hits |= (1 << 2)
 						}

interpreter/golabels/integrationtests/pprof/pprof_test.go

@@ -27,7 +27,9 @@ func setPprofLabels(t *testing.T, ctx context.Context, cookie string, busyFunc f
 	labels := pprof.Labels(
 		"l1"+cookie, "label1"+randomString(16),
 		"l2"+cookie, "label2"+randomString(24),
-		"l3"+cookie, "label3"+randomString(48))
+		// One more than the maximum -- in golabels_integration_test.go
+		// we will see it get cut off.
+		"l3"+cookie, "label3"+randomString(54))
 	lastUpdate := time.Now()
 	pprof.Do(ctx, labels, func(context.Context) {
 		for time.Since(lastUpdate) < 10*time.Second {

support/ebpf/go_labels.ebpf.c

@@ -30,13 +30,13 @@ get_go_custom_labels_from_slice(PerCPURecord *record, void *labels_slice_ptr)
     if (i >= labels_slice.len)
       break;
     CustomLabel *lbl = &out->labels[i];
-    u8 klen          = MIN(record->labels[i * 2].len, CUSTOM_LABEL_MAX_KEY_LEN - 1);
+    u8 klen          = MIN(record->labels[i * 2].len, CUSTOM_LABEL_MAX_KEY_LEN);
     if (bpf_probe_read_user(lbl->key, klen, record->labels[i * 2].str)) {
       DEBUG_PRINT(
         "cl: failed to read key for custom label (%lx)", (unsigned long)record->labels[i * 2].str);
       return false;
     }
-    u8 vlen = MIN(record->labels[i * 2 + 1].len, CUSTOM_LABEL_MAX_VAL_LEN - 1);
+    u8 vlen = MIN(record->labels[i * 2 + 1].len, CUSTOM_LABEL_MAX_VAL_LEN);
     if (bpf_probe_read_user(lbl->val, vlen, record->labels[i * 2 + 1].str)) {
       DEBUG_PRINT(
         "cl: failed to read key for custom label (%lx)",
@@ -110,11 +110,11 @@ get_go_custom_labels_from_map(PerCPURecord *record, void *labels_map_ptr_ptr, Go
       char *vstr       = map_value->values[i].str;
       unsigned vlen    = map_value->values[i].len;
       if (tophash != 0 && kstr != NULL) {
-        if (bpf_probe_read_user(lbl->key, MIN(klen, CUSTOM_LABEL_MAX_KEY_LEN - 1), kstr)) {
+        if (bpf_probe_read_user(lbl->key, MIN(klen, CUSTOM_LABEL_MAX_KEY_LEN), kstr)) {
           DEBUG_PRINT("cl: failed to read key for custom label (%lx)", (unsigned long)kstr);
           return false;
         }
-        if (bpf_probe_read_user(lbl->val, MIN(vlen, CUSTOM_LABEL_MAX_VAL_LEN - 1), vstr)) {
+        if (bpf_probe_read_user(lbl->val, MIN(vlen, CUSTOM_LABEL_MAX_VAL_LEN), vstr)) {
           DEBUG_PRINT("cl: failed to read value for custom label");
           return false;
         }

support/ebpf/interpreter_dispatcher.ebpf.c

@@ -355,13 +355,13 @@ read_labelset_into_trace(PerCPURecord *record, NativeCustomLabelsSet *p_current_
     if (!lbl->key.buf)
       continue;
     CustomLabel *out_lbl = &out->labels[ct];
-    unsigned klen        = MIN(lbl->key.len, CUSTOM_LABEL_MAX_KEY_LEN - 1);
+    unsigned klen        = MIN(lbl->key.len, CUSTOM_LABEL_MAX_KEY_LEN);
     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;
     }
-    unsigned vlen = MIN(lbl->value.len, CUSTOM_LABEL_MAX_VAL_LEN - 1);
+    unsigned vlen = MIN(lbl->value.len, CUSTOM_LABEL_MAX_VAL_LEN);
     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);

support/ebpf/tracemgmt.h

@@ -258,9 +258,8 @@ static inline EBPF_INLINE PerCPURecord *get_pristine_per_cpu_record()
   trace->apm_transaction_id.as_int = 0;
 
   trace->custom_labels.len = 0;
-  u64 *labels_space        = (u64 *)&trace->custom_labels.labels;
-  // I'm not sure this is necessary since we only increment len after
-  // we successfully write the label.
+  _Static_assert(sizeof(CustomLabel) % 8 == 0, "CustomLabel size must be a multiple of 8 bytes.");
+  u64 *labels_space = (u64 *)&trace->custom_labels.labels;
   UNROLL for (int i = 0; i < sizeof(CustomLabel) * MAX_CUSTOM_LABELS / 8; i++)
   {
     labels_space[i] = 0;

support/ebpf/tracer.ebpf.amd64

@@ -600,13 +600,12 @@ typedef struct __attribute__((packed)) ApmCorrelationBuf {
   ApmSpanID transaction_id;
 } ApmCorrelationBuf;
 
-#define CUSTOM_LABEL_MAX_KEY_LEN COMM_LEN
-// Big enough to hold UUIDs, etc.
-#define CUSTOM_LABEL_MAX_VAL_LEN 48
+#define CUSTOM_LABEL_MAX_KEY_LEN 25
+#define CUSTOM_LABEL_MAX_VAL_LEN 53
 
 typedef struct CustomLabel {
-  u8 key[CUSTOM_LABEL_MAX_KEY_LEN];
-  u8 val[CUSTOM_LABEL_MAX_VAL_LEN];
+  u8 key[CUSTOM_LABEL_MAX_KEY_LEN + 1];
+  u8 val[CUSTOM_LABEL_MAX_VAL_LEN + 1];
 } CustomLabel;
 
 typedef struct NativeCustomLabelsString {