fix(chromadb,pinecone,weaviate,lancedb,milvus): record exceptions on error spans#4006
Open
Koushik-Salammagari wants to merge 2 commits intotraceloop:mainfrom
Open
Conversation
…error spans When vector DB calls raise exceptions, spans were left in UNSET state with no error information. Add span.record_exception() and StatusCode.ERROR to all wrapped call sites. Milvus already set ERROR_TYPE attribute; this completes the OTel span status. Fixes traceloop#412
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughInstrumentation wrappers for Chromadb, Lancedb, Milvus, Pinecone, and Weaviate now disable automatic span exception/status handling and explicitly catch exceptions to record them on the active span, set the span status to ERROR, and re-raise the exception. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Wrapper as Instrumentation Wrapper
participant Span as OpenTelemetry Span
participant Backend as Wrapped Service
Note over Wrapper,Span: Wrapper starts span with record_exception=False,\nset_status_on_exception=False
Client->>Wrapper: call instrumented method(args)
Wrapper->>Span: tracer.start_as_current_span(name, record_exception=False, set_status_on_exception=False)
Wrapper->>Backend: invoke wrapped(*args, **kwargs)
alt Backend returns successfully
Backend-->>Wrapper: result
Wrapper->>Span: optionally emit events/attributes
Wrapper-->>Client: return result
else Backend raises exception
Backend-->>Wrapper: throws Exception e
Wrapper->>Span: span.record_exception(e)
Wrapper->>Span: span.set_status(Status(StatusCode.ERROR))
Wrapper-->>Client: re-raises Exception e
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… managers Add record_exception=False and set_status_on_exception=False to all start_as_current_span calls in chromadb, pinecone, weaviate, lancedb, and milvus wrappers to avoid double-recording exceptions that are already captured by the explicit span.record_exception(e) calls.
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.
Summary
Fixes #412 (partial — vector DB instrumentation packages)
When vector DB calls raise exceptions, spans were left in
UNSETstatus with no error information. This PR adds proper error recording to five vector DB packages:wrapper.py): addsStatus, StatusCodeimport + try/except aroundwrapped(*args, **kwargs)__init__.py): adds try/except around the wrapped call inside thewith tracer.start_as_current_span()blockwrapper.py): addsStatus, StatusCodeimport + try/exceptwrapper.py): addsStatus, StatusCodeimport + try/exceptwrapper.py): already had a try/except that setERROR_TYPEattribute — extended it withspan.record_exception(e)andspan.set_status(SpanStatus(StatusCode.ERROR))to complete OTel span status (aliased to avoid conflict with pymilvusStatus)This follows the same approach as #3970 (anthropic/groq/mistralai) and companion PR for LLM instrumentations.
Test plan
status=ERRORwith an attached exception eventstatus=OK(orUNSETwhere OK was never set)npx nx run-many -t test --projects=opentelemetry-instrumentation-chromadb,opentelemetry-instrumentation-pinecone,opentelemetry-instrumentation-weaviate,opentelemetry-instrumentation-lancedb,opentelemetry-instrumentation-milvusSummary by CodeRabbit