Repository navigation
chore(serverless): publish web request-start event - #19898
gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit into
Conversation
Codeowners resolved asResolved from the full PR diff against No remaining files require a CODEOWNERS review. |
Circular import analysis
|
Dependency direction analysis
|
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🔗 Commit SHA: 1e76b73 | Docs | View more details | Give us feedback! |
4aebb33 to
206cd22
Compare
206cd22 to
f3ea773
Compare
f3ea773 to
68bf61e
Compare
68bf61e to
8e26c22
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The environment detector lacks direct coverage, and the release note announces behavior completed only by a follow-up PR.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds pre-span MicroVM request detection to WSGI and ASGI instrumentation, supporting the runtime identity refresh stack.
Changes:
- Dispatches
web.request.startingfor matching MicroVM requests. - Adds explicit runtime identity refresh callbacks while preserving fork lineage.
- Adds focused integration/runtime tests and a release note.
File summaries
| File | Description |
|---|---|
ddtrace/contrib/_events/web_framework.py |
Defines the request-start event. |
ddtrace/contrib/internal/web.py |
Matches and dispatches MicroVM requests. |
ddtrace/contrib/internal/asgi/middleware.py |
Dispatches before ASGI span creation. |
ddtrace/contrib/internal/wsgi/wsgi.py |
Dispatches before WSGI span creation. |
ddtrace/internal/serverless/__init__.py |
Detects the MicroVM environment. |
ddtrace/internal/runtime/__init__.py |
Adds explicit identity refresh support. |
tests/contrib/asgi/test_asgi.py |
Tests ASGI matching and ordering. |
tests/contrib/wsgi/test_wsgi.py |
Tests WSGI request matching. |
tests/tracer/runtime/test_runtime_id.py |
Tests refresh and fork semantics. |
releasenotes/notes/add-lambda-microvm-web-request-starting-ffa6841a15e3c57c.yaml |
Announces MicroVM request detection. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8e26c22 to
cdc53d4
Compare
d28bc4e to
0d5cf10
Compare
0d5cf10 to
4654514
Compare
bfe3d91 to
a44c5ce
Compare
a44c5ce to
7398b18
Compare
Stacked PRs: - 👉 This #19778 — runtime identity foundation - #19898 — WSGI/ASGI request-start hook - #19939 — central MicroVM /run identity refresh - #19820 — tracer/native writer exporter refresh - #19821 — telemetry worker refresh - #19822 — runtime metrics runtime-id tags ## Description Adds `ddtrace.internal.runtime.refresh_identity()` for runtimes that need a fresh runtime ID without going through an OS fork. This introduces `on_runtime_identity_refresh()`, a dedicated callback registry for consumers that should react only to explicit identity refreshes. Existing `on_runtime_id_change()` behavior remains available for fork-related and general runtime-ID change handling. `refresh_identity()` rotates the runtime ID without recording parent or ancestor lineage because a resumed MicroVM run is not a real fork. Existing fork behavior and lineage tracking remain unchanged. Refresh callbacks are isolated by default so one failed consumer does not block others. Callers coordinating a refresh transaction can opt into error propagation with `raise_on_error=True` and retry the same identity refresh when a rebuild fails. ## Reference - [RFC](https://docs.google.com/document/d/17lde4Zak2YRBuDvf32KFanXj9TdpOv24GYaEY8swA1w/edit?tab=t.0#heading=h.wwu37decnk8j) ## Testing Added coverage for: - runtime ID rotation through `refresh_identity()` - preservation of parent and ancestor fork lineage - explicit refresh subscriber notification - refresh subscribers not being invoked by fork handling - callback failure isolation by default - optional refresh callback failure propagation - successful callbacks under `raise_on_error=True` Validation: - `scripts/run-tests -s --venv 190fcc7 -- -q tests/tracer/runtime/test_runtime_id.py` — 21 passed - `scripts/lint checks` — passed ## Risks Low. The new path is opt-in and is not wired to traffic in this PR. Existing fork behavior is preserved. Co-authored-by: tianning.li <tianning.li@datadoghq.com>
lym953
left a comment
There was a problem hiding this comment.
Approving blindly as I don't have knowledge on any of the files, and since apm-serverless has approved it.
Will discuss whether we should remove serverless-aws as owner.
7398b18 to
6e98bbd
Compare
quinna-h
left a comment
There was a problem hiding this comment.
LGTM overall for the test files that IDM owns; left one comment about a test that was supposed to prove the request-start event happens before the request span is created, but instead the span marker records when the middleware retrieves the span
6e98bbd to
a251b4d
Compare
a251b4d to
3d8d4ca
Compare
dubloom
left a comment
There was a problem hiding this comment.
Some small comments but overall LGTM
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11ab2d5fe7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…plications Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
Stacked PRs:
Description
This PR provides the framework side of the MicroVM identity-refresh flow. WSGI,
ASGI, and Django publish
web.request.startingbefore they create a root span,passing the request method and normalized path.
The publisher is deliberately product-neutral: it does not import MicroVM
helpers, recognize the MicroVM
/runendpoint, or decide whether to refresh aruntime ID. PR #19939 registers the MicroVM-only listener and owns that policy.
When no product listens for the event, the publisher returns after a single
listener check, before reading the request or normalizing the path.
Two helpers in
trace_utilsown the publishing:dispatch_wsgi_web_request_starting(environ)is used byDDWSGIMiddlewareand Django's
get_responsehook. WSGI and Django share an environ marker sonested layers publish one event. The Django hook also covers Django 3.0 ASGI
requests, which reach the sync
get_responsebecauseget_asgi_applicationis only wrapped on Django 3.1+.
dispatch_asgi_web_request_starting(scope)publishes only from the outer ASGIapplication. It accepts scopes whether or not
pathalready includesroot_path, so mounted or server-prefixed apps do not produce a duplicatedprefix.
Scope: the event is published for the generic WSGI/ASGI middleware and Django.
Frameworks whose automatic instrumentation creates the root span without
DDWSGIMiddleware(Falcon, Bottle, Pyramid, CherryPy, Molten) are out of scopefor this PR and will follow separately.
This implements the requested split in review comment r4146791727.
Reference
Testing
scripts/lint fmtandscripts/lint typingon the changed filesDjango 5.1 (py3.13), including a real
ASGIRequestreachingget_responseRisks
There is no public API change. The only production listener in this stack is
registered by PR #19939 in MicroVM processes, so non-MicroVM requests do not
dispatch this event.
🤖 Generated with Claude Code