From f6e2b7381356b70666ef983e0ac248e5b76ec397 Mon Sep 17 00:00:00 2001 From: Louis Parkin Date: Mon, 14 Sep 2026 10:31:37 +0200 Subject: [PATCH 1/3] Classify Dynatrace entity errors by status code, not message text 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. --- .../dynatrace/dynatrace_client.py | 29 +++++++++--- dynatrace_base/tests/test_dynatrace_client.py | 26 ++++++++++- .../dynatrace_health/dynatrace_health.py | 17 +++++-- .../tests/test_dynatrace_health.py | 45 +++++++++++++++++++ 4 files changed, 107 insertions(+), 10 deletions(-) diff --git a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py index 84db24fa..a644ecdd 100644 --- a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py +++ b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py @@ -11,6 +11,19 @@ from stackstate_checks.dynatrace.custom_auth import MsJWTAuth +class DynatraceApiError(Exception): + """ + Raised when a Dynatrace API call returns a non-200 response. + + Carries the status code so callers can branch on it. The message embeds the request + URL, so matching text against it reads entity ids as status codes. + """ + + def __init__(self, message, status_code=None): + super(DynatraceApiError, self).__init__(message) + self.status_code = status_code + + class _DynatraceClient: def __init__(self, token, verify=False, cert=None, keyfile=None, timeout=None, is_ms_jwt_auth=False): @@ -100,23 +113,27 @@ def do_request(current_headers): ): self._handle_entity_404(endpoint, msg) # Always raise after handling 404 so callers can react and tests assert - raise Exception( + raise DynatraceApiError( 'Got an unexpected error with status code %s and message: %s' - % (response.status_code, msg) + % (response.status_code, msg), + response.status_code ) elif response.status_code == 401: # Provide clearer guidance for non-JWT (or failed refresh) 401s - raise Exception( + raise DynatraceApiError( ( "401 unauthorized for %s. Verify token validity and required API v2 scopes " "(entities.read, events.read, eventTypes.read). Message: %s" ) - % (endpoint, msg) + % (endpoint, msg), + response.status_code ) else: self.log.error(msg) - raise Exception( - 'Got an unexpected error with status code %s and message: %s' % (response.status_code, msg)) + raise DynatraceApiError( + 'Got an unexpected error with status code %s and message: %s' + % (response.status_code, msg), + response.status_code) return response_json except Timeout: msg = "%d seconds timeout" % self.timeout diff --git a/dynatrace_base/tests/test_dynatrace_client.py b/dynatrace_base/tests/test_dynatrace_client.py index 80b51d2e..1b7ebf96 100644 --- a/dynatrace_base/tests/test_dynatrace_client.py +++ b/dynatrace_base/tests/test_dynatrace_client.py @@ -3,7 +3,7 @@ # Licensed under a 3-clause BSD style license (see LICENSE) import pytest -from stackstate_checks.dynatrace.dynatrace_client import DynatraceClientFactory +from stackstate_checks.dynatrace.dynatrace_client import DynatraceApiError, DynatraceClientFactory def test_endpoint_generation(dynatrace_client): @@ -38,6 +38,30 @@ def test_status_200(dynatrace_client, requests_mock, test_instance): assert response["events"][0]['eventId'] == '123' +def test_api_error_carries_status_code(dynatrace_client, requests_mock, test_instance): + """ + Check that the status code is available on the exception. The entity id used here + contains the hex sequence 404, which callers used to read out of the message text. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), + '/api/v2/entities/PROCESS_GROUP_INSTANCE-7091B9883B404E8E') + requests_mock.get(endpoint, text='{"detail": "denied by policy"}', status_code=403) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert exc.value.status_code == 403 + + +def test_entity_404_carries_status_code(dynatrace_client, requests_mock, test_instance): + """ + Check that a genuine missing entity is still reported as 404. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities/HOST-123') + requests_mock.get(endpoint, text='{"error": {"message": "Entity not found"}}', status_code=404) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert exc.value.status_code == 404 + + def test_entity_404_handling_single_type(dynatrace_client, requests_mock, test_instance, caplog): """ Test that 404 errors for entities are logged at INFO level with counting. diff --git a/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py b/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py index c880a9e8..fd849e89 100644 --- a/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py +++ b/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py @@ -9,7 +9,7 @@ from stackstate_checks.base import StackPackInstance, HealthStream, HealthStreamUrn, Health, Identifiers from stackstate_checks.checks import AgentCheck -from stackstate_checks.dynatrace.dynatrace_client import DynatraceClientFactory +from stackstate_checks.dynatrace.dynatrace_client import DynatraceApiError, DynatraceClientFactory from stackstate_checks.dynatrace.constants import SUPPORTED_ENTITY_TYPES_PARAM_SELECTORS from stackstate_checks.dynatrace_health.event_data_types import DynatraceEvent @@ -234,7 +234,7 @@ def _process_events(self, dynatrace_client, instance_info): display_name) except Exception as e: # Check if this is a 404 error (entity no longer exists) - if "404" in str(e) or "not found" in str(e).lower(): + if self._is_entity_not_found(e): # Extract entity type from entity_id if possible entity_type = self._extract_entity_type(entity_id) if entity_type not in entity_404_errors: @@ -290,7 +290,7 @@ def _process_events(self, dynatrace_client, instance_info): entity_id = self._get_entity_id(event) # Check if this is a 404 error (entity no longer exists) - if "404" in str(e) or "not found" in str(e).lower(): + if self._is_entity_not_found(e): # Extract entity type from entity_id if possible entity_type = self._extract_entity_type(entity_id) if entity_type not in entity_404_errors: @@ -540,6 +540,17 @@ def _get_entity_id(event): return event.entityId.entityId.id return 'unknown' + @staticmethod + def _is_entity_not_found(error): + """ + Whether an error means the entity is gone rather than that the request was rejected. + :param error: the exception raised while resolving an entity + :return: True if the entity no longer exists + """ + if isinstance(error, DynatraceApiError): + return error.status_code == 404 + return 'not found' in str(error).lower() + @staticmethod def _extract_entity_type(entity_id): """ diff --git a/dynatrace_health/tests/test_dynatrace_health.py b/dynatrace_health/tests/test_dynatrace_health.py index 2d38ec1a..866ba1c0 100644 --- a/dynatrace_health/tests/test_dynatrace_health.py +++ b/dynatrace_health/tests/test_dynatrace_health.py @@ -190,6 +190,51 @@ def test_custom_info_event(dynatrace_check, test_instance, requests_mock, health assert telemetry._topology_events[0]['msg_title'] == "Custom Info on Mobile App" +def _pgi_info_event(event_id, entity_id, entity_name): + """ + Helper function to build a CUSTOM_INFO event for a PROCESS_GROUP_INSTANCE entity + """ + event = json.loads(json.dumps(_get_varied_event_by_type("CUSTOM_INFO"))) + event['eventId'] = event_id + event['entityId']['entityId']['id'] = entity_id + event['entityId']['entityId']['type'] = 'PROCESS_GROUP_INSTANCE' + event['entityId']['name'] = entity_name + return event + + +@freeze_time('2025-07-22 08:26:24') +def test_rejected_entity_does_not_suppress_remaining_events(dynatrace_check, test_instance, requests_mock, + aggregator, telemetry): + """ + A rejected entity lookup must not be read as a missing entity. The first id below + contains the hex sequence 404, which used to mark PROCESS_GROUP_INSTANCE problematic + and drop every remaining event of that type for the rest of the run. + """ + os.environ["JWT_AUTH"] = "false" + rejected_id = 'PROCESS_GROUP_INSTANCE-7091B9883B404E8E' + resolvable_id = 'PROCESS_GROUP_INSTANCE-1234567890ABCDEF' + event_response = { + "totalCount": 2, + "pageSize": 2, + "events": [ + _pgi_info_event('rejected-1', rejected_id, 'checkout-worker'), + _pgi_info_event('resolvable-1', resolvable_id, 'billing-worker'), + ], + } + set_http_responses(requests_mock, custom_info_event=read_file('event_type_custom_info.json', 'samples')) + requests_mock.get("{}/api/v2/entities/{}".format(test_instance['url'], rejected_id), + text='{"detail": "denied by policy"}', status_code=403) + requests_mock.get("{}/api/v2/entities/{}".format(test_instance['url'], resolvable_id), + text=json.dumps({"displayName": "billing-worker"})) + _mock_events_endpoint(requests_mock, test_instance, event_response) + + dynatrace_check.run() + + aggregator.assert_service_check(dynatrace_check.SERVICE_CHECK_NAME, count=1, status=AgentCheck.OK) + assert len(telemetry._topology_events) == 1 + assert telemetry._topology_events[0]['msg_title'] == "Custom Info on billing-worker" + + @freeze_time('2025-07-22 08:26:24') def test_marked_for_termination_event(dynatrace_check, test_instance, requests_mock, health, aggregator, telemetry): event_type = "MARKED_FOR_TERMINATION" From 33af9bbc42296f2227adc60bb05eb89cda633ced Mon Sep 17 00:00:00 2001 From: Louis Parkin Date: Mon, 14 Sep 2026 10:32:31 +0200 Subject: [PATCH 2/3] Report the response body when a Dynatrace error carries no error field 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. --- .../dynatrace/dynatrace_client.py | 39 +++++++++++++-- dynatrace_base/tests/test_dynatrace_client.py | 48 +++++++++++++++++++ 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py index a644ecdd..875fa2dd 100644 --- a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py +++ b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py @@ -10,6 +10,8 @@ from stackstate_checks.dynatrace.custom_auth import MsJWTAuth +ERROR_BODY_MAX_CHARS = 500 + class DynatraceApiError(Exception): """ @@ -99,12 +101,23 @@ def do_request(current_headers): retry_headers = {"Authorization": "Bearer %s" % self.token} response = do_request(retry_headers) - response_json = response.json() + try: + response_json = response.json() + except ValueError: + # A proxy in front of Dynatrace can answer with a non-JSON error page + if response.status_code == 200: + raise + response_json = None + if response.status_code != 200: - if "error" in response_json: + msg = None + if isinstance(response_json, dict) and "error" in response_json: msg = response_json["error"].get("message") - else: + if not msg: msg = "Got %s when hitting %s" % (response.status_code, endpoint) + body = self._error_body_excerpt(response) + if body: + msg = "%s; response body: %s" % (msg, body) # Handle 404s for all entity types with smart logging and counting if ( @@ -140,6 +153,26 @@ def do_request(current_headers): self.log.error(msg) raise Exception("Timeout exception occurred for endpoint %s with message: %s" % (endpoint, msg)) + @staticmethod + def _error_body_excerpt(response): + """ + Returns a length-capped, single-line excerpt of an error response body, or None. + Anything proxying Dynatrace answers in its own envelope, where the body is the + only statement of who rejected the request. + :param response: the non-200 response + :return: the excerpt to append to the error message + """ + try: + text = response.text or "" + except Exception: + return None + text = " ".join(text.split()) + if not text: + return None + if len(text) > ERROR_BODY_MAX_CHARS: + return text[:ERROR_BODY_MAX_CHARS] + "... [truncated]" + return text + def get_endpoint(self, url, path): """ Creates the API endpoint from the path diff --git a/dynatrace_base/tests/test_dynatrace_client.py b/dynatrace_base/tests/test_dynatrace_client.py index 1b7ebf96..c4f45148 100644 --- a/dynatrace_base/tests/test_dynatrace_client.py +++ b/dynatrace_base/tests/test_dynatrace_client.py @@ -62,6 +62,54 @@ def test_entity_404_carries_status_code(dynatrace_client, requests_mock, test_in assert exc.value.status_code == 404 +def test_error_body_reported_when_no_error_field(dynatrace_client, requests_mock, test_instance): + """ + Check that a proxy's own error envelope reaches the message. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, text='{"messageId": "abc-123", "reason": "policy denied"}', status_code=403) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert 'policy denied' in str(exc.value) + assert 'abc-123' in str(exc.value) + + +def test_error_body_reported_when_body_is_not_json(dynatrace_client, requests_mock, test_instance): + """ + Check that a non-JSON error page is reported rather than raising a decode error. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, text='\n Gateway policy denied\n', status_code=403) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert exc.value.status_code == 403 + assert 'Gateway policy denied' in str(exc.value) + + +def test_error_body_is_truncated(dynatrace_client, requests_mock, test_instance): + """ + Check that a large error page cannot flood the log. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, text='x' * 5000, status_code=502) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert '[truncated]' in str(exc.value) + assert len(str(exc.value)) < 800 + + +def test_error_message_preferred_over_body(dynatrace_client, requests_mock, test_instance): + """ + Check that Dynatrace's own error message still wins when it is present. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, text='{"error": {"message": "Token is missing required scope"}}', status_code=403) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert 'Token is missing required scope' in str(exc.value) + assert 'response body' not in str(exc.value) + + def test_entity_404_handling_single_type(dynatrace_client, requests_mock, test_instance, caplog): """ Test that 404 errors for entities are logged at INFO level with counting. From 60d2d7baef716c0c921d56b966ed387d47dab061 Mon Sep 17 00:00:00 2001 From: Louis Parkin Date: Mon, 14 Sep 2026 10:46:12 +0200 Subject: [PATCH 3/3] Harden the error body excerpt and narrow entity-not-found to a real 404 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. --- .../dynatrace/dynatrace_client.py | 22 +++++++-- dynatrace_base/tests/test_dynatrace_client.py | 49 +++++++++++++++++++ .../dynatrace_health/dynatrace_health.py | 9 ++-- .../tests/test_dynatrace_health.py | 11 +++++ 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py index 875fa2dd..0eb965df 100644 --- a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py +++ b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py @@ -4,6 +4,7 @@ import logging import os +import re from collections import defaultdict from requests import Session, Timeout @@ -12,6 +13,9 @@ ERROR_BODY_MAX_CHARS = 500 +# A gateway can reflect the request headers back in its error body +AUTH_ECHO_PATTERN = re.compile(r'(?i)\b(authorization|api-token|bearer)\b([\s:=]*)\S+') + class DynatraceApiError(Exception): """ @@ -111,8 +115,13 @@ def do_request(current_headers): if response.status_code != 200: msg = None - if isinstance(response_json, dict) and "error" in response_json: - msg = response_json["error"].get("message") + if isinstance(response_json, dict): + # A proxy may use the same key for a plain string or a null + error = response_json.get("error") + if isinstance(error, dict): + msg = error.get("message") + elif isinstance(error, str): + msg = error if not msg: msg = "Got %s when hitting %s" % (response.status_code, endpoint) body = self._error_body_excerpt(response) @@ -153,12 +162,12 @@ def do_request(current_headers): self.log.error(msg) raise Exception("Timeout exception occurred for endpoint %s with message: %s" % (endpoint, msg)) - @staticmethod - def _error_body_excerpt(response): + def _error_body_excerpt(self, response): """ Returns a length-capped, single-line excerpt of an error response body, or None. Anything proxying Dynatrace answers in its own envelope, where the body is the - only statement of who rejected the request. + only statement of who rejected the request. Credentials are redacted before + truncation, so a reflected request header cannot reach the log intact. :param response: the non-200 response :return: the excerpt to append to the error message """ @@ -167,6 +176,9 @@ def _error_body_excerpt(response): except Exception: return None text = " ".join(text.split()) + if self.token: + text = text.replace(self.token, "[redacted]") + text = AUTH_ECHO_PATTERN.sub(r'\1\2[redacted]', text) if not text: return None if len(text) > ERROR_BODY_MAX_CHARS: diff --git a/dynatrace_base/tests/test_dynatrace_client.py b/dynatrace_base/tests/test_dynatrace_client.py index c4f45148..b66208de 100644 --- a/dynatrace_base/tests/test_dynatrace_client.py +++ b/dynatrace_base/tests/test_dynatrace_client.py @@ -98,6 +98,55 @@ def test_error_body_is_truncated(dynatrace_client, requests_mock, test_instance) assert len(str(exc.value)) < 800 +def test_error_body_redacts_reflected_token(dynatrace_client, requests_mock, test_instance): + """ + Check that a gateway reflecting the request headers cannot put the token in the log. + """ + token = test_instance.get('token') + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, status_code=403, + text='{"rejected": {"Authorization": "Api-Token %s"}}' % token) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert token not in str(exc.value) + assert '[redacted]' in str(exc.value) + + +def test_error_body_redacts_bearer_echo(dynatrace_client, requests_mock, test_instance): + """ + Check that an authorization echo is redacted even when the value is not our token. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, text='sent: Bearer eyJhbGciOi.someoneelsestoken', status_code=403) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert 'someoneelsestoken' not in str(exc.value) + + +def test_error_field_as_plain_string(dynatrace_client, requests_mock, test_instance): + """ + Check that a non-dict error value is reported instead of raising AttributeError. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, text='{"error": "Forbidden"}', status_code=403) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert exc.value.status_code == 403 + assert 'Forbidden' in str(exc.value) + + +def test_error_field_null_falls_back_to_body(dynatrace_client, requests_mock, test_instance): + """ + Check that a null error value falls through to the body excerpt. + """ + endpoint = dynatrace_client.get_endpoint(test_instance.get('url'), '/api/v2/entities') + requests_mock.get(endpoint, text='{"error": null, "reason": "quota exceeded"}', status_code=429) + with pytest.raises(DynatraceApiError) as exc: + dynatrace_client.get_dynatrace_json_response(endpoint) + assert exc.value.status_code == 429 + assert 'quota exceeded' in str(exc.value) + + def test_error_message_preferred_over_body(dynatrace_client, requests_mock, test_instance): """ Check that Dynatrace's own error message still wins when it is present. diff --git a/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py b/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py index fd849e89..b9d82c53 100644 --- a/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py +++ b/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py @@ -543,13 +543,14 @@ def _get_entity_id(event): @staticmethod def _is_entity_not_found(error): """ - Whether an error means the entity is gone rather than that the request was rejected. + Whether an error means the entity is gone rather than that the request failed. + Only a 404 from the API qualifies. A transport or local failure is no evidence + that the entity is missing, and treating it as such suppresses later events of + the same type for the rest of the run. :param error: the exception raised while resolving an entity :return: True if the entity no longer exists """ - if isinstance(error, DynatraceApiError): - return error.status_code == 404 - return 'not found' in str(error).lower() + return isinstance(error, DynatraceApiError) and error.status_code == 404 @staticmethod def _extract_entity_type(entity_id): diff --git a/dynatrace_health/tests/test_dynatrace_health.py b/dynatrace_health/tests/test_dynatrace_health.py index 866ba1c0..5965da61 100644 --- a/dynatrace_health/tests/test_dynatrace_health.py +++ b/dynatrace_health/tests/test_dynatrace_health.py @@ -12,6 +12,7 @@ from stackstate_checks.base import AgentCheck from stackstate_checks.base.utils.common import read_file +from stackstate_checks.dynatrace.dynatrace_client import DynatraceApiError from .conftest import set_http_responses @@ -190,6 +191,16 @@ def test_custom_info_event(dynatrace_check, test_instance, requests_mock, health assert telemetry._topology_events[0]['msg_title'] == "Custom Info on Mobile App" +def test_transport_error_is_not_a_missing_entity(dynatrace_check): + """ + Only a 404 from the API means the entity is gone. A transport failure whose message + happens to read like one must not suppress later events of the same type. + """ + assert dynatrace_check._is_entity_not_found(DynatraceApiError('gone', 404)) is True + assert dynatrace_check._is_entity_not_found(DynatraceApiError('denied', 403)) is False + assert dynatrace_check._is_entity_not_found(requests.ConnectionError('host not found')) is False + + def _pgi_info_event(event_id, entity_id, entity_name): """ Helper function to build a CUSTOM_INFO event for a PROCESS_GROUP_INSTANCE entity