Fingerprint telemetry exceptions by function names, not module paths

.github/scripts/telemetry_error_issues/normalization.py

@@ -8,7 +8,7 @@
 from __future__ import annotations
 
 import re
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, NamedTuple
 
 if TYPE_CHECKING:
     from collections.abc import Iterable
@@ -85,28 +85,54 @@ def normalize_path(path: str) -> str:
     return "/".join(path.split("/")[-FALLBACK_PATH_COMPONENTS:])
 
 
-def frame_key(frame: str) -> str | None:
-    """Return ``path:function`` for one ``FRAME_PATTERN`` match, without its line."""
+class FrameLocation(NamedTuple):
+    """The source file and function of one stack frame."""
+
+    path: str
+    function: str
+
+
+def frame_location(frame: str) -> FrameLocation | None:
+    """Return the normalized path and the function of one ``FRAME_PATTERN`` match.
+
+    The frame's line number is dropped.
+    """
     parts = FRAME_PARTS.match(frame)
     if parts is None:
         return None
-    return f"{normalize_path(parts['path'])}:{parts['function']}"
+    return FrameLocation(normalize_path(parts["path"]), parts["function"])
+
+
+def function_key(function: str) -> str:
+    """Return a function name without its leading underscores."""
+    return function.lstrip("_")
 
 
 def fingerprint_frames(frames: Iterable[str] | None) -> tuple[str, ...]:
-    """Choose the frames that identify where an exception was raised.
+    """Choose the frames that identify where an exception was raised, and key them.
 
     ``frames`` come from the first traceback of the stack only (see
     ``CHAINED_EXCEPTION_PATTERN``); for a chained exception that is its first
-    cause. These are the three innermost frames in application packages.
-    Without application frames they are the three innermost frames of any
-    kind, and without a stack, a placeholder.
+    cause. These are the three innermost frames whose paths are in
+    application packages. Without application frames they are the three
+    innermost frames of any kind, and without a stack, a placeholder.
+
+    Each chosen frame is keyed by its function name alone, without leading
+    underscores, so moving a function to another module, or making a private
+    helper public, keeps the fingerprint. Any other rename changes it.
     """
-    keys = [key for key in map(frame_key, frames or ()) if key is not None]
-    if not keys:
+    locations = [
+        location
+        for location in map(frame_location, frames or ())
+        if location is not None
+    ]
+    if not locations:
         return (MISSING,)
-    application_keys = [key for key in keys if APPLICATION_PATH.match(key)]
-    return tuple((application_keys or keys)[-FINGERPRINT_FRAME_COUNT:])
+    application_locations = [
+        location for location in locations if APPLICATION_PATH.match(location.path)
+    ]
+    chosen = (application_locations or locations)[-FINGERPRINT_FRAME_COUNT:]
+    return tuple(function_key(location.function) for location in chosen)
 
 
 def _validated(value: str | None, pattern: re.Pattern[str], max_length: int) -> str:

.github/scripts/telemetry_error_issues/records.py

@@ -20,10 +20,19 @@
     from datetime import datetime
 
 REQUEST_COMPLETED_EVENT = "request.completed"
-# Changing how fingerprints are computed changes every fingerprint, and the
-# job matches existing issues only by the current algorithm. A change to it
-# must say how existing issues are carried over.
-FINGERPRINT_ALGORITHM = 1
+# Changing how fingerprints are computed changes every fingerprint. The job
+# matches an issue by the fingerprint in its state comment only when the state
+# names the current algorithm, and otherwise by the fingerprint in its title,
+# which it never changes. A change to the algorithm must say how existing
+# issues are carried over.
+#
+# Algorithm 2 keys an exception's stack frames by function name, where
+# algorithm 1 used module path and function name. The algorithm number is
+# hashed into every fingerprint, so every group's fingerprint changed. Issues
+# opened under algorithm 1 were not carried over: their titles hold algorithm 1
+# fingerprints, so the job no longer updates or reopens them, and a group that
+# still fails opens a new issue.
+FINGERPRINT_ALGORITHM = 2
 FINGERPRINT_LENGTH = 16
 MIN_FINGERPRINT_KEY_LENGTH = 32
 
@@ -148,7 +157,8 @@ def fingerprint_parts(record: ErrorRecord) -> tuple[str, ...]:
     """Return the kind and the values that identify the record's error group.
 
     An exception's route is left out, so one shared failure doesn't open an
