fix: remove useless fields (correlation_id from ExecutionEvent and _sample_id from MethodSample

Author: kaahos

ddprof-lib/src/main/cpp/event.h

@@ -58,12 +58,10 @@ class ExecutionEvent : public Event {
   ExecutionMode _execution_mode;
   u64 _weight;
   u64 _call_trace_id;
-  u64 _correlation_id;
-  u64 _sample_id;
 
   ExecutionEvent()
       : Event(), _thread_state(OSThreadState::RUNNABLE), _execution_mode(ExecutionMode::UNKNOWN),
-        _weight(1), _call_trace_id(0), _correlation_id(0), _sample_id(0) {}
+        _weight(1), _call_trace_id(0) {}
 };
 
 class AllocEvent : public Event {

ddprof-lib/src/main/cpp/flightRecorder.cpp

@@ -1814,7 +1814,6 @@ void Recording::recordMethodSample(Buffer *buf, int tid, u64 call_trace_id,
   buf->put8(static_cast<int>(event->_execution_mode));
   buf->putVar64(event->_weight);
   buf->putVar64(correlation_id);
-  buf->putVar64(event->_sample_id);
   writeCurrentContext(buf);
   writeEventSizePrefix(buf, start);
   flushIfNeeded(buf);

ddprof-lib/src/main/cpp/jfrMetadata.cpp

@@ -139,7 +139,6 @@ void JfrMetadata::initialize(
                   << field("mode", T_EXECUTION_MODE, "Execution Mode", F_CPOOL)
                   << field("weight", T_LONG, "Sample weight")
                   << field("correlationId", T_LONG, "Async Stack Trace Correlation ID")
-                  << field("sampleId", T_LONG, "Sample ID")
                   << field("spanId", T_LONG, "Span ID")
                   << field("localRootSpanId", T_LONG, "Local Root Span ID") ||
               contextAttributes)

ddprof-lib/src/main/cpp/profiler.cpp

@@ -653,7 +653,6 @@ u64 Profiler::recordJVMTISample(u64 counter, int tid, jthread thread, jint event
 #endif // COUNTERS
   }
   if (!deferred) {
-    setWallSampleIdIfNeeded(event_type, event);
     _jfr.recordEvent(lock_index, tid, call_trace_id, event_type, event);
   }
 
@@ -673,7 +672,6 @@ void Profiler::recordDeferredSample(int tid, u64 call_trace_id, jint event_type,
     return;
   }
 
-  setWallSampleIdIfNeeded(event_type, event);
   _jfr.recordEvent(lock_index, tid, call_trace_id, event_type, event);
 
   _locks[lock_index].unlock();
@@ -743,7 +741,6 @@ bool Profiler::recordSample(void *ucontext, u64 counter, int tid,
     }
 #endif // COUNTERS
   }
-  setWallSampleIdIfNeeded(event_type, event);
   if (event_type == BCI_WALL) {
     static_cast<ExecutionEvent *>(event)->_call_trace_id = call_trace_id;
   }
@@ -757,7 +754,8 @@ bool Profiler::recordSample(void *ucontext, u64 counter, int tid,
 }
 
 bool Profiler::recordSampleDelegated(void *ucontext, u64 weight, int tid,
-                                     jint event_type, Event *event) {
+                                     jint event_type, Event *event,
+                                     u64 *recorded_correlation_id) {
   if (!VM::canRequestStackTrace()) {
     return false;
   }
@@ -790,12 +788,11 @@ bool Profiler::recordSampleDelegated(void *ucontext, u64 weight, int tid,
     return false;
   }
 
-  setWallSampleIdIfNeeded(event_type, event);
-  if (event_type == BCI_WALL) {
-    static_cast<ExecutionEvent *>(event)->_correlation_id = correlation_id;
-  }
   bool recorded =
       _jfr.recordEventDelegated(lock_index, tid, correlation_id, event_type, event);
+  if (recorded && recorded_correlation_id != nullptr) {
+    *recorded_correlation_id = correlation_id;
+  }
   _locks[lock_index].unlock();
   return recorded;
 }
@@ -901,7 +898,6 @@ void Profiler::recordExternalSample(u64 weight, int tid, int num_frames,
 
   u64 call_trace_id =
       _call_trace_storage.put(num_frames, extended_frames, truncated, weight);
-  setWallSampleIdIfNeeded(event_type, event);
   _jfr.recordEvent(lock_index, tid, call_trace_id, event_type, event);
 
   _locks[lock_index].unlock();

ddprof-lib/src/main/cpp/profiler.h

@@ -396,18 +396,14 @@ class alignas(alignof(SpinLock)) Profiler {
   bool recordSample(void *ucontext, u64 weight, int tid, jint event_type,
                     u64 call_trace_id, Event *event,
                     u64 *recorded_call_trace_id = nullptr);
-  void setWallSampleIdIfNeeded(jint event_type, Event *event) {
-    if (event_type == BCI_WALL && _wall_precheck) {
-      ((ExecutionEvent *)event)->_sample_id = atomicIncRelaxed(_sample_seq);
-    }
-  }
   // Delegated sample path: stack-walking is performed by the HotSpot JFR
   // RequestStackTrace extension (the JVM emits the stack trace into its own
   // JFR recording). We only emit the CPU/wall sample event with no
   // stack-trace reference, tagged by the correlation ID we passed to
   // RequestStackTrace as user_data.
   bool recordSampleDelegated(void *ucontext, u64 weight, int tid,
-                             jint event_type, Event *event);
+                             jint event_type, Event *event,
+                             u64 *recorded_correlation_id = nullptr);
   u64 recordJVMTISample(u64 weight, int tid, jthread thread, jint event_type, Event *event, bool deferred);
   void recordDeferredSample(int tid, u64 call_trace_id, jint event_type, Event *event);
   void recordExternalSample(u64 weight, int tid, int num_frames,

ddprof-lib/src/main/cpp/wallClock.cpp

@@ -484,9 +484,10 @@ void WallClockJvmti::signalHandler(int signo, siginfo_t *siginfo,
   // the thread is currently inside JVM-internal (non-Java) code.
   // JVMTI-delegated samples carry a correlation_id, not a call_trace_id, so
   // unowned tail flushing remains limited to the ASGCT wall engine.
+  u64 recorded_correlation_id = 0;
   bool recorded = Profiler::instance()->recordSampleDelegated(
-      nullptr, last_sample, tid, BCI_WALL, &event);
-  finishWallPrecheck(precheck, recorded, event._call_trace_id, event._correlation_id);
+      nullptr, last_sample, tid, BCI_WALL, &event, &recorded_correlation_id);
+  finishWallPrecheck(precheck, recorded, 0, recorded_correlation_id);
   Shims::instance().setSighandlerTid(-1);
   errno = saved_errno;
 }

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/SmokeWallTest.java

@@ -42,6 +42,7 @@ public void test(@CStack String cstack) throws ExecutionException, InterruptedEx
         verifyCStackSettings();
 
         IItemCollection events = verifyEvents("datadog.MethodSample");
+        TaskBlockAssertions.assertMethodSampleSchemaHasCorrelationIdButNoSampleId(events);
 
         for (IItemIterable cpuSamples : events) {
             IMemberAccessor<String, IItem> frameAccessor = JdkAttributes.STACK_TRACE_STRING.getAccessor(cpuSamples.getType());

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockAssertions.java

@@ -29,6 +29,8 @@ final class TaskBlockAssertions {
             attr("suppressedSampleCount", "suppressedSampleCount", "Suppressed Sample Count", NUMBER);
     private static final IAttribute<IQuantity> CORRELATION_ID =
             attr("correlationId", "correlationId", "Async Stack Trace Correlation ID", NUMBER);
+    private static final IAttribute<IQuantity> METHOD_SAMPLE_ID =
+            attr("sampleId", "sampleId", "Sample ID", NUMBER);
     private static final IAttribute<String> OBSERVED_BLOCKING_STATE =
             attr("observedBlockingState", "observedBlockingState", "Observed Blocking State", PLAIN_TEXT);
     private static final IAttribute<IQuantity> EPOCH_TASK_BLOCK_EMITTED =
@@ -105,6 +107,20 @@ static void assertContainsCorrelationId(IItemCollection taskBlockEvents) {
         assertTrue(correlationIds.size() > 0, "Expected at least one non-zero TaskBlock correlationId");
     }
 
+    static void assertMethodSampleSchemaHasCorrelationIdButNoSampleId(IItemCollection methodSampleEvents) {
+        int checked = 0;
+        for (IItemIterable iterable : methodSampleEvents) {
+            checked++;
+            assertTrue(
+                    CORRELATION_ID.getAccessor(iterable.getType()) != null,
+                    "MethodSample must expose correlationId");
+            assertNull(
+                    METHOD_SAMPLE_ID.getAccessor(iterable.getType()),
+                    "MethodSample must not expose sampleId");
+        }
+        assertTrue(checked > 0, "Expected at least one MethodSample type to inspect");
+    }
+
     static void assertWallClockEpochDoesNotExposeTaskBlockCounters(IItemCollection epochEvents) {
         int checked = 0;
         for (IItemIterable iterable : epochEvents) {