Cache failed Dynatrace entity lookups for the duration of a check run

[LouisParkin]

_get_entity_definition cached only successful lookups, so every event referencing an unreachable entity cost a request. Over 97h of customer logs that was 12286 requests where 7211 would have done — 41% avoidable, and all 1165 runs re-queried at least one failing entity more than once.

The cache is reset each run, so an entity that becomes reachable again is picked up on the next check a few minutes later. This is not the persistent negative caching that was declined in StackVista/stackstate#578; only the within-run repeats go away.

Also removes the PROCESS_GROUP_INSTANCE skip this makes redundant. Worth a look from a reviewer: that special case was still losing data after #373, because a genuine 404 on one entity dropped every other PROCESS_GROUP_INSTANCE event in the run, including entities that had never failed. test_missing_entity_does_not_suppress_type covers it.

One changed number to expect: get_entity_404_summary() now counts once per entity per run rather than once per referencing event.

Fixes #376

Validation: dynatrace_base 22, dynatrace_health 31, dynatrace_topology 40 — all passing, flake8 clean. The caching test was confirmed to fail without the change.

[Copilot]

🟡 Changes recommended

Reusing cached exception objects causes their traceback chains to grow on every cache hit.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Caches failed Dynatrace entity lookups per check run, reducing duplicate requests without persisting failures across runs.

Changes:

• Caches and re-raises failed entity lookups.
• Removes PROCESS_GROUP_INSTANCE type-wide suppression.
• Adds regression tests for request caching and independent entity processing.

File summaries

File	Description
dynatrace_health.py	Implements run-scoped failure caching and removes type suppression.
test_dynatrace_health.py	Tests failed lookup caching and continued same-type processing.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[LouisParkin]

Self-review notes before this goes further. One real defect, one cosmetic, one thing worth recording.

Traceback growth on the cached exception. _get_entity_definition stores the exception object and re-raises the same instance per event. Each raise appends a frame to its __traceback__, so it grows linearly for the run — measured 1004 frames after 1000 re-raises, and each frame pins its locals.

This is worst exactly where the change is aimed: the 7211 avoidable lookups in the description are repeats on a few dead entities, so one cached exception can accumulate thousands of frames in a single run of a long-lived agent.

The fix has to keep the object rather than rebuild it, because _is_entity_not_found tests isinstance(error, DynatraceApiError) and error.status_code == 404 — an attribute, not args, so type(e)(*e.args) would drop status_code. Resetting the traceback caps it at one frame (verified):

if isinstance(cached, Exception):
    cached.__traceback__ = None
    raise cached

Asymmetric reset in check(). _event_type_cache is reset unconditionally, _entity_cache behind if hasattr(self, '_entity_cache'). The guard has no effect; unconditional assignment matches the line above.

Two 404 tallies now mean different things. dynatrace_client.get_entity_404_summary() increments inside get_dynatrace_json_response, so with the cache in front of the HTTP call it counts per entity. The local entity_404_errors increments in the except block, which still runs per event, so it stays per event. Both are correct and the local log message still says "events", but the two numbers are no longer comparable and nothing says so.

Checked and holding up, so a reviewer need not re-derive:

• Per-run reset is real — check() before any processing, with a test.
• The get_entity_404_summary() claim is right, for a non-obvious reason: the cache sits in front of the HTTP layer where _handle_entity_404 increments.
• No ordering hazard with cache warming — _warm_all_supported_entities runs before event processing, so a cached failure cannot shadow warm data.
• Both new tests assert behaviour rather than smoke: one request for three events, and the healthy entity surviving a sibling's 404.
• _extract_entity_type is still used, so removing the skip left no dead code.