fix(profiling): reverse locations for stack v2 [backport-2.14] (#10871)

Manual backport of #10851 to
2.14

Fixes an issue where flamegraph was upside down for stack v2, when
DD_PROFILING_STACK_V2_ENABLED

## Checklist

- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp

@@ -63,6 +63,8 @@ extern "C"
                          int64_t line);
     void ddup_push_monotonic_ns(Datadog::Sample* sample, int64_t monotonic_ns);
     void ddup_flush_sample(Datadog::Sample* sample);
+    // Stack v2 specific flush, which reverses the locations
+    void ddup_flush_sample_v2(Datadog::Sample* sample);
     void ddup_drop_sample(Datadog::Sample* sample);
 #ifdef __cplusplus
 } // extern "C"

ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp

@@ -84,7 +84,7 @@ class Sample
     );
 
     // Flushes the current buffer, clearing it
-    bool flush_sample();
+    bool flush_sample(bool reverse_locations = false);
 
     static ddog_prof_Profile& profile_borrow();
     static void profile_release();

ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp

@@ -271,6 +271,12 @@ ddup_flush_sample(Datadog::Sample* sample) // cppcheck-suppress unusedFunction
     sample->flush_sample();
 }
 
+void
+ddup_flush_sample_v2(Datadog::Sample* sample) // cppcheck-suppress unusedFunction
+{
+    sample->flush_sample(/*reverse_locations*/ true);
+}
+
 void
 ddup_drop_sample(Datadog::Sample* sample) // cppcheck-suppress unusedFunction
 {

ddtrace/internal/datadog/profiling/dd_wrapper/src/sample.cpp

@@ -1,5 +1,6 @@
 #include "sample.hpp"
 
+#include <algorithm>
 #include <chrono>
 #include <thread>
 
@@ -104,14 +105,18 @@ Datadog::Sample::clear_buffers()
 }
 
 bool
-Datadog::Sample::flush_sample()
+Datadog::Sample::flush_sample(bool reverse_locations)
 {
     if (dropped_frames > 0) {
         const std::string name =
           "<" + std::to_string(dropped_frames) + " frame" + (1 == dropped_frames ? "" : "s") + " omitted>";
         Sample::push_frame_impl(name, "", 0, 0);
     }
 
+    if (reverse_locations) {
+        std::reverse(locations.begin(), locations.end());
+    }
+
     const ddog_prof_Sample sample = {
         .locations = { locations.data(), locations.size() },
         .values = { values.data(), values.size() },

ddtrace/internal/datadog/profiling/stack_v2/src/stack_renderer.cpp

@@ -144,7 +144,7 @@ StackRenderer::render_stack_end()
         return;
     }
 
-    ddup_flush_sample(sample);
+    ddup_flush_sample_v2(sample);
     ddup_drop_sample(sample);
     sample = nullptr;
 }

releasenotes/notes/profiling-stack-v2-upside-down-82ebd3073b5e8e43.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    profiling: fixes an issue where flamegraph was upside down for stack v2,
+      ``DD_PROFILING_STACK_V2_ENABLED``.

tests/profiling/collector/pprof_utils.py

@@ -28,6 +28,12 @@ def reinterpret_int_as_int64(value: int) -> int:
     return ctypes.c_int64(value).value
 
 
+class StackLocation:
+    def __init__(self, function_name: str, filename: str):
+        self.function_name = function_name
+        self.filename = filename
+
+
 class LockEventType(Enum):
     ACQUIRE = 1
     RELEASE = 2
@@ -50,7 +56,8 @@ def __init__(
 
 
 class StackEvent(EventBaseClass):
-    def __init__(self, *args, **kwargs):
+    def __init__(self, locations: typing.Optional[typing.Any] = None, *args, **kwargs):
+        self.locations = locations
         super().__init__(*args, **kwargs)
 
 
@@ -265,5 +272,23 @@ def assert_lock_event(profile, sample: pprof_pb2.Sample, expected_event: LockEve
     assert_base_event(profile, sample, expected_event)
 
 
+# helper function to check whether the expected stack event is present in the samples
+def has_sample_with_locations(
+    profile, samples: typing.List[pprof_pb2.Sample], expected_locations: typing.List[StackLocation]
+) -> bool:
+    for sample in samples:
+        for location_id, expected_location in zip(sample.location_id, expected_locations):
+            location = get_location_with_id(profile, location_id)
+            function = get_function_with_id(profile, location.line[0].function_id)
+            if profile.string_table[function.name] != expected_location.function_name:
+                continue
+            if not profile.string_table[function.filename].endswith(expected_location.filename):
+                continue
+
+            return True
+
+    return False
+
+
 def assert_stack_event(profile, sample: pprof_pb2.Sample, expected_event: StackEvent):
     assert_base_event(profile, sample, expected_event)

tests/profiling_v2/collector/test_stack.py

@@ -1,6 +1,61 @@
 import pytest
 
 
+@pytest.mark.subprocess(env=dict(DD_PROFILING_STACK_V2_ENABLED="true"))
+def test_stack_v2_locations():
+    import os
+    import time
+
+    from ddtrace.internal.datadog.profiling import ddup
+    from ddtrace.profiling.collector import stack
+    from tests.profiling.collector import pprof_utils
+
+    test_name = "test_locations"
+    pprof_prefix = "/tmp/" + test_name
+    output_filename = pprof_prefix + "." + str(os.getpid())
+
+    assert ddup.is_available
+    ddup.config(env="test", service=test_name, version="my_version", output_filename=pprof_prefix)
+    ddup.start()
+
+    def baz():
+        time.sleep(0.01)
+
+    def bar():
+        baz()
+
+    def foo():
+        bar()
+
+    with stack.StackCollector(None):
+        for _ in range(5):
+            foo()
+    ddup.upload()
+
+    profile = pprof_utils.parse_profile(output_filename)
+    samples = pprof_utils.get_samples_with_value_type(profile, "wall-time")
+    assert len(samples) > 0
+
+    expected_locations = [
+        pprof_utils.StackLocation(
+            function_name="baz",
+            filename="test_stack.py",
+        ),
+        pprof_utils.StackLocation(
+            function_name="bar",
+            filename="test_stack.py",
+        ),
+        pprof_utils.StackLocation(
+            function_name="foo",
+            filename="test_stack.py",
+        ),
+    ]
+
+    assert pprof_utils.has_sample_with_locations(
+        profile, samples, expected_locations
+    ), "Sample with expected locations not found"
+
+
 # Tests here are marked as subprocess as they are flaky when not marked as such,
 # similar to test_user_threads_have_native_id in test_threading.py. For some
 # reason, when the Span is created, it's not linked to the MainThread, and the