Add /otlp/ segment to observability exporter URL paths#229
Add /otlp/ segment to observability exporter URL paths#229
Conversation
Update Agent365Exporter to route traces through the OTLP-aligned endpoint paths for both standard and S2S endpoints. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates Agent365Exporter’s trace export endpoints to include an /otlp/ path segment, aligning URL routes with OTLP naming conventions and adjusting unit tests accordingly.
Changes:
- Updated exporter trace URL construction for both standard and S2S endpoints to include
/otlp/. - Updated unit test assertions to match the new URL format.
- Updated option documentation to reflect the new S2S path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/observability/core/agent365-exporter.test.ts | Updates expected URL assertions to include /otlp/ segment. |
| packages/agents-a365-observability/src/tracing/exporter/Agent365ExporterOptions.ts | Updates JSDoc for useS2SEndpoint path to include /otlp/. |
| packages/agents-a365-observability/src/tracing/exporter/Agent365Exporter.ts | Updates endpoint path construction and class doc to include /otlp/ for trace export routes. |
|
|
||
| const urlArg = fetchCalls[0][0] as string; | ||
| expect(urlArg).toMatch(`/observabilityService/tenants/${tenantId}/agents/${agentId}/traces?api-version=1`); | ||
| expect(urlArg).toMatch(`/observabilityService/tenants/${tenantId}/otlp/agents/${agentId}/traces?api-version=1`); |
There was a problem hiding this comment.
expect(...).toMatch(string) treats the string as a RegExp pattern, so the ? in traces?api-version=1 is interpreted as a regex quantifier (making the preceding character optional) rather than a literal question mark. This can make the assertions pass even when the query string is wrong. Prefer toContain(...) for substring checks, or pass a RegExp that escapes the ? (e.g., \\?).
| expect(fetchCalls.length).toBe(1); | ||
| const urlArg = fetchCalls[0][0] as string; | ||
| expect(urlArg).toMatch(`/observabilityService/tenants/${tenantId}/agents/${agentId}/traces?api-version=1`); | ||
| expect(urlArg).toMatch(`/observabilityService/tenants/${tenantId}/otlp/agents/${agentId}/traces?api-version=1`); |
There was a problem hiding this comment.
expect(...).toMatch(string) treats the string as a RegExp pattern, so the ? in traces?api-version=1 is interpreted as a regex quantifier (making the preceding character optional) rather than a literal question mark. This can make the assertions pass even when the query string is wrong. Prefer toContain(...) for substring checks, or pass a RegExp that escapes the ? (e.g., \\?).
| // Select endpoint path based on S2S flag (includes tenantId in path) | ||
| const endpointRelativePath = | ||
| this.options.useS2SEndpoint | ||
| ? `/observabilityService/tenants/${encodeURIComponent(tenantId)}/agents/${encodeURIComponent(agentId)}/traces` | ||
| : `/observability/tenants/${encodeURIComponent(tenantId)}/agents/${encodeURIComponent(agentId)}/traces`; | ||
| ? `/observabilityService/tenants/${encodeURIComponent(tenantId)}/otlp/agents/${encodeURIComponent(agentId)}/traces` | ||
| : `/observability/tenants/${encodeURIComponent(tenantId)}/otlp/agents/${encodeURIComponent(agentId)}/traces`; |
There was a problem hiding this comment.
The two path templates are nearly identical and differ only by the service prefix (/observabilityService vs /observability). Consider extracting a small helper (or building the prefix separately) to reduce duplication and make future path changes less error-prone.
nikhilNava
left a comment
nikhilNava
left a comment
There was a problem hiding this comment.
Can you add example of this working with DW scenario?
Summary
Agent365Exporterto include/otlp/in the trace export URL paths (both standard and S2S endpoints).../tenants/{tenantId}/otlp/agents/{agentId}/tracesTest plan
🤖 Generated with Claude Code