fix(api): log comp report output dir failures with exc_info (#12142)

Co-authored-by: Alan Buscaglia <gentlemanprogramming@gmail.com>

api/changelog.d/report-output-dir-error-grouping.fixed.md

@@ -0,0 +1 @@
+Compliance report output directory failures are now logged with the exception attached and fingerprinted by `errno` in Sentry, so `ENOSPC`, `ENOENT` and `EACCES` no longer share a single issue

api/src/backend/api/tests/test_sentry.py

@@ -1,9 +1,10 @@
+import errno
 import logging
 from unittest.mock import MagicMock, patch
 
 import pytest
 from config.settings import sentry as sentry_settings
-from config.settings.sentry import before_send
+from config.settings.sentry import before_send, errno_fingerprint
 
 
 def test_initialize_sentry_skips_without_dsn():
@@ -188,3 +189,165 @@ def test_before_send_passes_non_defunct_neo4j_log():
     event = MagicMock()
 
     assert before_send(event, hint) == event
+
+
+def _filesystem_hint(exception, msg="Error generating output directory"):
+    """Build the hint the logging integration sends for a filesystem failure."""
+    exc_info = (type(exception), exception, exception.__traceback__)
+    log_record = _make_log_record(msg)
+    log_record.exc_info = exc_info
+    setattr(
+        log_record,
+        sentry_settings.ERROR_CATEGORY_ATTRIBUTE,
+        sentry_settings.FILESYSTEM_ERROR_CATEGORY,
+    )
+    return {"log_record": log_record, "exc_info": exc_info}
+
+
+@pytest.mark.parametrize(
+    ("error_number", "message", "expected_suffix"),
+    [
+        (errno.ENOSPC, "No space left on device", "errno:ENOSPC"),
+        (errno.ENOENT, "No such file or directory", "errno:ENOENT"),
+        (errno.EACCES, "Permission denied", "errno:EACCES"),
+    ],
+)
+def test_before_send_fingerprints_oserror_by_errno(
+    error_number, message, expected_suffix
+):
+    """Filesystem failures raised from the same call site must not be merged."""
+    event = {}
+
+    result = before_send(event, _filesystem_hint(OSError(error_number, message)))
+
+    assert result is event
+    assert event["fingerprint"] == ["{{ default }}", expected_suffix]
+
+
+def test_before_send_fingerprints_differ_per_errno():
+    """ENOSPC and ENOENT from the same call site produce different issues."""
+    enospc_event = {}
+    enoent_event = {}
+
+    before_send(
+        enospc_event, _filesystem_hint(OSError(errno.ENOSPC, "No space left on device"))
+    )
+    before_send(
+        enoent_event,
+        _filesystem_hint(OSError(errno.ENOENT, "No such file or directory")),
+    )
+
+    assert enospc_event["fingerprint"] != enoent_event["fingerprint"]
+
+
+def test_before_send_fingerprints_wrapped_oserror():
+    """The errno is found even when the OSError is wrapped by another error."""
+    try:
+        try:
+            raise OSError(errno.ENOSPC, "No space left on device")
+        except OSError as os_error:
+            raise RuntimeError("Error generating output directory") from os_error
+    except RuntimeError as wrapper:
+        event = {}
+        before_send(event, _filesystem_hint(wrapper))
+
+    assert event["fingerprint"] == ["{{ default }}", "errno:ENOSPC"]
+
+
+def test_before_send_does_not_fingerprint_non_oserror():
+    """Non-filesystem exceptions keep Sentry's default grouping."""
+    event = {}
+
+    result = before_send(event, _filesystem_hint(ValueError("boom")))
+
+    assert result is event
+    assert "fingerprint" not in event
+
+
+def test_before_send_does_not_fingerprint_unrelated_oserror_log():
+    """Only records declaring the filesystem category opt into the errno grouping."""
+    exception = OSError(errno.ENOSPC, "No space left on device")
+    log_record = _make_log_record("Unrelated failure")
+    exc_info = (OSError, exception, None)
+    log_record.exc_info = exc_info
+    event = {}
+
+    result = before_send(event, {"log_record": log_record, "exc_info": exc_info})
+
+    assert result is event
+    assert "fingerprint" not in event
+
+
+def test_before_send_does_not_fingerprint_exception_events():
+    """Exception events without a log record keep Sentry's default grouping."""
+    event = {}
+
+    result = before_send(
+        event,
+        {"exc_info": (OSError, OSError(errno.ENOSPC, "No space left on device"), None)},
+    )
+
+    assert result is event
+    assert "fingerprint" not in event
+
+
+@pytest.mark.parametrize("fingerprint", [["scope-fingerprint"], []])
+def test_before_send_keeps_existing_fingerprint(fingerprint):
+    """A fingerprint set by a scope or an integration is never overwritten."""
+    expected_fingerprint = fingerprint.copy()
+    event = {"fingerprint": fingerprint}
+
+    before_send(
+        event, _filesystem_hint(OSError(errno.ENOSPC, "No space left on device"))
+    )
+
+    assert event["fingerprint"] == expected_fingerprint
+
+
+def test_before_send_ignores_suppressed_context():
+    """`raise ... from None` hides the context, so it must not group the event."""
+    try:
+        try:
+            raise OSError(errno.ENOSPC, "No space left on device")
+        except OSError:
+            raise RuntimeError("Error generating output directory") from None
+    except RuntimeError as wrapper:
+        event = {}
+        before_send(event, _filesystem_hint(wrapper))
+
+    assert "fingerprint" not in event
+
+
+def test_errno_fingerprint_follows_implicit_context():
+    """An implicit `raise` during handling still exposes the original errno."""
+    try:
+        try:
+            raise OSError(errno.EACCES, "Permission denied")
+        except OSError:
+            raise RuntimeError("Error generating output directory")
+    except RuntimeError as wrapper:
+        assert errno_fingerprint(wrapper) == "errno:EACCES"
+
+
+def test_before_send_does_not_fingerprint_oserror_without_errno():
+    """An OSError without errno has nothing to split the issue by."""
+    event = {}
+
+    before_send(event, _filesystem_hint(OSError("no errno here")))
+
+    assert "fingerprint" not in event
+
+
+def test_errno_fingerprint_uses_raw_number_for_unknown_errno():
+    """Unmapped errno values still split the issue instead of being dropped."""
+    assert errno_fingerprint(OSError(9999, "unknown")) == "errno:9999"
+
+
+def test_errno_fingerprint_stops_on_self_referencing_chain():
+    """A cyclic exception chain must not hang the fingerprint lookup."""
+    first = ValueError("first")
+    second = ValueError("second")
+    first.__cause__ = second
+    second.__cause__ = first
+
+    assert errno_fingerprint(first) is None

api/src/backend/config/settings/sentry.py

@@ -1,6 +1,20 @@
+from errno import errorcode
+
 import sentry_sdk
 from config.env import env
 
+# How many links of the __cause__/__context__ chain are inspected when looking
+# for the OSError that actually caused the event.
+MAX_EXCEPTION_CHAIN_DEPTH = 10
+
+# LogRecord attribute describing what kind of failure the record reports, set by
+# the caller through `logger.exception(..., extra={"error_category": ...})`.
+ERROR_CATEGORY_ATTRIBUTE = "error_category"
+
+# Category of records whose events are grouped by the errno of the underlying
+# OSError. Only records that declare it opt into the errno fingerprint.
+FILESYSTEM_ERROR_CATEGORY = "filesystem"
+
 IGNORED_EXCEPTIONS = [
     # Provider is not connected due to credentials errors
     "is not connected",
@@ -80,6 +94,38 @@ IGNORED_EXCEPTIONS = [
 ]
 
 
+def errno_fingerprint(exception):
+    """
+    Return an errno-based fingerprint suffix for OSError-like exceptions.
+
+    Filesystem failures such as ENOSPC (disk full), ENOENT (missing mount point)
+    or EACCES (wrong permissions) are all OSError raised from the same call
+    site, so Sentry's default grouping merges them into a single issue even
+    when the exception is attached to the event. Appending the errno keeps the
+    default grouping and splits the issue per failure cause.
+
+    Only the part of the chain Sentry itself displays is inspected: a
+    `raise ... from None` sets __suppress_context__, so the implicit
+    __context__ is dropped from the event and must not group it either.
+
+    Returns None when no OSError with an errno is found in the exception chain.
+    """
+    seen = set()
+    for _ in range(MAX_EXCEPTION_CHAIN_DEPTH):
+        if exception is None or id(exception) in seen:
+            break
+        seen.add(id(exception))
+        if isinstance(exception, OSError) and exception.errno is not None:
+            return f"errno:{errorcode.get(exception.errno, exception.errno)}"
+        if exception.__cause__ is not None:
+            exception = exception.__cause__
+        elif exception.__suppress_context__:
+            break
+        else:
+            exception = exception.__context__
+    return None
+
+
 def before_send(event, hint):
     """
     before_send handles the Sentry events in order to send them or not
@@ -115,10 +161,28 @@ def before_send(event, hint):
 
     # Ignore exceptions with the ignored_exceptions
     if "exc_info" in hint and hint["exc_info"]:
-        exc_value = str(hint["exc_info"][1])
+        exception = hint["exc_info"][1]
+        exc_value = str(exception)
         if any(ignored in exc_value for ignored in IGNORED_EXCEPTIONS):
             return None  # Explicitly return None to drop the event
 
+    # Split filesystem issues per errno instead of grouping every failure raised
+    # from the same call site under a single issue. Only records that declare
+    # themselves as filesystem failures opt in, and a fingerprint already set by
+    # a scope or an integration always wins.
+    log_record = hint.get("log_record")
+    exc_info = hint.get("exc_info")
+    if (
+        log_record is not None
+        and exc_info
+        and getattr(log_record, ERROR_CATEGORY_ATTRIBUTE, None)
+        == FILESYSTEM_ERROR_CATEGORY
+        and "fingerprint" not in event
+    ):
+        fingerprint_suffix = errno_fingerprint(exc_info[1])
+        if fingerprint_suffix:
+            event["fingerprint"] = ["{{ default }}", fingerprint_suffix]
+
     return event
 
 

api/src/backend/tasks/jobs/report.py

@@ -13,6 +13,7 @@ from api.db_utils import rls_transaction
 from api.models import Provider, Scan, ScanSummary, StateChoices, ThreatScoreSnapshot
 from celery.utils.log import get_task_logger
 from config.django.base import DJANGO_TMP_OUTPUT_DIRECTORY
+from config.settings.sentry import ERROR_CATEGORY_ATTRIBUTE, FILESYSTEM_ERROR_CATEGORY
 from prowler.lib.check.compliance_models import (
     Compliance,
     get_bulk_compliance_frameworks_universal,
@@ -960,7 +961,15 @@ def generate_compliance_reports(
             first_output_path = next(iter(output_paths.values()))
             out_dir = str(Path(first_output_path).parent.parent)
     except Exception as e:
-        logger.error("Error generating output directory: %s", e)
+        # logger.exception attaches the exception (and its traceback) to the
+        # Sentry event and the filesystem category opts that event into the
+        # errno fingerprint, so ENOSPC, ENOENT and EACCES raised from this same
+        # call site land on separate issues.
+        logger.exception(
+            "Error generating output directory: %s",
+            e,
+            extra={ERROR_CATEGORY_ATTRIBUTE: FILESYSTEM_ERROR_CATEGORY},
+        )
         error_dict = {"error": str(e), "upload": False, "path": ""}
         if generate_threatscore:
             results["threatscore"] = error_dict.copy()

api/src/backend/tasks/tests/test_reports.py

@@ -1,3 +1,5 @@
+import errno
+import logging
 import os
 import time
 import uuid
@@ -14,6 +16,11 @@ from api.models import (
     StateChoices,
     StatusChoices,
 )
+from config.settings.sentry import (
+    ERROR_CATEGORY_ATTRIBUTE,
+    FILESYSTEM_ERROR_CATEGORY,
+    before_send,
+)
 from prowler.lib.check.models import Severity
 from reportlab.lib import colors
 from tasks.jobs.report import (
@@ -1676,6 +1683,78 @@ class TestGenerateComplianceReportsCIS:
         assert result["cis"]["upload"] is False
         assert result["cis"]["error"] == "dir boom"
 
+    @patch("tasks.jobs.report._aggregate_requirement_statistics_from_database")
+    @patch("tasks.jobs.report._generate_compliance_output_directory")
+    @patch("tasks.jobs.report.Compliance.get_bulk")
+    def test_output_directory_failures_are_grouped_per_errno(
+        self,
+        mock_get_bulk,
+        mock_generate_output_dir,
+        mock_stats,
+        monkeypatch,
+        caplog,
+        tenants_fixture,
+        scans_fixture,
+        aws_provider,
+    ):
+        """A full disk and a missing mount point must not share a Sentry issue.
+
+        Both are OSError raised from the same ``os.makedirs`` call, so they only
+        stay apart if the exception reaches ``before_send``, which fingerprints
+        it by errno.
+        """
+        tenant = tenants_fixture[0]
+        scan = scans_fixture[0]
+        provider = aws_provider
+
+        self._force_scan_has_findings(monkeypatch)
+        mock_stats.return_value = {}
+        mock_get_bulk.return_value = {"cis_5.0_aws": Mock()}
+
+        fingerprints = []
+        for error_number, message in (
+            (errno.ENOSPC, "No space left on device: '/tmp/prowler_api_output'"),
+            (errno.ENOENT, "No such file or directory: '/mnt/output'"),
+        ):
+            mock_generate_output_dir.side_effect = OSError(error_number, message)
+            caplog.clear()
+
+            with caplog.at_level(logging.ERROR, logger="tasks.jobs.report"):
+                generate_compliance_reports(
+                    tenant_id=str(tenant.id),
+                    scan_id=str(scan.id),
+                    provider_id=str(provider.id),
+                    generate_threatscore=False,
+                    generate_ens=False,
+                    generate_nis2=False,
+                    generate_csa=False,
+                    generate_cis=True,
+                )
+
+            record = next(
+                record
+                for record in caplog.records
+                if "Error generating output directory" in record.getMessage()
+            )
+            # Without exc_info the Sentry event carries no exception at all and
+            # nothing can tell the two failures apart.
+            assert record.exc_info is not None
+            # The category is what scopes the errno fingerprint to this record.
+            assert (
+                getattr(record, ERROR_CATEGORY_ATTRIBUTE, None)
+                == FILESYSTEM_ERROR_CATEGORY
+            )
+
+            # Same hint the Sentry logging integration builds for this record.
+            event = {}
+            before_send(event, {"log_record": record, "exc_info": record.exc_info})
+            fingerprints.append(event["fingerprint"])
+
+        assert fingerprints == [
+            ["{{ default }}", "errno:ENOSPC"],
+            ["{{ default }}", "errno:ENOENT"],
+        ]
+
 
 class TestPickLatestCisVariant:
     """Unit tests for `_pick_latest_cis_variant` helper."""