April 21st Ishaan Branch - #26213
Conversation
…26183) * fix(otel): preserve Splunk Observability Cloud trace OTLP URL Splunk ingest uses /v2/trace/otlp; _normalize_otel_endpoint must not append /v1/traces. - Return trace endpoints unchanged when they match Splunk OTLP path patterns - Add unit tests for observability.splunkcloud.com, signalfx.com, and /trace/otlp suffix - Set OTEL_EXPORTER_OTLP_PROTOCOL in protocol selection tests (from_env precedence over OTEL_EXPORTER) Made-with: Cursor * test(otel): use parameterized.expand for Splunk OTLP URL cases Made-with: Cursor * fix(otel): narrow Splunk trace URL guard to /v2/trace/otlp only Made-with: Cursor * test(otel): cover OTEL_EXPORTER fallback when OTLP protocol env unset Made-with: Cursor
Greptile SummaryThis PR fixes Splunk Observability Cloud OTLP trace ingestion by adding a guard in Confidence Score: 5/5Safe to merge; all findings are P2 style suggestions that do not affect correctness. The core fix is a small, focused 4-line guard with good parameterized test coverage. The model pricing entry is additive. Remaining comments are minor style nits (substring vs path-anchor match, redundant pop/restore pattern) that don't impact runtime behavior. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Adds a narrow guard in _normalize_otel_endpoint to skip path-rewriting for Splunk /v2/trace/otlp trace endpoints. |
| model_prices_and_context_window.json | Adds openrouter/anthropic/claude-opus-4.7 pricing and capability entry with standard fields. |
| tests/test_litellm/integrations/test_opentelemetry.py | Adds parameterized Splunk URL tests and two new fallback tests; uses manual os.environ.pop inside test body for env isolation which is slightly fragile. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[_normalize_otel_endpoint called] --> B{endpoint empty?}
B -- yes --> C[return endpoint unchanged]
B -- no --> D{valid signal_type?}
D -- no --> E[log warning, return unchanged]
D -- yes --> F[rstrip trailing slash]
F --> G{signal_type == traces AND /v2/trace/otlp in endpoint?}
G -- yes --> H[return endpoint unchanged - Splunk guard]
G -- no --> I{already ends with /v1/signal_type?}
I -- yes --> J[return endpoint unchanged]
I -- no --> K{ends with other signal path?}
K -- yes --> L[replace with target path]
K -- no --> M[append /v1/signal_type]
Reviews (2): Last reviewed commit: "Add Openrouter Opus 4.7 Entry (#26130)" | Re-trigger Greptile
| @@ -1339,7 +1369,7 @@ def test_protocol_selection_from_environment_http(self): | |||
| @patch.dict( | |||
| os.environ, | |||
| { | |||
| "OTEL_EXPORTER": "otlp_grpc", | |||
| "OTEL_EXPORTER_OTLP_PROTOCOL": "grpc", | |||
| "OTEL_EXPORTER_OTLP_ENDPOINT": "http://collector:4317", | |||
| }, | |||
| clear=False, | |||
| @@ -1360,6 +1390,60 @@ def test_protocol_selection_from_environment_grpc(self): | |||
| self.assertIsInstance(processor, BatchSpanProcessor) | |||
| self.assertIsInstance(processor.span_exporter, OTLPSpanExporterGRPC) | |||
There was a problem hiding this comment.
Existing tests changed from
OTEL_EXPORTER to OTEL_EXPORTER_OTLP_PROTOCOL
test_protocol_selection_from_environment_http and test_protocol_selection_from_environment_grpc previously verified that OTEL_EXPORTER=otlp_http/otlp_grpc selects the correct exporter. They now test OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf/grpc instead — a different env var and a different code path. The original OTEL_EXPORTER behaviour is now only covered by the new *_fallback_* tests (which manually pop the protocol key). While coverage is preserved, the existing test names (test_protocol_selection_from_environment_*) no longer reflect what they test. If OTEL_EXPORTER handling regressed silently, these renamed-in-place tests would not catch it at first glance. Consider keeping the original env var in these tests or renaming them to reflect the new intent.
Rule Used: What: Flag any modifications to existing tests and... (source)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
* fix(otel): preserve Splunk Observability Cloud trace OTLP endpoint (BerriAI#26183) * fix(otel): preserve Splunk Observability Cloud trace OTLP URL Splunk ingest uses /v2/trace/otlp; _normalize_otel_endpoint must not append /v1/traces. - Return trace endpoints unchanged when they match Splunk OTLP path patterns - Add unit tests for observability.splunkcloud.com, signalfx.com, and /trace/otlp suffix - Set OTEL_EXPORTER_OTLP_PROTOCOL in protocol selection tests (from_env precedence over OTEL_EXPORTER) Made-with: Cursor * test(otel): use parameterized.expand for Splunk OTLP URL cases Made-with: Cursor * fix(otel): narrow Splunk trace URL guard to /v2/trace/otlp only Made-with: Cursor * test(otel): cover OTEL_EXPORTER fallback when OTLP protocol env unset Made-with: Cursor * Add Openrouter Opus 4.7 Entry (BerriAI#26130) --------- Co-authored-by: milan-berri <milan@berri.ai> Co-authored-by: Matt Greathouse <matt5316@gmail.com>
…26183)
Splunk ingest uses /v2/trace/otlp; _normalize_otel_endpoint must not append /v1/traces.
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes