Commit 38f917b
authored
fix(agentex): build valid OTLP metrics URLs (#346)
## 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 -->1 parent 648e81c commit 38f917b
2 files changed
Lines changed: 68 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
51 | 76 | | |
52 | 77 | | |
53 | 78 | | |
| |||
93 | 118 | | |
94 | 119 | | |
95 | 120 | | |
96 | | - | |
| 121 | + | |
97 | 122 | | |
98 | 123 | | |
99 | 124 | | |
| |||
Lines changed: 42 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
0 commit comments