Skip to content

Report event submission failures that the Datadog client returns instead of raising - #22

Open
eastagiletracker wants to merge 1 commit into
ustudio:masterfrom
eastagiletracker:agile-board/report-muted-event-errors
Open

Report event submission failures that the Datadog client returns instead of raising#22
eastagiletracker wants to merge 1 commit into
ustudio:masterfrom
eastagiletracker:agile-board/report-muted-event-errors

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes reporting event submission failures that the Datadog client returns instead of raising, so that an event the API rejects reaches handleError rather than disappearing without a trace. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/298. You can sign in with your GitHub ID to claim ownership of the project.

What this fixes

DatadogLogHandler.emit wraps the submission in try/except Exception and hands failures to handleError — the reporting route added in #15, "so that the errors can be reported by the base Handler". The datadog client, though, is muted by default (datadog.api._mute is True unless callers pass initialize(mute=False)), and in that mode api_client.py catches ApiError/ClientError, logs it on the datadog.api logger, and returns the error payload — {"errors": [...]} — to the caller. So Event.create does not raise for the everyday failures: a wrong or expired API key (403 {"errors": ["Forbidden"]}), a payload the API rejects, or a connection error. emit reads that response as success, and the log record is dropped with nothing surfaced through the handler's own error reporting — which is what "nothing shows up in Datadog" looks like from the application side.

Reproduced on master at fe4bc06, with a local server standing in for the Events API and answering the way Datadog answers a bad key:

import json
import logging
import threading
from http.server import BaseHTTPRequestHandler, HTTPServer

import datadog
from datadog_logger import log_error_events


class Rejects(BaseHTTPRequestHandler):
    def do_POST(self):
        self.rfile.read(int(self.headers.get("Content-Length", 0)))
        body = json.dumps({"errors": ["Forbidden"]}).encode()
        self.send_response(403)
        self.send_header("Content-Type", "application/json")
        self.send_header("Content-Length", str(len(body)))
        self.end_headers()
        self.wfile.write(body)

    def log_message(self, *args):
        pass


server = HTTPServer(("127.0.0.1", 0), Rejects)
threading.Thread(target=server.serve_forever, daemon=True).start()

datadog.initialize(api_key="fake", app_key="fake",
                   api_host="http://127.0.0.1:%d" % server.server_address[1])

logging.basicConfig()
# Keep the client's own error logging out of the root logger, so the only thing
# under test is what the handler does with the response it gets back.
logging.getLogger("datadog.api").propagate = False

log_error_events()
logging.error("Oh no!")

On master the entire output is the local ERROR:root:Oh no! line; the event never reached Datadog and the handler said nothing about it. With this change the standard logging error report follows it:

ERROR:root:Oh no!
--- Logging error ---
Traceback (most recent call last):
  File ".../datadog_logger/handler.py", line 54, in emit
    raise ApiError(response)
datadog.api.exceptions.ApiError: {'errors': ['Forbidden']}

The change captures what Event.create returns and, when the response carries errors, raises ApiError inside the existing try so the failure takes the same route as a raised exception. A successful submission is untouched (the API answers {"status": "ok", "event": {...}}, and an empty errors list is treated as success too), an unmuted client still raises and is handled exactly as before, and no name or signature in the package changes.

Verification, on Python 3.12 with the locked dev dependencies: pytest 12 passed before, 15 passed after; flake8 and mypy (strict) clean before and after. The new test_emit_calls_handle_error_when_response_contains_errors fails against the current master handler with AssertionError: Expected 'handleError' to be called once. Called 0 times. and passes with the change; the two companion tests assert handleError stays out of the way for a successful response and for {"errors": []}, and pass in both trees.

How this was managed

The work above was tracked as a single story, Report event submission failures that the Datadog client returns instead of raising, on a board at https://eastagiletracker.com/projects/298 that was populated from this repository's own issues and pull requests (21 stories, 3 labels), so the history you see there is yours.

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

…not raise them when it is muted (the default), and passing them to handleError so that events rejected by the API are reported instead of being silently dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant