diff --git a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py index 84db24fa..0eb965df 100644 --- a/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py +++ b/dynatrace_base/stackstate_checks/dynatrace/dynatrace_client.py @@ -4,12 +4,31 @@ import logging import os +import re from collections import defaultdict from requests import Session, Timeout from stackstate_checks.dynatrace.custom_auth import MsJWTAuth +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): + """ + 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: @@ -86,12 +105,28 @@ 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 = response_json["error"].get("message") - else: + msg = None + 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) + if body: + msg = "%s; response body: %s" % (msg, body) # Handle 404s for all entity types with smart logging and counting if ( @@ -100,29 +135,56 @@ 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 self.log.error(msg) raise Exception("Timeout exception occurred for endpoint %s with message: %s" % (endpoint, msg)) + 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. 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 + """ + try: + text = response.text or "" + 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: + 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 80b51d2e..b66208de 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,127 @@ 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_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_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. + """ + 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. diff --git a/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py b/dynatrace_health/stackstate_checks/dynatrace_health/dynatrace_health.py index c880a9e8..b9d82c53 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,18 @@ 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 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 + """ + 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 2d38ec1a..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,61 @@ 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 + """ + 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"