Skip to content

feat: add request id to django and flask libraries and propagate event context to celery tasks #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 2, 2025

Conversation

rabidpraxis
Copy link
Contributor

@rabidpraxis rabidpraxis commented May 29, 2025

This PR adds request ID tracking for both Django and Flask.

We inject the request_id into the event_context to be propagated to all emitted events. For Django, we first check the request object for any id or request_id attributes, in case some middleware has already created a request_id. This also inserts the correlation_context into error notices.

I also updated the Celery plugin to automatically rehydrate any parent event context into the worker before it starts processing tasks.

There is a bit of code reworking the Celery worker. I had an issue where events would not be sent in forked workers. I initially thought the only solution was to route events from worker processes back to the main process (where the batch worker was still alive), but that was overcomplicating things. Really, each worker process just needed its own working batch worker thread. I also realized that calling honeybadger.event in a worker did not work with my previous implementation.

When Celery forks worker processes, they inherit a copy of the batch worker object but the actual thread dies during the fork. So we had workers trying to queue events to a dead thread. Instead of building a complex IPC system to send events back to the main process, the fix was to just restart the batch worker thread in each worker process. Now every worker has its own live thread that can actually send events, and they all work independently without needing to communicate with the parent process at all.

@rabidpraxis rabidpraxis requested review from roelbondoc, subzero10, a team and Copilot May 29, 2025 21:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces request ID tracking across Django, Flask, and Celery, ensuring that request_id is captured or generated at the start of web requests and propagated into event contexts and Celery tasks. It also restarts the Honeybadger events worker in each Celery forked process.

  • Inject request_id from headers or middleware into Honeybadger event context for Flask and Django.
  • Pass correlation_context (containing request_id) through payload creation and notices.
  • Update Celery plugin to embed and restore event context in task headers and restart the events worker thread after forking.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
honeybadger/tests/*.py Add tests for correlation context, request ID propagation, and worker init behavior.
honeybadger/payload.py Support correlation_context in create_payload.
honeybadger/notice.py Add request_id field and include it in _correlation_context.
honeybadger/core.py Extract request_id from event context and pass to Notice.
honeybadger/events_worker.py Allow restarting the batch worker thread after process fork.
honeybadger/contrib/flask.py Generate or extract X-Request-ID and set event context.
honeybadger/contrib/django.py Extract or generate request_id and set event context.
honeybadger/contrib/celery.py Propagate event context into task headers; restart events worker; use signals instead of polling.

rabidpraxis and others added 4 commits May 29, 2025 14:12
@stympy stympy changed the title feat(insights) add request id to django and flask libraries and propagate event context to celery tasks feat: add request id to django and flask libraries and propagate event context to celery tasks Jun 2, 2025
request_id = (
getattr(request, "id", None)
or getattr(request, "request_id", None)
or request.headers.get("X-Request-ID", None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't have control of the contents of the X-Request-ID header, and since it is going to be passed along to our API, this value should be truncated to a reasonable length and characters outside of the set used in UUIDs ([a-z0-9-]+) should be stripped.

Copy link
Contributor Author

@rabidpraxis rabidpraxis Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I decided to sanitize at the points of collection, as that felt like the best place since we store it in the event, use it for sampling (in the other PR), and it goes into the notice.

@@ -201,6 +202,12 @@ def _initialize_honeybadger(self, config):
def _handle_request_started(self, sender, *args, **kwargs):
from flask import request

request_id = request.headers.get("X-Request-ID")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto the previous comment about sanitizing this value.

@@ -80,6 +80,8 @@ def notify(
merged_ctx = {**base, **(context or {})}
merged_tags = list({*tag_ctx, *(tags or [])})

request_id = self._get_event_context().get("request_id", None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be best to centralize the sanitization here. :)

@rabidpraxis rabidpraxis merged commit 06e1eec into insights-instrumentation Jun 2, 2025
34 checks passed
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.

2 participants