Classify Dynatrace entity errors by status code and report proxy error bodies - #375
Merged
LouisParkin merged 3 commits intoSep 14, 2026
Merged
Conversation
The health check decided an entity was missing by looking for '404' in the exception message. That message embeds the request URL, so an entity id whose hex contains 404 was read as a missing entity, which marked its type problematic and suppressed every later event of that type in the run. Both call sites shared one problematic-type set, so the misclassification also suppressed health-state creation. The client now raises DynatraceApiError carrying the status code.
Only a Dynatrace-shaped envelope produced a useful message, so a proxy in front of it was reduced to a status code and a URL. The body is now appended when the error field is absent, collapsed to one line and capped. The JSON parse ran before the status check, so a non-JSON error page raised a decode error instead of being reported. It is now guarded for non-200 responses.
craffit
approved these changes
Sep 14, 2026
There was a problem hiding this comment.
🟡 Changes recommended
Message-based classification, malformed error envelopes, and possible credential disclosure remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds reliable Dynatrace HTTP error classification and improved proxy diagnostics.
Changes:
- Introduces status-aware
DynatraceApiError. - Reports truncated, single-line proxy response bodies.
- Adds regression tests for entity suppression and error handling.
File summaries
| File | Description |
|---|---|
dynatrace_health/tests/test_dynatrace_health.py |
Tests rejected-entity event handling. |
dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py |
Classifies missing entities using API errors. |
dynatrace_base/tests/test_dynatrace_client.py |
Tests status codes and response-body diagnostics. |
dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py |
Adds typed errors and body excerpts. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Redact credentials before truncating the excerpt: a gateway can reflect the request headers, and truncation does not stop the token reaching the log.
Tolerate a non-dict error value. A proxy answering {"error": "Forbidden"} raised AttributeError, bypassing both the typed error and the body fallback.
Require a typed 404 for entity-not-found. The message-text fallback let a transport failure reading 'host not found' suppress later events of the same type, which is the failure mode this branch set out to remove.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two defects found while investigating Rabobank's Dynatrace connectivity report (StackVista/stackstate#578).
The health check decided an entity was missing by looking for
404in the exception message, which embeds the request URL. An entity id whose hex contains404was therefore read as a missing entity, marked its type problematic, and suppressed every later event of that type in the run. In the customer's logs that fired in 1165 of 1165 runs, silently droppingPROCESS_GROUP_INSTANCEevents. The client now raisesDynatraceApiErrorcarrying the status code, and both call sites branch on that.Separately, only a Dynatrace-shaped
{"error": ...}envelope produced a useful message, so a proxy sitting in front of Dynatrace was reduced to a status code and a URL. The body is now appended whenerroris absent, collapsed to one line and capped at 500 chars. The JSON parse also ran before the status check, so a non-JSON error page raised a decode error rather than being reported — guarded for non-200 responses.Worth knowing before merge: the first fix increases API call volume. The broken classification was accidentally suppressing work, so a persistently-rejecting environment goes from stopping after the first bad entity to one lookup per referencing event. The fix is still correct — it was dropping real events — but the log-volume symptom gets worse. Negative caching was considered and declined (#373 records why); a per-run 403 suppression would address it and is not in this PR.
Fixes #373
Fixes #374
Validation:
dynatrace_base18,dynatrace_health28,dynatrace_topology40 — all passing, flake8 clean. The health regression test was confirmed to fail without the classification fix.