fix: dont raise a ValueError on set_user when no span [backport #5548 to 1.12] (#5568)

Backport of #5548 to 1.12

Don't raise a ValueError if there is a call to `set_user` or any of the
other event SDK functions that require a span, without a span, just log
+ regression test.

Fixes #5485

## 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] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [X] Change is maintainable (easy to change, telemetry, documentation).
- [X] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines)
are followed.
- [X] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [X] PR description includes explicit acknowledgement/acceptance of the
performance implications of this PR as reported in the benchmarks PR
comment.

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.

Co-authored-by: Emmett Butler <[email protected]>
Co-authored-by: Tahir H. Butt <[email protected]>
Co-authored-by: Munir Abdinur <[email protected]>

Author: 4 people

ddtrace/appsec/trace_utils.py

@@ -146,6 +146,15 @@ def should_block_user(tracer, userid):  # type: (Tracer, str) -> bool
 
     # Early check to avoid calling the WAF if the request is already blocked
     span = tracer.current_root_span()
+    if not span:
+        log.warning(
+            "No root span in the current execution. should_block_user returning False"
+            "See https://docs.datadoghq.com/security_platform/application_security"
+            "/setup_and_configure/"
+            "?tab=set_user&code-lang=python for more information.",
+        )
+        return False
+
     if _context.get_item(WAF_CONTEXT_NAMES.BLOCKED, span=span):
         return True
 

ddtrace/contrib/trace_utils.py

@@ -631,14 +631,14 @@ def set_user(tracer, user_id, name=None, email=None, scope=None, role=None, sess
             span.set_tag_str(user.ROLE, role)
         if session_id:
             span.set_tag_str(user.SESSION_ID, session_id)
+
+        if config._appsec_enabled:
+            from ddtrace.appsec.trace_utils import block_request_if_user_blocked
+
+            block_request_if_user_blocked(tracer, user_id)
     else:
         log.warning(
             "No root span in the current execution. Skipping set_user tags. "
             "See https://docs.datadoghq.com/security_platform/application_security/setup_and_configure/"
             "?tab=set_user&code-lang=python for more information.",
         )
-
-    if config._appsec_enabled:
-        from ddtrace.appsec.trace_utils import block_request_if_user_blocked
-
-        block_request_if_user_blocked(tracer, user_id)

releasenotes/notes/fix-block-request-without-span-c5ea72a06bd389b3.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    ASM: fix calling `set_user` without a created span raising a `ValueError`.

tests/appsec/test_appsec_trace_utils.py

@@ -1,7 +1,11 @@
+import logging
+
 import pytest
 
 from ddtrace import constants
 from ddtrace.appsec._constants import APPSEC
+from ddtrace.appsec.trace_utils import block_request_if_user_blocked
+from ddtrace.appsec.trace_utils import should_block_user
 from ddtrace.appsec.trace_utils import track_custom_event
 from ddtrace.appsec.trace_utils import track_user_login_failure_event
 from ddtrace.appsec.trace_utils import track_user_login_success_event
@@ -19,7 +23,8 @@ class EventsSDKTestCase(TracerTestCase):
     _BLOCKED_USER = "123456"
 
     @pytest.fixture(autouse=True)
-    def inject_fixtures(self, tracer_appsec):  # noqa: F811
+    def inject_fixtures(self, tracer_appsec, caplog):  # noqa: F811
+        self._caplog = caplog
         self._tracer_appsec = tracer_appsec
 
     def test_track_user_login_event_success_without_metadata(self):
@@ -137,3 +142,20 @@ def test_set_user_blocked(self):
                 assert span.get_tag(user.SCOPE)
                 assert span.get_tag(user.SESSION_ID)
                 assert _context.get_item("http.request.blocked", span=span)
+
+    def test_no_span_doesnt_raise(self):
+        from ddtrace import tracer
+
+        with self._caplog.at_level(logging.DEBUG):
+            try:
+                should_block_user(tracer, "111")
+                block_request_if_user_blocked(tracer, "111")
+                track_custom_event(tracer, "testevent", {})
+                track_user_login_success_event(tracer, "111", {})
+                track_user_login_failure_event(tracer, "111", {})
+                set_user(tracer, "111")
+            except Exception as e:
+                pytest.fail("Should not raise but raised %s" % str(e))
+
+            assert any("No root span" in record.message for record in self._caplog.records)
+            assert any(record.levelno == logging.WARNING for record in self._caplog.records)