fix(pytest): ensure pytest module always exist, adjust suite path accordingly [backport 1.17] (#6416)

Backport ae7bab3 from #6368 to 1.17.

CI Visibility: Pytest module should always be reported, although it can
be empty if, for example, tests files are located at the root of the
project.

Taking into account that the Pytest Package may not exist, we need to
use Pytest Module to report the test module span (as with the test
suite).

Also, the test suite path is reported as relative to the test module.

Example of data reported with Flask tests (where there is no Pytest
Package):
Before:
```
{
  "command": "pytest --ddtrace tests",
  "fingerprint": "c3acffec7c31fbcd",
  "framework": "pytest",
  "framework_version": "6.2.4",
  "full_name": "tests/test_testing.py.test_cli_custom_obj",
  "has_parameters": false,
  "name": "test_cli_custom_obj",
  "service": "flask",
  "status": "pass",
  "suite": "tests/test_testing.py",
  "type": "test"
}
```
After:
```
{
  "bundle": "tests",
  "command": "pytest --ddtrace tests",
  "fingerprint": "406b79a08cd4b768",
  "framework": "pytest",
  "framework_version": "6.2.4",
  "full_name": "tests.test_testing.py.test_cli_custom_obj",
  "has_parameters": false,
  "module": "tests",
  "module_path": "tests",
  "name": "test_cli_custom_obj",
  "service": "flask",
  "status": "pass",
  "suite": "test_testing.py",
  "type": "test"
}
```

## 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: Federico Mon <[email protected]>

Author: github-actions[bot]

ddtrace/contrib/pytest/plugin.py

@@ -13,6 +13,7 @@
 """
 from doctest import DocTest
 import json
+import os
 import re
 from typing import Dict
 
@@ -69,6 +70,16 @@ def _store_span(item, span):
     setattr(item, "_datadog_span", span)
 
 
+def _extract_module_span(item):
+    """Extract span from `pytest.Item` instance."""
+    return getattr(item, "_datadog_span_module", None)
+
+
+def _store_module_span(item, span):
+    """Store span at `pytest.Item` instance."""
+    setattr(item, "_datadog_span_module", span)
+
+
 def _mark_failed(item):
     """Store test failed status at `pytest.Item` instance."""
     if item.parent:
@@ -128,26 +139,43 @@ def _get_pytest_command(config):
 
 def _get_module_path(item):
     """Extract module path from a `pytest.Item` instance."""
-    if not isinstance(item, pytest.Package):
+    if not isinstance(item, (pytest.Package, pytest.Module)):
         return None
     return item.nodeid.rpartition("/")[0]
 
 
-def _get_module_name(item):
+def _get_module_name(item, is_package=True):
     """Extract module name (fully qualified) from a `pytest.Item` instance."""
-    return item.module.__name__
+    if is_package:
+        return item.module.__name__
+    return item.nodeid.rpartition("/")[0].replace("/", ".")
 
 
-def _get_suite_name(item):
-    """Extract suite name from a `pytest.Item` instance."""
-    return item.nodeid.rpartition("/")[-1]
+def _get_suite_name(item, test_module_path=None):
+    """
+    Extract suite name from a `pytest.Item` instance.
+    If the module path doesn't exist, the suite path will be reported in full.
+    """
+    if test_module_path:
+        if not item.nodeid.startswith(test_module_path):
+            log.warning("Suite path is not under module path: '%s' '%s'", item.nodeid, test_module_path)
+        suite_path = os.path.relpath(item.nodeid, start=test_module_path)
+        return suite_path
+    return item.nodeid
 
 
-def _start_test_module_span(item):
+def _start_test_module_span(pytest_package_item=None, pytest_module_item=None):
     """
     Starts a test module span at the start of a new pytest test package.
     Note that ``item`` is a ``pytest.Package`` object referencing the test module being run.
     """
+    is_package = True
+    item = pytest_package_item
+
+    if pytest_package_item is None and pytest_module_item is not None:
+        item = pytest_module_item
+        is_package = False
+
     test_session_span = _extract_span(item.session)
     test_module_span = _CIVisibility._instance.tracer._start_span(
         "pytest.test_module",
@@ -164,23 +192,27 @@ def _start_test_module_span(item):
     test_module_span.set_tag_str(_EVENT_TYPE, _MODULE_TYPE)
     test_module_span.set_tag_str(_SESSION_ID, str(test_session_span.span_id))
     test_module_span.set_tag_str(_MODULE_ID, str(test_module_span.span_id))
-    test_module_span.set_tag_str(test.MODULE, _get_module_name(item))
+    test_module_span.set_tag_str(test.MODULE, _get_module_name(item, is_package))
     test_module_span.set_tag_str(test.MODULE_PATH, _get_module_path(item))
-    _store_span(item, test_module_span)
-    return test_module_span
+    if is_package:
+        _store_span(item, test_module_span)
+    else:
+        _store_module_span(item, test_module_span)
+    return test_module_span, is_package
 
 
-def _start_test_suite_span(item):
+def _start_test_suite_span(item, test_module_span):
     """
     Starts a test suite span at the start of a new pytest test module.
     Note that ``item`` is a ``pytest.Module`` object referencing the test file being run.
     """
     test_session_span = _extract_span(item.session)
-    parent_span = test_session_span
-    test_module_span = None
-    if isinstance(item.parent, pytest.Package):
+    if test_module_span is None and isinstance(item.parent, pytest.Package):
         test_module_span = _extract_span(item.parent)
-        parent_span = test_module_span
+
+    parent_span = test_module_span
+    if parent_span is None:
+        parent_span = test_session_span
 
     test_suite_span = _CIVisibility._instance.tracer._start_span(
         "pytest.test_suite",
@@ -197,11 +229,13 @@ def _start_test_suite_span(item):
     test_suite_span.set_tag_str(_EVENT_TYPE, _SUITE_TYPE)
     test_suite_span.set_tag_str(_SESSION_ID, str(test_session_span.span_id))
     test_suite_span.set_tag_str(_SUITE_ID, str(test_suite_span.span_id))
+    test_module_path = None
     if test_module_span is not None:
         test_suite_span.set_tag_str(_MODULE_ID, str(test_module_span.span_id))
         test_suite_span.set_tag_str(test.MODULE, test_module_span.get_tag(test.MODULE))
-        test_suite_span.set_tag_str(test.MODULE_PATH, test_module_span.get_tag(test.MODULE_PATH))
-    test_suite_span.set_tag_str(test.SUITE, _get_suite_name(item))
+        test_module_path = test_module_span.get_tag(test.MODULE_PATH)
+        test_suite_span.set_tag_str(test.MODULE_PATH, test_module_path)
+    test_suite_span.set_tag_str(test.SUITE, _get_suite_name(item, test_module_path))
     _store_span(item, test_suite_span)
     return test_suite_span
 
@@ -338,14 +372,20 @@ def pytest_runtest_protocol(item, nextitem):
         pytest_module_item = _find_pytest_item(item, pytest.Module)
         pytest_package_item = _find_pytest_item(pytest_module_item, pytest.Package)
 
+        module_is_package = True
+
         test_module_span = _extract_span(pytest_package_item)
-        if pytest_package_item is not None and test_module_span is None:
-            if test_module_span is None:
-                test_module_span = _start_test_module_span(pytest_package_item)
+        if not test_module_span:
+            test_module_span = _extract_module_span(pytest_module_item)
+            if test_module_span:
+                module_is_package = False
+
+        if test_module_span is None:
+            test_module_span, module_is_package = _start_test_module_span(pytest_package_item, pytest_module_item)
 
         test_suite_span = _extract_span(pytest_module_item)
         if pytest_module_item is not None and test_suite_span is None:
-            test_suite_span = _start_test_suite_span(pytest_module_item)
+            test_suite_span = _start_test_suite_span(pytest_module_item, test_module_span)
             # Start coverage for the test suite if coverage is enabled
             if _CIVisibility._instance._collect_coverage_enabled:
                 _initialize(str(item.config.rootdir))
@@ -366,20 +406,18 @@ def pytest_runtest_protocol(item, nextitem):
             span.set_tag_str(test.COMMAND, _get_pytest_command(item.config))
             span.set_tag_str(_SESSION_ID, str(test_session_span.span_id))
 
-            if test_module_span is not None:
-                span.set_tag_str(_MODULE_ID, str(test_module_span.span_id))
-                span.set_tag_str(test.MODULE, test_module_span.get_tag(test.MODULE))
-                span.set_tag_str(test.MODULE_PATH, test_module_span.get_tag(test.MODULE_PATH))
-
-            if test_suite_span is not None:
-                span.set_tag_str(_SUITE_ID, str(test_suite_span.span_id))
-                test_class_hierarchy = _get_test_class_hierarchy(item)
-                if test_class_hierarchy:
-                    span.set_tag_str(test.CLASS_HIERARCHY, test_class_hierarchy)
-                if hasattr(item, "dtest") and isinstance(item.dtest, DocTest):
-                    span.set_tag_str(test.SUITE, "{}.py".format(item.dtest.globs["__name__"]))
-                else:
-                    span.set_tag_str(test.SUITE, test_suite_span.get_tag(test.SUITE))
+            span.set_tag_str(_MODULE_ID, str(test_module_span.span_id))
+            span.set_tag_str(test.MODULE, test_module_span.get_tag(test.MODULE))
+            span.set_tag_str(test.MODULE_PATH, test_module_span.get_tag(test.MODULE_PATH))
+
+            span.set_tag_str(_SUITE_ID, str(test_suite_span.span_id))
+            test_class_hierarchy = _get_test_class_hierarchy(item)
+            if test_class_hierarchy:
+                span.set_tag_str(test.CLASS_HIERARCHY, test_class_hierarchy)
+            if hasattr(item, "dtest") and isinstance(item.dtest, DocTest):
+                span.set_tag_str(test.SUITE, "{}.py".format(item.dtest.globs["__name__"]))
+            else:
+                span.set_tag_str(test.SUITE, test_suite_span.get_tag(test.SUITE))
 
             span.set_tag_str(test.TYPE, SpanTypes.TEST)
             span.set_tag_str(test.FRAMEWORK_VERSION, pytest.__version__)
@@ -412,21 +450,25 @@ def pytest_runtest_protocol(item, nextitem):
             yield
 
         nextitem_pytest_module_item = _find_pytest_item(nextitem, pytest.Module)
-        if test_suite_span is not None and (
-            nextitem is None or nextitem_pytest_module_item != pytest_module_item and not test_suite_span.finished
-        ):
+        if nextitem is None or nextitem_pytest_module_item != pytest_module_item and not test_suite_span.finished:
             _mark_test_status(pytest_module_item, test_suite_span)
             # Finish coverage for the test suite if coverage is enabled
             if _CIVisibility._instance._collect_coverage_enabled:
                 _coverage_end(test_suite_span)
             test_suite_span.finish()
 
-        nextitem_pytest_package_item = _find_pytest_item(nextitem, pytest.Package)
-        if test_module_span is not None and (
-            nextitem is None or nextitem_pytest_package_item != pytest_package_item and not test_module_span.finished
-        ):
-            _mark_test_status(pytest_package_item, test_module_span)
-            test_module_span.finish()
+            if not module_is_package:
+                test_module_span.set_tag_str(test.STATUS, test_suite_span.get_tag(test.STATUS))
+                test_module_span.finish()
+            else:
+                nextitem_pytest_package_item = _find_pytest_item(nextitem, pytest.Package)
+                if (
+                    nextitem is None
+                    or nextitem_pytest_package_item != pytest_package_item
+                    and not test_module_span.finished
+                ):
+                    _mark_test_status(pytest_package_item, test_module_span)
+                    test_module_span.finish()
 
 
 @pytest.hookimpl(hookwrapper=True)

releasenotes/notes/fix-pytest-always-create-module-971348889d0cdf36.yaml

@@ -0,0 +1,6 @@
+---
+prelude: >
+    **Breaking change** for CI Visibility: `test.suite` and `test.full_name` are changed, so any visualization or monitor that uses these fields is potentially affected
+fixes:
+  - |
+    pytest: This fix resolves an issue where test modules could be non-existent, causing errors in the CI Visibility product.

tests/ci_visibility/test_encoder.py

@@ -230,6 +230,7 @@ def test_ok():
         expected_meta.update({b"_dd.origin": b"ciapp-test"})
         expected_meta.pop(b"test_session_id")
         expected_meta.pop(b"test_suite_id")
+        expected_meta.pop(b"test_module_id")
         expected_metrics = {
             "{}".format(key).encode("utf-8"): value for key, value in sorted(given_test_span._metrics.items())
         }
@@ -245,6 +246,7 @@ def test_ok():
                 b"service": given_test_span.service.encode("utf-8"),
                 b"span_id": given_test_span.span_id,
                 b"start": given_test_span.start_ns,
+                b"test_module_id": int(given_test_span.get_tag("test_module_id")),
                 b"test_session_id": int(given_test_span.get_tag("test_session_id")),
                 b"test_suite_id": int(given_test_span.get_tag("test_suite_id")),
                 b"trace_id": given_test_span.trace_id,
@@ -275,15 +277,17 @@ def test_ok():
         ci_agentless_encoder.put(spans)
         event_payload = ci_agentless_encoder.encode()
         decoded_event_payload = self.tracer.encoder._decode(event_payload)
-        given_test_suite_span = spans[2]
-        given_test_suite_event = decoded_event_payload[b"events"][2]
+        given_test_suite_span = spans[3]
+        assert given_test_suite_span.get_tag("type") == "test_suite_end"
+        given_test_suite_event = decoded_event_payload[b"events"][3]
         expected_meta = {
             "{}".format(key).encode("utf-8"): "{}".format(value).encode("utf-8")
             for key, value in sorted(given_test_suite_span._meta.items())
         }
         expected_meta.update({b"_dd.origin": b"ciapp-test"})
         expected_meta.pop(b"test_session_id")
         expected_meta.pop(b"test_suite_id")
+        expected_meta.pop(b"test_module_id")
         expected_metrics = {
             "{}".format(key).encode("utf-8"): value for key, value in sorted(given_test_suite_span._metrics.items())
         }
@@ -297,6 +301,7 @@ def test_ok():
                 b"resource": given_test_suite_span.resource.encode("utf-8"),
                 b"service": given_test_suite_span.service.encode("utf-8"),
                 b"start": given_test_suite_span.start_ns,
+                b"test_module_id": int(given_test_suite_span.get_tag("test_module_id")),
                 b"test_session_id": int(given_test_suite_span.get_tag("test_session_id")),
                 b"test_suite_id": int(given_test_suite_span.get_tag("test_suite_id")),
                 b"type": given_test_suite_span.get_tag("type").encode("utf-8"),

tests/contrib/asynctest/test_asynctest.py

@@ -57,6 +57,6 @@ def test_asynctest():
         rec.assertoutcome(passed=1)
         spans = self.pop_spans()
 
-        assert len(spans) == 3
+        assert len(spans) == 4
         test_span = spans[0]
         assert test_span.get_tag(test.STATUS) == test.Status.PASS.value