Skip to content

Commit 48617d5

Browse files
fix(ci_visibility): xdist coverage upload with proper session id [backport 3.13] (#14551)
Backport 7e67963 from #14478 to 3.13. CI Visibility: This fixes an issue where the coverage data from pytest-xdist sessions were not using the proper session id. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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: Federico Mon <[email protected]>
1 parent 0eb8734 commit 48617d5

File tree

3 files changed

+83
-2
lines changed

3 files changed

+83
-2
lines changed

ddtrace/internal/ci_visibility/encoder.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,11 @@ def put(self, item):
264264
for span in item
265265
if COVERAGE_TAG_NAME in span.get_tags() or span.get_struct_tag(COVERAGE_TAG_NAME) is not None
266266
]
267+
# Also include session span for parent session ID lookup, even if it doesn't have coverage data
268+
session_span = next((span for span in item if span.get_tag(EVENT_TYPE) == SESSION_TYPE), None)
269+
if session_span and session_span not in spans_with_coverage:
270+
spans_with_coverage.append(session_span)
271+
267272
if not spans_with_coverage:
268273
raise NoEncodableSpansError()
269274
return super(CIVisibilityCoverageEncoderV02, self).put(spans_with_coverage)
@@ -294,8 +299,9 @@ def _build_body(self, data: bytes) -> List[bytes]:
294299
)
295300

296301
def _build_data(self, traces: List[List[Span]]) -> Optional[bytes]:
302+
new_parent_session_span_id = self._get_parent_session(traces)
297303
normalized_covs = [
298-
self._convert_span(span)
304+
self._convert_span(span, new_parent_session_span_id=new_parent_session_span_id)
299305
for trace in traces
300306
for span in trace
301307
if (COVERAGE_TAG_NAME in span.get_tags() or span.get_struct_tag(COVERAGE_TAG_NAME) is not None)
@@ -325,7 +331,7 @@ def _convert_span(
325331
files = json.loads(str(span.get_tag(COVERAGE_TAG_NAME)))["files"]
326332

327333
converted_span = {
328-
"test_session_id": int(span.get_tag(SESSION_ID) or "1"),
334+
"test_session_id": new_parent_session_span_id or int(span.get_tag(SESSION_ID) or "1"),
329335
"test_suite_id": int(span.get_tag(SUITE_ID) or "1"),
330336
"files": files,
331337
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
CI Visibility: This fix resolves an issue where coverage from sessions with pytest-xdist were not submitted with the proper session id, preventing Test Impact Analysis feature from working properly.

tests/ci_visibility/test_encoder.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,3 +888,74 @@ def test_full_encoding_with_parent_session_override():
888888
# Both should use the parent session ID (0xAAAAAA) instead of worker session ID
889889
assert test_event[b"content"][b"test_session_id"] == 0xAAAAAA
890890
assert session_event[b"content"][b"test_session_id"] == 0xAAAAAA
891+
892+
893+
def test_coverage_encoder_includes_session_span():
894+
"""Test that coverage encoder includes session span even if it doesn't have coverage data"""
895+
# Create session span without coverage data
896+
session_span = Span(name="test.session", span_id=0xAAAAAA, service="test")
897+
session_span.set_tag(EVENT_TYPE, SESSION_TYPE)
898+
session_span.set_tag(SESSION_ID, "12345")
899+
session_span.parent_id = 0x999999 # Has parent ID for xdist
900+
901+
# Create test span with coverage data
902+
coverage_data = {"files": [{"filename": "test.py", "segments": [[1, 0, 1, 0, -1]]}]}
903+
test_span = Span(name="test.case", span_id=0xBBBBBB, service="test", span_type="test")
904+
test_span.set_tag(COVERAGE_TAG_NAME, json.dumps(coverage_data))
905+
test_span.set_tag(SESSION_ID, "12345")
906+
test_span.set_tag(SUITE_ID, "67890")
907+
908+
# Create span without coverage data (should be filtered out)
909+
regular_span = Span(name="regular.span", span_id=0xCCCCCC, service="test")
910+
911+
trace = [session_span, test_span, regular_span]
912+
913+
encoder = CIVisibilityCoverageEncoderV02(0, 0)
914+
encoder.put(trace)
915+
916+
# Check what got stored in the buffer
917+
assert len(encoder.buffer) == 1
918+
stored_trace = encoder.buffer[0]
919+
920+
# Should include session span and coverage span, but not regular span
921+
assert len(stored_trace) == 2
922+
span_names = {span.name for span in stored_trace}
923+
assert "test.session" in span_names
924+
assert "test.case" in span_names
925+
assert "regular.span" not in span_names
926+
927+
928+
def test_coverage_encoder_parent_session_id_propagation():
929+
"""Test that coverage encoder properly uses parent session ID from session span"""
930+
# Create session span with parent_id (simulating xdist worker)
931+
session_span = Span(name="worker.session", span_id=0xBBBBBB, service="test")
932+
session_span.set_tag(EVENT_TYPE, SESSION_TYPE)
933+
session_span.set_tag(SESSION_ID, "123")
934+
session_span.parent_id = 0xAAAAAA # Parent session ID from main process
935+
936+
# Create test span with coverage data
937+
coverage_data = {"files": [{"filename": "test.py", "segments": [[1, 0, 1, 0, -1]]}]}
938+
test_span = Span(name="test.case", span_id=0xCCCCCC, service="test", span_type="test")
939+
test_span.set_tag(COVERAGE_TAG_NAME, json.dumps(coverage_data))
940+
test_span.set_tag(SESSION_ID, "123")
941+
test_span.set_tag(SUITE_ID, "67890")
942+
943+
traces = [[session_span, test_span]]
944+
945+
encoder = CIVisibilityCoverageEncoderV02(0, 0)
946+
encoder.put(traces[0])
947+
948+
# Build coverage data
949+
coverage_data_bytes = encoder._build_data(traces)
950+
assert coverage_data_bytes is not None
951+
952+
# Decode the coverage data
953+
decoded = msgpack.unpackb(coverage_data_bytes, raw=True, strict_map_key=False)
954+
coverages = decoded[b"coverages"]
955+
956+
assert len(coverages) == 1
957+
coverage_event = coverages[0]
958+
959+
# Should use parent session ID (0xAAAAAA) instead of worker session ID
960+
assert coverage_event[b"test_session_id"] == 0xAAAAAA
961+
assert coverage_event[b"test_suite_id"] == 67890

0 commit comments

Comments
 (0)