Skip to content

Commit f47817e

Browse files
vitor-de-araujobrettlangdon
authored andcommitted
chore(ci_visibility): ensure spans are finished by the end of the session (#14066)
At the end of the test session, we finish all unfinished spans recursively by calling `test_session.finish(force=True)`, but `force` only applies if the item's current status is `SPECIAL_STATUS.UNFINISHED`. It looks like in some obscure circumstances, a test can be unfinished without having the `SPECIAL_STATUS.UNFINISHED` status, and in this case the recursive finish will ignore them. This PR changes the logic so that any unfinished span is finished. Also, test spans don't have a parent (they have their own context, separate from the test session/module/suite). This means that when a test span finishes, there is no parent to reactivate, and so there is no active span. This confuses the logic that [checks for unfinished spans](https://github.com/DataDog/dd-trace-py/blob/v3.10.2/ddtrace/_trace/tracer.py#L622) when finish test session/module/suite. This PR reactivates the parent span to avoid this issue. ## 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)
1 parent cddb219 commit f47817e

File tree

1 file changed

+15
-10
lines changed
  • ddtrace/internal/ci_visibility/api

1 file changed

+15
-10
lines changed

ddtrace/internal/ci_visibility/api/_base.py

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,10 @@ def _finish_span(self, override_finish_time: Optional[float] = None) -> None:
235235
self._add_all_tags_to_span()
236236
self._span.finish(finish_time=override_finish_time)
237237

238+
parent_span = self.get_parent_span()
239+
if parent_span:
240+
self._tracer.context_provider.activate(parent_span)
241+
238242
def _set_default_tags(self) -> None:
239243
"""Applies the tags that should be on every span regardless of the item type
240244
@@ -617,18 +621,19 @@ def finish(
617621
# Respect override status no matter what
618622
self.set_status(override_status)
619623

624+
if force:
625+
# Finish all children regardless of their status
626+
for child in self._children.values():
627+
if not child.is_finished():
628+
child.finish(force=force)
629+
self.set_status(self.get_raw_status())
630+
620631
item_status = self.get_status()
621632

622-
if item_status == SPECIAL_STATUS.UNFINISHED:
623-
if force:
624-
# Finish all children regardless of their status
625-
for child in self._children.values():
626-
if not child.is_finished():
627-
child.finish(force=force)
628-
self.set_status(self.get_raw_status())
629-
else:
630-
return
631-
elif not isinstance(item_status, SPECIAL_STATUS):
633+
if item_status == SPECIAL_STATUS.UNFINISHED and not force:
634+
return
635+
636+
if not isinstance(item_status, SPECIAL_STATUS):
632637
self.set_status(item_status)
633638

634639
super().finish(force=force, override_status=override_status, override_finish_time=override_finish_time)

0 commit comments

Comments
 (0)