Split OTLP endpoint path to fix Langfuse/LangSmith integration#4815
Split OTLP endpoint path to fix Langfuse/LangSmith integration#4815ChrisJBurns merged 5 commits intomainfrom
Conversation
The OTLP HTTP SDK's WithEndpoint() only accepts host:port, but users need to include a path for platforms like Langfuse and LangSmith. The path component was being URL-encoded, breaking requests. Parse the endpoint into host:port and path, using WithURLPath() for the path. Fixes #4811 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4815 +/- ##
=======================================
Coverage 69.12% 69.12%
=======================================
Files 530 531 +1
Lines 55157 55170 +13
=======================================
+ Hits 38125 38136 +11
- Misses 14105 14106 +1
- Partials 2927 2928 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ChrisJBurns
left a comment
There was a problem hiding this comment.
Multi-Agent Consensus Review
Agents consulted: site-reliability-engineer, go-expert-developer, code-reviewer, toolhive-expert
Consensus Summary
| # | Finding | Consensus | Severity | Action |
|---|---|---|---|---|
| 1 | splitEndpointPath not defensive against scheme-prefixed endpoints | 7/10 | MEDIUM | Discuss |
| 2 | Missing test case for bare trailing slash endpoint | 9/10 | LOW | Fix |
| 3 | Hardcoded signal path suffixes | 10/10 | LOW | Fix |
| 4 | Removed error tests reduce negative coverage | 10/10 | MEDIUM | Fix |
Overall
This is a well-crafted, focused bug fix with thorough manual verification against Langfuse on Kind. The approach is sound: splitEndpointPath correctly handles the OTLP HTTP SDK's expectation of host:port in WithEndpoint() while preserving custom URL paths via WithURLPath(). Backward compatibility is maintained — existing setups without custom paths are completely unaffected because WithURLPath is only called when basePath != "".
All four agents unanimously agree the core approach is correct. The findings are primarily about test hardening: restoring error-path coverage that was removed (F4, unanimous), adding an edge-case test for bare trailing slashes (F2), and a minor code hygiene suggestion to extract hardcoded OTLP path constants (F3). The one MEDIUM finding about scheme handling (F1) is a pre-existing issue in the CLI path that predates this PR — it's worth a follow-up but should not block merging.
Generated with Claude Code
- Strip http:// and https:// prefixes defensively in splitEndpointPath so the CLI path (which skips NormalizeTelemetryConfig) works correctly - Extract "/v1/traces" and "/v1/metrics" as package-level constants - Add test cases: bare trailing slash, scheme-prefixed endpoints - Restore error-path coverage for metrics exporter (invalid CA cert) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Link to the OTLP/HTTP spec and explain why we need to append these suffixes manually when a custom base path is provided. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
WithEndpoint()only acceptshost:port, but platforms like Langfuse and LangSmith require a custom base path in the endpoint URL. When users configure an endpoint likehttps://cloud.langfuse.com/api/public/otel, the path component gets URL-encoded (slashes become%2F), breaking all requests to these backends.host:portand path components using a newsplitEndpointPathhelper, then pass the host toWithEndpoint()and the path (with signal suffix appended) toWithURLPath().Fixes #4811
Type of change
Test plan
task test)task lint-fix)Manual verification with Langfuse on Kind
Deployed a full Langfuse v3 stack (PostgreSQL, ClickHouse, Redis, MinIO) into a Kind cluster alongside the ToolHive operator built from this branch. Created an
MCPTelemetryConfigwith the exact scenario from the bug report — an endpoint with a custom path:Before the fix, the proxy runner would log:
After the fix, traces export silently (no error), and OTLP trace JSON files appear in MinIO:
The trace payload contains correct span data with
service.name: "echo-server", MCP-specific attributes (mcp.server.name,mcp.transport), and standard OTEL attributes.Full reproduction steps
Prerequisites
kind,kubectl,task,helminstalled1. Create cluster and deploy operator
2. Create namespaces
3. Deploy Langfuse stack
Apply each manifest and wait for readiness:
4. Deploy MCPServer with Langfuse telemetry
5. Send traffic and verify
6. Teardown
Manifest: postgres.yaml
Manifest: redis.yaml
Manifest: clickhouse.yaml
Manifest: minio.yaml
Manifest: langfuse.yaml
Manifest: mcp-with-langfuse.yaml
Changes
pkg/telemetry/providers/otlp/endpoint.gosplitEndpointPathhelper that separateshost:portfrom pathpkg/telemetry/providers/otlp/endpoint_test.gopkg/telemetry/providers/otlp/tracing.gosplitEndpointPath, addWithURLPath(basePath+"/v1/traces")pkg/telemetry/providers/otlp/metrics.gosplitEndpointPath, addWithURLPath(basePath+"/v1/metrics")pkg/telemetry/providers/otlp/tracing_test.gopkg/telemetry/providers/otlp/metrics_test.goDoes this introduce a user-facing change?
Yes. Users can now configure OTLP endpoints with custom base paths (e.g.,
https://cloud.langfuse.com/api/public/otel) in bothMCPTelemetryConfigCRDs and inline telemetry config. Previously, any path in the endpoint was URL-encoded, breaking integration with Langfuse, LangSmith, and collectors behind reverse proxies with path prefixes.Implementation plan
Approved implementation plan
splitEndpointPathhelper inpkg/telemetry/providers/otlp/endpoint.gothat splits a scheme-stripped endpoint intohost:portand base pathtracing.go, callsplitEndpointPathand conditionally addWithURLPath(basePath+"/v1/traces")metrics.go, same treatment withWithURLPath(basePath+"/v1/metrics")Key SDK details:
WithURLPath()replaces the entire URL path (does not append to default), so we must manually constructbasePath + "/v1/traces"andbasePath + "/v1/metrics". The SDK'scleanPath()normalizes the path afterward.Generated with Claude Code