chore(http_server): emit pre-request web event - #19817
Conversation
litianningdatadog
added
changelog/no-changelog
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55febfa654
ℹ️ 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".
| ) -> bool: | ||
| parsed = wrapped(*args, **kwargs) | ||
| if parsed: | ||
| core.dispatch(core.WEB_REQUEST_STARTING, (instance.command, instance.path)) |
There was a problem hiding this comment.
Whenever this integration is enabled and parse_request() succeeds, evaluating core.WEB_REQUEST_STARTING raises AttributeError: ddtrace.internal.core exports the dispatch helpers but defines no such attribute, while the event is defined as WebFrameworkEvents.WEB_REQUEST_STARTING.value. This happens before core.dispatch, so every valid request handled by a patched BaseHTTPRequestHandler is aborted instead of continuing.
Useful? React with 👍 / 👎.
| loaded = sys.modules.get("http.server") | ||
| if loaded is not None: | ||
| return loaded | ||
| import http.server |
There was a problem hiding this comment.
When manual patch() runs before http.server has been imported, this branch executes a deferred import, and the helper's comment explains that its structure is intended to work around the circular on-import state. The repository explicitly bans imports inside function bodies as circular-import workarounds and requires the dependency to be restructured, so this should not remain in the new patch module.
AGENTS.md reference: AGENTS.md:L21-L21
Useful? React with 👍 / 👎.
| { | ||
| "implementation": "A", | ||
| "type": "boolean", | ||
| "default": "true" |
There was a problem hiding this comment.
Outside an AWS Lambda MicroVM, PATCH_MODULES["http_server"] is false unless explicitly overridden, but this registry entry advertises an unconditional default of true. Configuration consumers and generated documentation will therefore report the integration as enabled in environments where auto-patching skips it; represent this environment-dependent default as conditional or unknown rather than true.
Useful? React with 👍 / 👎.
| "aiopg": True, | ||
| "aiobotocore": False, | ||
| "httplib": False, | ||
| "http_server": bool(env.get("AWS_LAMBDA_MICROVM_IMAGE_ARN")), |
There was a problem hiding this comment.
Adding http_server to PATCH_MODULES without also adding it to INTEGRATION_CONFIGS leaves ddtrace.config.http_server invalid: applications using the standard integration configuration access receive AttributeError, and any later config._add("http_server", ...) defaults would be logged and ignored. Register the name even though this no-span integration currently has no service setting.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
|
|
||
| .. _http_server: | ||
|
|
||
| http_server |
There was a problem hiding this comment.
This adds the integration's automodule page but omits it from the Supported Libraries table in docs/index.rst, where neighboring gunicorn, httplib, and httpx integrations are discoverable. As a result, users browsing the primary integration inventory cannot find the new page or learn that automatic enablement is MicroVM-specific; add the corresponding index row and conditional-auto-enable note.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
1 participant
Description
Follow-up to #19816. Adds a stdlib
http_serverintegration for rawhttp.serverhandlers that need to emitWEB_REQUEST_STARTINGbefore request handling continues.The integration wraps
http.server.BaseHTTPRequestHandler.parse_requestand dispatches the event with(method, path)after the request line parses successfully. It does not create spans. It is default-enabled like other auto-patched integrations and can be disabled withDD_TRACE_HTTP_SERVER_ENABLED=false; it intentionally has no service-name config.This PR also adds the integration metadata needed for auto-patching, docs discovery, suite selection, and supported-configuration validation. The
.riot/requirements/*.txtfiles are generated locks for the newhttp_serverRiot env across the Python versions selected byselect_pys(). The supported-configuration checker now allows no-span integrations to skipDD_{NAME}_SERVICE[_NAME]entries.This replaces #19799 after renaming the branch from
tianning.li/3-http-server-request-starting-eventtotianning.li/2-1-http-server-request-starting-event.Testing
scripts/lint fmt ddtrace/_monkey.py ddtrace/contrib/internal/http_server/__init__.py ddtrace/contrib/internal/http_server/patch.py ddtrace/internal/settings/_supported_configurations.py riotfile.py scripts/supported_configurations.py tests/contrib/http_server/__init__.py tests/contrib/http_server/test_http_server_patch.py tests/contrib/http_server/test_microvm_identity_refresh.pyscripts/lint style ddtrace/_monkey.py ddtrace/contrib/internal/http_server/__init__.py ddtrace/contrib/internal/http_server/patch.py ddtrace/internal/settings/_supported_configurations.py riotfile.py scripts/supported_configurations.py tests/contrib/http_server/__init__.py tests/contrib/http_server/test_http_server_patch.py tests/contrib/http_server/test_microvm_identity_refresh.pyscripts/lint supported-config-checkscripts/run-tests --venv 13ed954 tests/contrib/http_serverRelease note
No Reno fragment. This is internal runtime-identity plumbing for a no-span integration and does not change trace output by itself. Use
changelog/no-changelog.Additional Notes
Depends on #19816 and #19778.