Prioritize full over partial granularity

* priority +=2 when full granularity and sampled=true
* priority +=1 when partial granularity and sampled=true
* always_on sampler for full granularity sets priority=3 instead of 2

Author: hmstepanek

newrelic/api/transaction.py

@@ -571,6 +571,12 @@ def __exit__(self, exc, value, tb):
             # Sampled and priority need to be computed at the end of the
             # transaction when distributed tracing or span events are enabled.
             self._make_sampling_decision()
+        else:
+            # If dt is disabled, set sampled=False and priority random number between 0 and 1.
+            # The priority of the transaction is used for other data like transaction
+            # events even when span events are not sent.
+            self._priority = float(f"{random.random():.6f}")  # noqa: S311
+            self._sampled = False
 
         self._cached_path._name = self.path
         agent_attributes = self.agent_attributes
@@ -1017,8 +1023,9 @@ def sampling_algo_compute_sampled_and_priority(self, priority, sampled, sampler_
         if sampled is None:
             _logger.debug("No trusted account id found. Sampling decision will be made by adaptive sampling algorithm.")
             sampled = self._application.compute_sampled(**sampler_kwargs)
+            # Increment the priority + 2 for full and + 1 for partial granularity.
             if sampled:
-                priority += 1
+                priority += 1 + int(sampler_kwargs.get("full_granularity"))
         return priority, sampled
 
     def _compute_sampled_and_priority(
@@ -1063,7 +1070,8 @@ def _compute_sampled_and_priority(
             )
         if config == "always_on":
             sampled = True
-            priority = 2.0
+            # priority=3 for full granularity and priority=2 for partial granularity.
+            priority = 2.0 + int(full_granularity)
         elif config == "always_off":
             sampled = False
             priority = 0
@@ -1139,9 +1147,9 @@ def _make_sampling_decision(self):
             return
 
         # This is only reachable if both full and partial granularity tracing are off.
-        # Set priority=0 and do not sample. This enables DT headers to still be sent
-        # even if the trace is never sampled.
-        self._priority = 0
+        # Set priority to random number between 0 and 1 and do not sample. This enables
+        # DT headers to still be sent even if the trace is never sampled.
+        self._priority = float(f"{random.random():.6f}")  # noqa: S311
         self._sampled = False
 
     def _freeze_path(self):

tests/agent_features/test_distributed_tracing.py

@@ -16,6 +16,7 @@
 import copy
 import json
 import time
+import random
 
 import pytest
 import webtest
@@ -25,6 +26,7 @@
 from testing_support.validators.validate_function_not_called import validate_function_not_called
 from testing_support.validators.validate_transaction_event_attributes import validate_transaction_event_attributes
 from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics
+from testing_support.validators.validate_transaction_object_attributes import validate_transaction_object_attributes
 
 from newrelic.api.application import application_instance
 from newrelic.api.function_trace import function_trace
@@ -542,17 +544,17 @@ def _test_inbound_dt_payload_acceptance():
     "newrelic_header,traceparent_sampled,newrelic_sampled,root_setting,remote_parent_sampled_setting,remote_parent_not_sampled_setting,expected_sampled,expected_priority,expected_adaptive_sampling_algo_called",
     (
         (False, None, None, "default", "default", "default", None, None, True),  # Uses adaptive sampling algo.
-        (False, None, None, "always_on", "default", "default", True, 2, False),  # Always sampled.
+        (False, None, None, "always_on", "default", "default", True, 3, False),  # Always sampled.
         (False, None, None, "always_off", "default", "default", False, 0, False),  # Never sampled.
         (True, True, None, "default", "default", "default", None, None, True),  # Uses adaptive sampling algo.
-        (True, True, None, "default", "always_on", "default", True, 2, False),  # Always sampled.
+        (True, True, None, "default", "always_on", "default", True, 3, False),  # Always sampled.
         (True, True, None, "default", "always_off", "default", False, 0, False),  # Never sampled.
         (True, False, None, "default", "default", "default", None, None, True),  # Uses adaptive sampling algo.
         (True, False, None, "default", "always_on", "default", None, None, True),  # Uses adaptive sampling alog.
         (True, False, None, "default", "always_off", "default", None, None, True),  # Uses adaptive sampling algo.
         (True, True, None, "default", "default", "always_on", None, None, True),  # Uses adaptive sampling algo.
         (True, True, None, "default", "default", "always_off", None, None, True),  # Uses adaptive sampling algo.
-        (True, False, None, "default", "default", "always_on", True, 2, False),  # Always sampled.
+        (True, False, None, "default", "default", "always_on", True, 3, False),  # Always sampled.
         (True, False, None, "default", "default", "always_off", False, 0, False),  # Never sampled.
         (
             True,
@@ -587,9 +589,9 @@ def _test_inbound_dt_payload_acceptance():
             1.23456,
             False,
         ),  # Uses sampling decision in W3C TraceState header.
-        (True, True, False, "default", "always_on", "default", True, 2, False),  # Always sampled.
+        (True, True, False, "default", "always_on", "default", True, 3, False),  # Always sampled.
         (True, True, True, "default", "always_off", "default", False, 0, False),  # Never sampled.
-        (True, False, False, "default", "default", "always_on", True, 2, False),  # Always sampled.
+        (True, False, False, "default", "default", "always_on", True, 3, False),  # Always sampled.
         (True, False, True, "default", "default", "always_off", False, 0, False),  # Never sampled.
         (
             True,
@@ -602,7 +604,7 @@ def _test_inbound_dt_payload_acceptance():
             0.1234,
             False,
         ),  # Uses sampling and priority from newrelic header.
-        (True, None, True, "default", "always_on", "default", True, 2, False),  # Always sampled.
+        (True, None, True, "default", "always_on", "default", True, 3, False),  # Always sampled.
         (True, None, True, "default", "always_off", "default", False, 0, False),  # Never sampled.
         (
             True,
@@ -637,7 +639,7 @@ def _test_inbound_dt_payload_acceptance():
             0.1234,
             False,
         ),  # Uses sampling and priority from newrelic header.
-        (True, None, False, "default", "default", "always_on", True, 2, False),  # Always sampled.
+        (True, None, False, "default", "default", "always_on", True, 3, False),  # Always sampled.
         (True, None, False, "default", "default", "always_off", False, 0, False),  # Never sampled.
         (True, None, None, "default", "default", "default", None, None, True),  # Uses adaptive sampling algo.
     ),
@@ -878,8 +880,7 @@ def _test():
     "full_granularity_enabled,full_granularity_remote_parent_sampled_setting,partial_granularity_enabled,partial_granularity_remote_parent_sampled_setting,expected_sampled,expected_priority,expected_adaptive_sampling_algo_called",
     (
         (True, "always_off", True, "adaptive", None, None, True),  # Uses adaptive sampling algo.
-        (True, "always_on", True, "adaptive", True, 2, False),  # Uses adaptive sampling algo.
-        (False, "always_on", False, "adaptive", False, 0, False),  # Uses adaptive sampling algo.
+        (True, "always_on", True, "adaptive", True, 3, False),  # Always samples.
     ),
 )
 def test_distributed_trace_remote_parent_sampling_decision_between_full_and_partial_granularity(
@@ -1394,3 +1395,59 @@ def _test():
         assert application.sampler._samplers[expected_sampling_instance_called].ratio == expected_ratio
 
     _test()
+
+
+@pytest.mark.parametrize(
+    "dt_settings,expected_priority,expected_sampled",
+    (
+        (  # When dt is enabled but full and partial are disabled.
+            {
+                "distributed_tracing.sampler.full_granularity.enabled": False,
+                "distributed_tracing.sampler.partial_granularity.enabled": False,
+            },
+            0.123, # random
+            False,
+        ),
+        (  # When dt is disabled.
+            {
+                "distributed_tracing.enabled": False,
+            },
+            0.123, # random
+            False,
+        ),
+        (  # Verify when full granularity sampled +2 is added to the priority.
+            {
+                "distributed_tracing.sampler.full_granularity.root.trace_id_ratio_based.ratio": 1,
+            },
+            2.123, # random + 2
+            True,
+        ),
+        (  # Verify when partial granularity sampled +1 is added to the priority.
+            {
+                "distributed_tracing.sampler.full_granularity.enabled": False,
+                "distributed_tracing.sampler.partial_granularity.enabled": True,
+                "distributed_tracing.sampler.partial_granularity.root.trace_id_ratio_based.ratio": 1,
+            },
+            1.123, # random + 1
+            True,
+        ),
+    ),
+)
+def test_distributed_trace_enabled_settings_set_correct_sampled_priority(
+    dt_settings,
+    expected_priority,
+    expected_sampled,
+    monkeypatch,
+):
+    monkeypatch.setattr(random, 'random', lambda *args, **kwargs: 0.123)
+
+    test_settings = _override_settings.copy()
+    test_settings.update(dt_settings)
+
+    @override_application_settings(test_settings)
+    @validate_transaction_object_attributes({"sampled": expected_sampled, "priority": expected_priority})
+    @background_task(name="test_distributed_trace_attributes")
+    def _test():
+        pass
+
+    _test()

tests/testing_support/validators/validate_transaction_object_attributes.py

@@ -0,0 +1,38 @@
+# Copyright 2010 New Relic, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from newrelic.common.object_wrapper import transient_function_wrapper
+
+
+def validate_transaction_object_attributes(attributes):
+    # This validates attributes on the transaction object that is passed to
+    # `StatsEngine.record_transaction`.
+    #
+    # Args:
+    #
+    #       attributes: a dictionary of expected attributes (key) and values (value)
+
+    @transient_function_wrapper("newrelic.core.stats_engine", "StatsEngine.record_transaction")
+    def _validate_attributes(wrapped, instance, args, kwargs):
+        def _bind_params(transaction, *args, **kwargs):
+            return transaction
+
+        transaction = _bind_params(*args, **kwargs)
+
+        for attr, value in attributes.items():
+            assert getattr(transaction, attr) == value
+
+        return wrapped(*args, **kwargs)
+
+    return _validate_attributes