fix(tracing): return trace regardless of sampling decision unless stats comp enabled backport 1.20 (#8399)

Note this backport modifies a snapshot file in addition to the backport,
since that was backported to 1.20 originally, but never ran due to test
suite issues.

Backport 9427b01 from #8385 to 1.20.

Return trace regardless of sampling decision unless stats computation is
on.
## Checklist

- [x] Change(s) are motivated and described in the PR description
- [x] Testing strategy is described if automated tests are not included
in the PR
- [x] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.
- [x] If change touches code that signs or publishes builds or packages,
or handles credentials of any kind, I've requested a review from
`@DataDog/security-design-and-guidance`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [x] 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)

---------

Co-authored-by: Zachary Groves <[email protected]>
Co-authored-by: Juanjo Alvarez Martinez <[email protected]>
Co-authored-by: Christophe Papazian <[email protected]>
Co-authored-by: Federico Mon <[email protected]>
Co-authored-by: ZStriker19 <[email protected]>

Author: 5 people

ddtrace/internal/processor/trace.py

@@ -95,11 +95,9 @@ def process_trace(self, trace):
 
                     return single_spans or None
 
-            for span in trace:
-                if span.sampled:
-                    return trace
+            return trace
 
-            log.debug("dropping trace %d with %d spans", trace[0].trace_id, len(trace))
+        log.debug("dropping trace %d with %d spans", trace[0].trace_id, len(trace))
 
         return None
 

releasenotes/notes/always_send_trace_to_agent-f5dad70a40aa92a9.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    tracing: This fix resolves an issue where previously some traces that were not sampled were not sent to the trace-agent, possibly affecting metrics.
+    With this fix, all traces are sent to the agent.
+    

tests/snapshots/test_sampling_with_rate_sampler_with_tiny_rate.json

@@ -1,35 +1,30 @@
 [[
   {
     "name": "trace8",
-    "service": "",
+    "service": null,
     "resource": "trace8",
     "trace_id": 0,
     "span_id": 1,
     "parent_id": 0,
-    "type": "",
-    "error": 0,
     "meta": {
-      "_dd.p.tid": "65cbd89000000000",
       "language": "python",
-      "runtime-id": "7a2aa1b276634a76b25c928fffea4cdc"
+      "runtime-id": "3fca1d80c0964fbbb4f687fc836bc225"
     },
     "metrics": {
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
-      "process_id": 83161
+      "process_id": 55908
     },
-    "duration": 64000,
-    "start": 1707858064774777000
+    "duration": 127000,
+    "start": 1709568577619719000
   },
      {
        "name": "child",
-       "service": "",
+       "service": null,
        "resource": "child",
        "trace_id": 0,
        "span_id": 2,
        "parent_id": 1,
-       "type": "",
-       "error": 0,
-       "duration": 13000,
-       "start": 1707858064774816000
+       "duration": 59000,
+       "start": 1709568577619765000
      }]]

tests/tracer/test_sampler.py

@@ -104,12 +104,13 @@ def test_sample_rate_deviation(self):
                 span.finish()
 
             samples = tracer.pop()
-
+            # non sampled spans do not have sample rate applied
+            sampled_spans = [s for s in samples if s.sampled]
             assert (
-                samples[0].get_metric(SAMPLE_RATE_METRIC_KEY) == sample_rate
+                sampled_spans[0].get_metric(SAMPLE_RATE_METRIC_KEY) == sample_rate
             ), "Sampled span should have sample rate properly assigned"
 
-            deviation = abs(len(samples) - (iterations * sample_rate)) / (iterations * sample_rate)
+            deviation = abs(len(sampled_spans) - (iterations * sample_rate)) / (iterations * sample_rate)
             assert (
                 deviation < 0.05
             ), "Actual sample rate should be within 5 percent of set sample " "rate (actual: %f, set: %f)" % (
@@ -131,7 +132,7 @@ def test_deterministic_behavior(self):
             assert (
                 len(samples) <= 1
             ), "evaluating sampling rules against a span should result in either dropping or not dropping it"
-            sampled = 1 == len(samples)
+            sampled = 1 == len([sample for sample in samples if sample.sampled is True])
             for _ in range(10):
                 other_span = Span(str(i), trace_id=span.trace_id)
                 assert sampled == tracer._sampler.sample(

tests/tracer/test_tracer.py

@@ -1739,7 +1739,9 @@ def test_spans_sampled_out(tracer, test_spans):
             span.sampled = False
 
     spans = test_spans.pop()
-    assert len(spans) == 0
+    assert len(spans) == 3
+    for span in spans:
+        assert span.sampled is False
 
 
 def test_spans_sampled_one(tracer, test_spans):