-    issue for every route that reaches it.
+    issue for every route that reaches it. Its stack frames are keyed by
+    function name, not module path (see ``fingerprint_frames``).
     """
     kind = record.kind
     if kind == "exception":

changelog.d/telemetry-fingerprint-function-names.changed.md

@@ -0,0 +1 @@
+The daily telemetry error report now identifies each stack frame in an exception's fingerprint by its function name, without leading underscores, instead of its module path and function name (fingerprint algorithm 2), so moving a function to another module no longer opens a duplicate issue. Every error group gets a new fingerprint: issues opened under algorithm 1 are no longer updated or reopened, and a group that still fails opens a new issue.

gcp/observability/README.md

@@ -155,12 +155,18 @@ through `github_client`. Each run:
    that tie, such as two exceptions logged at the same moment, are ordered by
    their own values, so the same records give the same report whatever order
    the query returns them in.
-3. Groups failures by a 16-character HMAC-SHA256 fingerprint of the service
-   family and either the exception type and innermost application stack
-   frames, the route and message template, or the route and HTTP status.
-   Frames come from the first traceback of the stack, which for a chained
-   exception is its first cause. The logging library keeps only the first
-   16,384 characters of a stack, so a long cause can drop the final
+3. Groups failures by a 16-character HMAC-SHA256 fingerprint of the
+   fingerprint algorithm number, the service family and either the exception
+   type and innermost application stack frames, the route and message
+   template, or the route and HTTP status. The frames are the three innermost
+   whose file paths are in application packages (or, when there are none,
+   the three innermost of any kind), and each is identified by its function
+   name alone, without leading underscores and without its module path or
+   line number. Moving a function to another module, or renaming a private
+   helper `_name` to `name`, keeps the fingerprint; any other rename changes
+   it. Frames come from the first traceback of the stack, which for a
+   chained exception is its first cause. The logging library keeps only the
+   first 16,384 characters of a stack, so a long cause can drop the final
    traceback, but the first one is always kept.
 4. Creates or updates one issue per group in this repository, labelled
    `telemetry-error` and `service: <service family>`. A group opens an issue
@@ -268,6 +274,18 @@ already has one. A key change needs a plan for moving the existing issues to
 their new fingerprints, as does any change to `FINGERPRINT_ALGORITHM` in the
 script.
 
+Fingerprint algorithm 2 replaced algorithm 1, which identified each stack
+frame by its module path and function name, so that moving code between
+modules no longer opens a duplicate issue. Because the algorithm number is
+part of every fingerprint, message and failed-request groups got new
+fingerprints too, although their inputs didn't change. Issues opened under
+algorithm 1 were not carried over: the job no longer matches them, so it
+neither updates nor reopens them, and a group that still fails opens a new
+issue once it meets the thresholds. An algorithm 1 issue can be closed by
+hand, or carried over by replacing the fingerprint at the end of its title
+with the group's new one; the job then matches it by its title and rewrites
+its marked section.
+
 ### Known gap: Cloud Run platform failures
 
 The source sinks forward only records in the application log format

tests/unit/telemetry_error_issues/test_normalization.py

@@ -9,7 +9,7 @@
 from telemetry_error_issues.bigquery import FRAME_PATTERN
 from telemetry_error_issues.normalization import (
     fingerprint_frames,
-    frame_key,
+    frame_location,
     normalize_path,
     public_error_type,
     public_family,
@@ -108,48 +108,48 @@ def test_frames_pattern_has_one_capturing_group_for_bigquery() -> None:
 def test_frames_pattern_extracts_frame_headers_without_source_or_message() -> None:
     frames = extract_frames(STANDARD_STACK)
 
-    assert [frame_key(frame) for frame in frames] == [
-        "flask/app.py:dispatch_request",
-        "policyengine_api/routes/report_routes.py:create_report",
-        "policyengine_api/services/report_service.py:create",
-        "sqlalchemy/orm/session.py:flush",
+    assert [frame_location(frame) for frame in frames] == [
+        ("flask/app.py", "dispatch_request"),
+        ("policyengine_api/routes/report_routes.py", "create_report"),
+        ("policyengine_api/services/report_service.py", "create"),
+        ("sqlalchemy/orm/session.py", "flush"),
     ]
     assert not any("SENTINEL" in frame or "\n" in frame for frame in frames)
 
 
 def test_frames_chained_traceback_contributes_only_its_first_traceback() -> None:
     frames = extract_frames(CHAINED_STACK)
 
-    assert [frame_key(frame) for frame in frames] == [
-        "policyengine_api/services/cache.py:read",
+    assert [frame_location(frame) for frame in frames] == [
+        ("policyengine_api/services/cache.py", "read"),
     ]
 
 
 def test_frames_explicitly_chained_traceback_contributes_only_its_cause() -> None:
     frames = extract_frames(chained_stack(recursion_depth=2))
 
-    assert [frame_key(frame) for frame in frames] == [
-        "policyengine_api/services/household.py:calculate",
-        "policyengine_core/simulation.py:calculate",
-        "policyengine_core/simulation.py:calculate",
+    assert [frame_location(frame) for frame in frames] == [
+        ("policyengine_api/services/household.py", "calculate"),
+        ("policyengine_core/simulation.py", "calculate"),
+        ("policyengine_core/simulation.py", "calculate"),
     ]
 
 
 def test_frames_pattern_reads_exception_group_tracebacks() -> None:
     frames = extract_frames(EXCEPTION_GROUP_STACK)
 
-    assert [frame_key(frame) for frame in frames] == [
-        "policyengine_simulation_gateway/app.py:run",
-        "policyengine_simulation_gateway/worker.py:step",
+    assert [frame_location(frame) for frame in frames] == [
+        ("policyengine_simulation_gateway/app.py", "run"),
+        ("policyengine_simulation_gateway/worker.py", "step"),
     ]
 
 
 def test_frames_pattern_skips_a_frame_header_cut_off_by_truncation() -> None:
     cut_at = STANDARD_STACK.index("create_report") + len("create_re")
     frames = extract_frames(STANDARD_STACK[:cut_at])
 
-    assert [frame_key(frame) for frame in frames] == [
-        "flask/app.py:dispatch_request",
+    assert [frame_location(frame) for frame in frames] == [
+        ("flask/app.py", "dispatch_request"),
     ]
 
 
@@ -179,11 +179,11 @@ def test_frames_paths_drop_installation_prefixes(path: str, normalized: str) ->
     assert normalize_path(path) == normalized
 
 
-def test_frames_keys_ignore_line_numbers() -> None:
-    first = frame_key('/app/policyengine_api/x.py", line 10, in handler')
-    second = frame_key('/srv/policyengine_api/x.py", line 99, in handler')
+def test_frames_locations_ignore_line_numbers() -> None:
+    first = frame_location('/app/policyengine_api/x.py", line 10, in handler')
+    second = frame_location('/srv/policyengine_api/x.py", line 99, in handler')
 
-    assert first == second == "policyengine_api/x.py:handler"
+    assert first == second == ("policyengine_api/x.py", "handler")
 
 
 def test_frames_choose_the_three_innermost_application_frames() -> None:
@@ -196,11 +196,7 @@ def test_frames_choose_the_three_innermost_application_frames() -> None:
         '/venv/lib/python3.12/site-packages/sqlalchemy/orm/f.py", line 6, in raises',
     ]
 
-    assert fingerprint_frames(frames) == (
-        "policyengine_api/services/b.py:middle",
-        "policyengine_api/services/c.py:inner",
-        "policyengine_api/services/e.py:callback",
-    )
+    assert fingerprint_frames(frames) == ("middle", "inner", "callback")
 
 
 def test_frames_count_simulation_and_stage12_packages_as_application_code() -> None:
@@ -211,9 +207,20 @@ def test_frames_count_simulation_and_stage12_packages_as_application_code() -> N
         '/pkg/synchronicity/combined_types.py", line 4, in __call__',
     ]
 
+    assert fingerprint_frames(frames) == ("execute", "save")
+
+
+def test_frames_are_keyed_by_function_name_without_leading_underscores() -> None:
+    frames = [
+        '/app/policyengine_api/services/regions.py", line 12, in _resolve_region',
+        '/app/policyengine_api/services/regions.py", line 30, in __load_boundaries',
+        '/app/policyengine_api/services/regions.py", line 44, in __getitem__',
+    ]
+
     assert fingerprint_frames(frames) == (
-        "policyengine_simulation_executor/run.py:execute",
-        "policyengine_stage12_persistence/store.py:save",
+        "resolve_region",
+        "load_boundaries",
+        "getitem__",
     )
 
 
@@ -225,11 +232,7 @@ def test_frames_without_application_frames_use_the_innermost_frames() -> None:
         '/pkg/synchronicity/d.py", line 4, in fourth',
     ]
 
-    assert fingerprint_frames(frames) == (
-        "modal/b.py:second",
-        "modal/c.py:third",
-        "synchronicity/d.py:fourth",
-    )
+    assert fingerprint_frames(frames) == ("second", "third", "fourth")
 
 
 @pytest.mark.parametrize("frames", [[], None, ["not a frame header"]])

tests/unit/telemetry_error_issues/test_records.py

@@ -24,6 +24,32 @@
 )
 
 
+def _frame_header(module_path: str, function_name: str, line_number: int) -> str:
+    """Return one stack frame header as the query extracts it."""
+    return f'/app/{module_path}", line {line_number}, in {function_name}'
+
+
+# One exception's stack before and after a file-length split moved the service
+# functions into new modules, and after the split made a private helper public.
+FRAMES_BEFORE_SPLIT = (
+    _frame_header("policyengine_api/routes/report_routes.py", "create_report", 41),
+    _frame_header("policyengine_api/services/report_service.py", "create", 88),
+    _frame_header(
+        "policyengine_api/services/report_service.py", "_resolve_region", 140
+    ),
+)
+FRAMES_AFTER_SPLIT = (
+    _frame_header("policyengine_api/routes/report_routes.py", "create_report", 41),
+    _frame_header("policyengine_api/services/report/creation.py", "create", 30),
+    _frame_header("policyengine_api/services/report/regions.py", "_resolve_region", 12),
+)
+FRAMES_AFTER_SPLIT_WITH_PUBLIC_HELPER = (
+    _frame_header("policyengine_api/routes/report_routes.py", "create_report", 41),
+    _frame_header("policyengine_api/services/report/creation.py", "create", 30),
+    _frame_header("policyengine_api/services/report/regions.py", "resolve_region", 12),
+)
+
+
 def test_primary_prefers_the_exception_record_of_a_failed_request() -> None:
     exception = exception_record()
     records = [
@@ -157,6 +183,46 @@ def test_fingerprint_exception_ignores_lines_path_prefixes_and_routes() -> None:
     assert fingerprint(first, FINGERPRINT_KEY) == fingerprint(second, FINGERPRINT_KEY)
 
 
+def test_fingerprint_exception_is_the_same_after_its_functions_move_modules() -> None:
+    before_split = exception_record(frames=FRAMES_BEFORE_SPLIT)
+    after_split = exception_record(frames=FRAMES_AFTER_SPLIT)
+
+    assert fingerprint(before_split, FINGERPRINT_KEY) == fingerprint(
+        after_split, FINGERPRINT_KEY
+    )
+
+
+@pytest.mark.parametrize(
+    "private_helper_frames",
+    [FRAMES_BEFORE_SPLIT, FRAMES_AFTER_SPLIT],
+    ids=["in-its-old-module", "in-its-new-module"],
+)
+def test_fingerprint_exception_ignores_leading_underscores_in_function_names(
+    private_helper_frames: tuple[str, ...],
+) -> None:
+    private_helper = exception_record(frames=private_helper_frames)
+    public_helper = exception_record(frames=FRAMES_AFTER_SPLIT_WITH_PUBLIC_HELPER)
+
+    assert fingerprint(private_helper, FINGERPRINT_KEY) == fingerprint(
+        public_helper, FINGERPRINT_KEY
+    )
+
+
+def test_fingerprint_exception_differs_when_a_function_is_renamed() -> None:
+    renamed_frames = (
+        *FRAMES_AFTER_SPLIT[:-1],
+        _frame_header(
+            "policyengine_api/services/report/regions.py", "_find_region", 12
+        ),
+    )
+    original = exception_record(frames=FRAMES_AFTER_SPLIT)
+    renamed = exception_record(frames=renamed_frames)
+
+    assert fingerprint(original, FINGERPRINT_KEY) != fingerprint(
+        renamed, FINGERPRINT_KEY
+    )
+
+
 def test_fingerprint_exception_merges_simulation_service_versions() -> None:
     first = exception_record(service_name="policyengine-simulation-py1-2-3")
     second = exception_record(service_name="policyengine-simulation-py1-2-4")
@@ -228,8 +294,8 @@ def test_fingerprint_key_parts_for_each_kind() -> None:
         "exception",
         "policyengine-api",
         "StaleDataError",
-        "policyengine_api/routes/report_routes.py:create_report",
-        "policyengine_api/services/report_service.py:create",
+        "create_report",
+        "create",
     )
     assert fingerprint_parts(message) == (
         "message",

tests/unit/telemetry_error_issues/test_rendering.py

@@ -219,11 +219,12 @@ def test_render_body_state_comment_records_the_group_and_window() -> None:
     _, body = _render(group)
 
     assert parse_state(body) == {
-        "algorithm": 1,
+        "algorithm": 2,
         "fingerprint": group.fingerprint,
         "kind": "exception",
         "window_end": "2026-10-21T04:00:00+00:00",
     }
+    assert "- Kind: exception (fingerprint algorithm 2)" in body
 
 
 def test_render_body_stays_under_the_github_limit() -> None: