Skip to content

Commit 54480da

Browse files
fix(asm): insert user information in span [backport 1.18] (#6918)
Backport 85fa4bb from #6616 to 1.18. Fixes #6551 Make it posible to choose user information span, by default it was root_span but sometimes this root_span is not the one where user information should be set. See issue for more details. ## 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/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [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)) ## 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. - [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: Pedro Vicente <[email protected]>
1 parent 87e134c commit 54480da

File tree

5 files changed

+100
-21
lines changed

5 files changed

+100
-21
lines changed

ddtrace/appsec/trace_utils.py

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,19 @@
2424

2525

2626
def _track_user_login_common(
27-
tracer, success, metadata=None, login_events_mode=LOGIN_EVENTS_MODE.SDK, login=None, name=None, email=None
27+
tracer, # type: Tracer
28+
success, # type: bool
29+
metadata=None, # type: Optional[dict]
30+
login_events_mode=LOGIN_EVENTS_MODE.SDK, # type: str
31+
login=None, # type: Optional[str]
32+
name=None, # type: Optional[str]
33+
email=None, # type: Optional[str]
34+
span=None, # type: Optional[Span]
2835
):
29-
# type: (Tracer, bool, Optional[dict], str, Optional[str], Optional[str], Optional[str]) -> Optional[Span]
36+
# type: (...) -> Optional[Span]
3037

31-
span = tracer.current_root_span()
38+
if span is None:
39+
span = tracer.current_root_span()
3240
if span:
3341
success_str = "success" if success else "failure"
3442
tag_prefix = "%s.%s" % (APPSEC.USER_LOGIN_EVENT_PREFIX, success_str)
@@ -65,19 +73,20 @@ def _track_user_login_common(
6573

6674

6775
def track_user_login_success_event(
68-
tracer,
69-
user_id,
70-
metadata=None,
71-
login=None,
72-
name=None,
73-
email=None,
74-
scope=None,
75-
role=None,
76-
session_id=None,
77-
propagate=False,
78-
login_events_mode=LOGIN_EVENTS_MODE.SDK,
76+
tracer, # type: Tracer
77+
user_id, # type: str
78+
metadata=None, # type: Optional[dict]
79+
login=None, # type: Optional[str]
80+
name=None, # type: Optional[str]
81+
email=None, # type: Optional[str]
82+
scope=None, # type: Optional[str]
83+
role=None, # type: Optional[str]
84+
session_id=None, # type: Optional[str]
85+
propagate=False, # type: bool
86+
login_events_mode=LOGIN_EVENTS_MODE.SDK, # type: str
87+
span=None, # type: Optional[Span]
7988
):
80-
# type: (Tracer, str, Optional[dict], Optional[str], Optional[str], Optional[str], Optional[str], Optional[str], Optional[str], bool, str) -> None # noqa: E501
89+
# type: (...) -> None # noqa: E501
8190
"""
8291
Add a new login success tracking event. The parameters after metadata (name, email,
8392
scope, role, session_id, propagate) will be passed to the `set_user` function that will be called
@@ -90,12 +99,12 @@ def track_user_login_success_event(
9099
:param metadata: a dictionary with additional metadata information to be stored with the event
91100
"""
92101

93-
span = _track_user_login_common(tracer, True, metadata, login_events_mode, login, name, email)
102+
span = _track_user_login_common(tracer, True, metadata, login_events_mode, login, name, email, span)
94103
if not span:
95104
return
96105

97106
# usr.id will be set by set_user
98-
set_user(tracer, user_id, name, email, scope, role, session_id, propagate)
107+
set_user(tracer, user_id, name, email, scope, role, session_id, propagate, span)
99108

100109

101110
def track_user_login_failure_event(tracer, user_id, exists, metadata=None, login_events_mode=LOGIN_EVENTS_MODE.SDK):

ddtrace/contrib/trace_utils.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -622,14 +622,24 @@ def set_flattened_tags(
622622
span.set_tag(tag, processor(v) if processor is not None else v)
623623

624624

625-
def set_user(tracer, user_id, name=None, email=None, scope=None, role=None, session_id=None, propagate=False):
626-
# type: (Tracer, str, Optional[str], Optional[str], Optional[str], Optional[str], Optional[str], bool) -> None
625+
def set_user(
626+
tracer, # type: Tracer
627+
user_id, # type: str
628+
name=None, # type: Optional[str]
629+
email=None, # type: Optional[str]
630+
scope=None, # type: Optional[str]
631+
role=None, # type: Optional[str]
632+
session_id=None, # type: Optional[str]
633+
propagate=False, # type bool
634+
span=None, # type: Optional[Span]
635+
):
636+
# type: (...) -> None
627637
"""Set user tags.
628638
https://docs.datadoghq.com/logs/log_configuration/attributes_naming_convention/#user-related-attributes
629639
https://docs.datadoghq.com/security_platform/application_security/setup_and_configure/?tab=set_tag&code-lang=python
630640
"""
631-
632-
span = tracer.current_root_span()
641+
if span is None:
642+
span = tracer.current_root_span()
633643
if span:
634644
# Required unique identifier of the user
635645
str_user_id = str(user_id)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
ASM: This fix resolves issue where user information was only set in root span.
5+
Now span for user information can be selected.

tests/appsec/test_appsec_trace_utils.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,39 @@ def test_track_user_login_event_success_without_metadata(self):
6060
assert root_span.get_tag(user.ROLE) == "boss"
6161
assert root_span.get_tag(user.SESSION_ID) == "test_session_id"
6262

63+
def test_track_user_login_event_success_in_span_without_metadata(self):
64+
with self.trace("test_success1") as parent_span:
65+
user_span = self.trace("user_span")
66+
user_span.parent_id = parent_span.span_id
67+
track_user_login_success_event(
68+
self.tracer,
69+
"1234",
70+
metadata=None,
71+
name="John",
72+
73+
scope="test_scope",
74+
role="boss",
75+
session_id="test_session_id",
76+
span=user_span,
77+
)
78+
79+
success_prefix = "%s.success" % APPSEC.USER_LOGIN_EVENT_PREFIX
80+
failure_prefix = "%s.failure" % APPSEC.USER_LOGIN_EVENT_PREFIX
81+
82+
assert user_span.get_tag("%s.track" % success_prefix) == "true"
83+
assert user_span.get_tag("%s.sdk" % success_prefix) == "true"
84+
assert not user_span.get_tag("%s.auto.mode" % success_prefix)
85+
assert not user_span.get_tag("%s.track" % failure_prefix)
86+
# set_user tags
87+
assert user_span.get_tag(user.ID) == "1234" and parent_span.get_tag(user.ID) is None
88+
assert user_span.get_tag(user.NAME) == "John" and parent_span.get_tag(user.NAME) is None
89+
assert user_span.get_tag(user.EMAIL) == "[email protected]" and parent_span.get_tag(user.EMAIL) is None
90+
assert user_span.get_tag(user.SCOPE) == "test_scope" and parent_span.get_tag(user.SCOPE) is None
91+
assert user_span.get_tag(user.ROLE) == "boss" and parent_span.get_tag(user.ROLE) is None
92+
assert (
93+
user_span.get_tag(user.SESSION_ID) == "test_session_id" and parent_span.get_tag(user.SESSION_ID) is None
94+
)
95+
6396
def test_track_user_login_event_success_auto_mode_safe(self):
6497
with self.trace("test_success1"):
6598
track_user_login_success_event(

tests/tracer/test_tracer.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,28 @@ def test_tracer_set_user(self):
532532
assert span.get_tag(user.SCOPE)
533533
assert span.context.dd_user_id is None
534534

535+
def test_tracer_set_user_in_span(self):
536+
parent_span = self.trace("root_span")
537+
user_span = self.trace("user_span")
538+
user_span.parent_id = parent_span.span_id
539+
set_user(
540+
self.tracer,
541+
user_id="usr.id",
542+
email="usr.email",
543+
name="usr.name",
544+
session_id="usr.session_id",
545+
role="usr.role",
546+
scope="usr.scope",
547+
span=user_span,
548+
)
549+
assert user_span.get_tag(user.ID) and parent_span.get_tag(user.ID) is None
550+
assert user_span.get_tag(user.EMAIL) and parent_span.get_tag(user.EMAIL) is None
551+
assert user_span.get_tag(user.SESSION_ID) and parent_span.get_tag(user.SESSION_ID) is None
552+
assert user_span.get_tag(user.NAME) and parent_span.get_tag(user.NAME) is None
553+
assert user_span.get_tag(user.ROLE) and parent_span.get_tag(user.ROLE) is None
554+
assert user_span.get_tag(user.SCOPE) and parent_span.get_tag(user.SCOPE) is None
555+
assert user_span.context.dd_user_id is None
556+
535557
def test_tracer_set_user_mandatory(self):
536558
span = self.trace("fake_span")
537559
set_user(

0 commit comments

Comments
 (0)