dd-trace-go icon indicating copy to clipboard operation
dd-trace-go copied to clipboard

profiler: remove TestProfilerPassthrough and BenchmarkDoRequest

Open nsrip-dd opened this issue 1 year ago • 1 comments

This test does not verify anything useful. It modifies several private variables/fields to change the behavior of the profiler, and makes assertions about the modified behavior. If we change the internal implementation of the profiler, this test could break without identifying an actual bug. Tests should, where possible, only make assertions about the observable behavior (what the profiler sends to the agent) based on the public API (what a customer will actually use). TestAllUploaded does what this test is meant to do, but without perturbing the implementation.

BenchmarkDoRequest also modifies implementation details, and more importantly measures something that is in reality almost entirely I/O-bound in an unrealistic way. The real overhead of the profiler comes from elsewhere: delta profile computation, which we already measure, and the overhead of the profilers themselves, which are better studied on real workloads.

nsrip-dd avatar May 20 '24 14:05 nsrip-dd

Benchmarks

Benchmark execution time: 2024-05-21 16:18:58

Comparing candidate commit 3df13f1ad1fe5ece24be2eccacec40866f83e887 in PR branch PROF-8798-remove-testprofilerpassthrough with baseline commit dfa02a30ff701163a0d40511c57fdf59fdc044f4 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 43 metrics, 1 unstable metrics.

pr-commenter[bot] avatar May 20 '24 14:05 pr-commenter[bot]