fix(agentex): only bracket IPv6 literals when building the OTLP metrics URL#323
Closed
parveshsaini wants to merge 2 commits into
Closed
fix(agentex): only bracket IPv6 literals when building the OTLP metrics URL#323parveshsaini wants to merge 2 commits into
parveshsaini wants to merge 2 commits into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Author
|
Hey Team! If the proposed changes looks right, can we have a quick review here please? Thanks! @danielmillerp @RoxyFarhad @smoreinis @MichaelSun48 @declan-scale |
Collaborator
|
thanks for flagging this! should get addressed as part of #346 |
Collaborator
|
closing in favor of 346 |
scale-sf
added a commit
that referenced
this pull request
Jul 7, 2026
## Summary - fix worker OTLP metrics URL construction for normal `DD_AGENT_HOST` values like `localhost`, service names, and IPv4 literals - keep IPv6 literals bracketed and preserve explicit ports - add focused unit coverage for metrics URL construction Closes #313. Supersedes stale external PR #323 with the same underlying fix on a fresh branch. ## Tests - `uv run ruff check agentex/src/temporal/run_worker.py agentex/tests/unit/temporal/test_run_worker_metrics_url.py` - `uv run --package agentex-backend --group test pytest agentex/tests/unit/temporal/test_run_worker_metrics_url.py -m unit -q` - `uv run --package agentex-backend --group test pytest agentex/tests/unit/temporal -m unit -q` - Runtime repro with `temporalio 1.23.0`: old `http://[{DD_AGENT_HOST}]:4317` URLs reject `localhost`, `datadog-agent`, and `10.0.0.5` with `Invalid OTel URL: invalid IPv6 address`; `build_metrics_url(...)` outputs are accepted by `Runtime(telemetry=TelemetryConfig(metrics=OpenTelemetryConfig(url=...)))`. <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes the OTLP metrics URL construction for Temporal workers by replacing the broken `http://[{DD_AGENT_HOST}]:4317` string interpolation with a dedicated `build_metrics_url` function that correctly handles all common `DD_AGENT_HOST` shapes and adds focused unit tests for each case. - `build_metrics_url` correctly handles plain hostnames, IPv4 literals, bare IPv6 addresses (brackets them), pre-bracketed IPv6 (including the previously-identified empty-trailing-port edge case `"[::1]:"`, now covered by a test), and explicit ports for all host types. - One unguarded edge case remains: a malformed input that begins with `"["` but has no closing `"]"` bypasses the inner parsing block and is then double-bracketed by the final colon-presence check, producing an invalid URL like `http://[[::1]:4317`. <details><summary><h3>Confidence Score: 5/5</h3></summary> Safe to merge — the fix correctly resolves the invalid IPv6-bracket wrapping for all realistic DD_AGENT_HOST values and is well-covered by the new unit tests. The core bug (wrapping every host in IPv6 brackets) is definitively fixed and tested across all meaningful host shapes. The one unguarded path — a malformed input that begins with an unclosed bracket — is an unrealistic operator configuration and does not affect production behaviour. No files require special attention; the logic in run_worker.py is straightforward and the test file covers all realistic cases. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex/src/temporal/run_worker.py | Replaces the broken `http://[{DD_AGENT_HOST}]:4317` template with `build_metrics_url`, correctly handling plain hostnames, IPv4, bare IPv6 (brackets them), pre-bracketed IPv6 (strips and re-brackets), and explicit ports. One edge case remains: a malformed input like `"[::1"` (missing closing bracket) bypasses the inner block and then gets double-bracketed by the trailing guard, producing an invalid URL. | | agentex/tests/unit/temporal/test_run_worker_metrics_url.py | New unit tests covering None/empty input, plain hostnames, IPv4, bare IPv6, pre-bracketed IPv6 (including the previously-reported `"[::1]:"` empty-port case), and explicit-port variants. No tests for the malformed `"[::1"` (missing closing bracket) case. | </details> <details><summary><h3>Flowchart</h3></summary> <a href="#gh-light-mode-only"> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[build_metrics_url] --> B{host_url falsy?} B -- Yes --> C[return None] B -- No --> D[host = host_url.strip] D --> E{host empty?} E -- Yes --> C E -- No --> F[port = 4317 default] F --> G{host starts with bracket?} G -- Yes --> H[find closing bracket] H --> I{bracket found?} I -- Yes --> J[parse rest after bracket] J --> K{rest starts with colon?} K -- Yes --> L[port = rest after colon or default] K -- No --> M[port stays default] L --> N[host = inner IPv6 address] M --> N I -- No --> O[host unchanged with leading bracket - BUG PATH] G -- No --> P{exactly one colon in host?} P -- Yes --> Q[split host and port] P -- No --> R[host unchanged] Q --> S{colon in host?} N --> S O --> S R --> S S -- Yes --> T[wrap host in brackets] S -- No --> U[return http://host:port] T --> U ``` </a> <a href="#gh-dark-mode-only"> ```mermaid %%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[build_metrics_url] --> B{host_url falsy?} B -- Yes --> C[return None] B -- No --> D[host = host_url.strip] D --> E{host empty?} E -- Yes --> C E -- No --> F[port = 4317 default] F --> G{host starts with bracket?} G -- Yes --> H[find closing bracket] H --> I{bracket found?} I -- Yes --> J[parse rest after bracket] J --> K{rest starts with colon?} K -- Yes --> L[port = rest after colon or default] K -- No --> M[port stays default] L --> N[host = inner IPv6 address] M --> N I -- No --> O[host unchanged with leading bracket - BUG PATH] G -- No --> P{exactly one colon in host?} P -- Yes --> Q[split host and port] P -- No --> R[host unchanged] Q --> S{colon in host?} N --> S O --> S R --> S S -- Yes --> T[wrap host in brackets] S -- No --> U[return http://host:port] T --> U ``` </a> </details> <sub>Reviews (2): Last reviewed commit: ["fix(agentex): default empty OTLP metrics..."](24de57e) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=42166768)</sub> <!-- /greptile_comment -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
run_workerbuilt the worker's OTLP metrics endpoint by unconditionally wrappingDD_AGENT_HOSTin IPv6 literal brackets:This change extracts a small
build_metrics_urlhelper that brackets the host only when it is an IPv6 literal (i.e. it contains a colon), and uses the host as-is otherwise.Why
Per RFC 3986, brackets are only valid around an IPv6 literal. For the common cases, a hostname (
localhost), a Kubernetes service name (datadog-agent), or an IPv4 literal (10.0.0.5) - the old code produced a malformed URL likehttp://[localhost]:4317.That
metrics_urlflows intoTemporalClientFactory.create_client_from_env(...)→create_client(...)→OpenTelemetryConfig(url=metrics_url). The Temporal SDK validates the URL eagerly when building theRuntime, raisingInvalid OTel URL: invalid IPv6 address. Because that construction is inside thetryblock increate_client, it is re-raised asTemporalConnectionError, so the worker fails to start entirely wheneverDD_AGENT_HOSTis set to anything other than a bare IPv6 literal.This also matches how
DD_AGENT_HOSTis already used elsewhere in the repo without brackets (e.g.app.py'sstatsd_host=os.getenv("DD_AGENT_HOST", "localhost")).How it was verified
Reproduced on Linux (WSL Ubuntu, Python 3.12,
temporalio1.23.0 peruv.lock): feedinghttp://[localhost]:4317/http://[datadog-agent]:4317/http://[10.0.0.5]:4317toOpenTelemetryConfigraisesInvalid OTel URL: invalid IPv6 address; the bracket-less forms are accepted.Added
tests/unit/temporal/test_run_worker_metrics_url.pycovering hostname / IPv4 (not bracketed), IPv6 literal (bracketed), and the unset case (None). The hostname/IPv4 assertions fail against the previous always-bracket behavior and pass with this change.Closes #313
Greptile Summary
build_metrics_urlhelper for constructing the worker OTLP metrics endpoint fromDD_AGENT_HOST.Confidence Score: 5/5
The change is narrowly scoped to OTLP metrics URL construction and is covered by focused unit tests for the relevant host forms.
The helper behavior matches the described URL requirements, and the tests exercise unset, hostname, IPv4, IPv6, explicit port, and already-bracketed IPv6 cases.
What T-Rex did
Reviews (2): Last reviewed commit: "fix(agentex): handle host:port and brack..." | Re-trigger Greptile