-
Notifications
You must be signed in to change notification settings - Fork 777
feat: add optional dependencies (extras) for traceloop-sdk integrations, update docs, and require_dependency checks #3241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ns, update docs, and require_dependency checks
Your Name seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThis update introduces an extras-based dependency management system for the traceloop-sdk package. It adds detailed installation instructions to the README files, refactors instrumentor initialization to enforce dependency presence via a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SDK
participant require_dependency
participant InstrumentationPackage
User->>SDK: Initialize instrumentor (e.g., init_openai_instrumentor)
SDK->>require_dependency: require_dependency("openai", "llm")
alt Package installed
require_dependency-->>SDK: Success
SDK->>InstrumentationPackage: Import and instrument
else Package missing
require_dependency-->>SDK: Raise ImportError with pip install instructions
SDK-->>User: Error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to c91e24b in 58 seconds. Click for details.
- Reviewed
861
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
3
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/pyproject.toml:110
- Draft comment:
Nested extras (e.g. 'all' includes keys like 'llm' and 'frameworks') may not be auto-expanded by Poetry. Verify that these nested extra references work as expected. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is asking the PR author to verify that nested extra references work as expected, which falls under the rule of not asking the author to ensure behavior is intended or to double-check things. It doesn't provide a specific suggestion or point out a clear issue.
2. packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py:1027
- Draft comment:
For consistency, consider using require_dependency here (as in other instrumentors) instead of manually checking with is_package_installed. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py:650
- Draft comment:
Ensure that checking only for 'qdrant-client' (via require_dependency) covers all installation variants. Previously both 'qdrant_client' and 'qdrant-client' were checked. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that a change in dependency checking covers all installation variants. This is a request for confirmation of intention, which violates the rules. The comment does not provide a specific suggestion or ask for a specific test to be written.
Workflow ID: wflow_zLZJb7APwlh3zcBQ
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (16)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (16)
589-603
: Same concern for Anthropic instrumentorAdd
require_dependency("opentelemetry-instrumentation-anthropic", "llm")
for a clearer error path.
612-623
: Same pattern for CohereApply the dual-check pattern here as well.
632-643
: Vector-store instrumentors need the extra check
pinecone
client is verified butopentelemetry-instrumentation-pinecone
isn’t.
652-663
: …and likewise for Qdrant.
710-721
: …and Haystack.
730-741
: …and LangChain.
806-817
: …and Together.
826-837
: …and LlamaIndex.
846-857
: …and Milvus.
911-922
: …and Bedrock.
931-942
: …and SageMaker.
968-979
: …and Vertex AI.
988-999
: …and Watsonx.
1008-1019
: …and Weaviate.
1099-1110
: …and Groq.
1119-1130
: …and CrewAI.
🧹 Nitpick comments (3)
packages/traceloop-sdk/README.md (1)
25-33
: Align the “everything” extra with the root READMEThis README uses
[all]
, whereas the root README uses[full]
. Keeping only one public alias (or clearly documenting both) will spare users from trial-and-error.packages/traceloop-sdk/traceloop/sdk/utils/__init__.py (1)
25-47
:require_dependency
should verify the instrumentation package as wellThe helper guarantees that the target library (e.g.
openai
) exists, but the subsequentimport opentelemetry.instrumentation.*
can still fail if the instrumentation extra wasn’t installed.
Either:- if not is_package_installed(package): + missing = [p for p in (package, f"opentelemetry-instrumentation-{package}") + if not is_package_installed(p)] + if missing:or call
require_dependency
twice in the caller for both names.packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
563-577
: Also gate the instrumentation dependency
require_dependency("openai", "llm")
checks the client library but notopentelemetry-instrumentation-openai
. If the user installs onlyopenai
, they’ll receive a less helpfulModuleNotFoundError
later.
Consider a second check:require_dependency("opentelemetry-instrumentation-openai", "llm")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md
(1 hunks)packages/traceloop-sdk/README.md
(1 hunks)packages/traceloop-sdk/pyproject.toml
(1 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py
(8 hunks)packages/traceloop-sdk/traceloop/sdk/utils/__init__.py
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-04T15:23:44.799Z
Learnt from: nina-kollman
PR: traceloop/openllmetry#3219
File: packages/traceloop-sdk/tests/datasets/mock_objects.py:105-106
Timestamp: 2025-08-04T15:23:44.799Z
Learning: In the traceloop-sdk project, directly accessing private attributes like _client is acceptable in test mock objects and utilities, even though it breaks encapsulation principles, because it's isolated to test code and not production code.
Applied to files:
packages/traceloop-sdk/README.md
🧬 Code Graph Analysis (1)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (20)
packages/traceloop-sdk/traceloop/sdk/utils/__init__.py (1)
require_dependency
(28-47)packages/traceloop-sdk/traceloop/sdk/telemetry.py (3)
Telemetry
(12-94)capture
(66-73)log_exception
(75-86)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/__init__.py (1)
OpenAIInstrumentor
(11-54)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/__init__.py (1)
AnthropicInstrumentor
(639-751)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (1)
CohereInstrumentor
(132-168)packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py (1)
PineconeInstrumentor
(182-240)packages/opentelemetry-instrumentation-qdrant/opentelemetry/instrumentation/qdrant/__init__.py (1)
QdrantInstrumentor
(38-64)packages/opentelemetry-instrumentation-haystack/opentelemetry/instrumentation/haystack/__init__.py (1)
HaystackInstrumentor
(43-75)packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py (1)
LangchainInstrumentor
(32-185)packages/opentelemetry-instrumentation-together/opentelemetry/instrumentation/together/__init__.py (1)
TogetherAiInstrumentor
(133-170)packages/opentelemetry-instrumentation-llamaindex/opentelemetry/instrumentation/llamaindex/__init__.py (1)
LlamaIndexInstrumentor
(49-100)packages/opentelemetry-instrumentation-milvus/opentelemetry/instrumentation/milvus/__init__.py (1)
MilvusInstrumentor
(77-147)packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py (1)
BedrockInstrumentor
(557-657)packages/opentelemetry-instrumentation-sagemaker/opentelemetry/instrumentation/sagemaker/__init__.py (1)
SageMakerInstrumentor
(169-213)packages/opentelemetry-instrumentation-vertexai/opentelemetry/instrumentation/vertexai/__init__.py (1)
VertexAIInstrumentor
(301-348)packages/opentelemetry-instrumentation-watsonx/opentelemetry/instrumentation/watsonx/__init__.py (1)
WatsonxInstrumentor
(630-717)packages/opentelemetry-instrumentation-weaviate/opentelemetry/instrumentation/weaviate/__init__.py (1)
WeaviateInstrumentor
(214-246)packages/opentelemetry-instrumentation-marqo/opentelemetry/instrumentation/marqo/__init__.py (1)
MarqoInstrumentor
(44-75)packages/opentelemetry-instrumentation-groq/opentelemetry/instrumentation/groq/__init__.py (1)
GroqInstrumentor
(383-483)packages/opentelemetry-instrumentation-crewai/opentelemetry/instrumentation/crewai/instrumentation.py (1)
CrewAIInstrumentor
(18-54)
"../opentelemetry-instrumentation-cohere", | ||
"../opentelemetry-instrumentation-groq", | ||
"../opentelemetry-instrumentation-ollama", | ||
"../opentelemetry-instrumentation-mistralai", | ||
"../opentelemetry-instrumentation-bedrock", | ||
"../opentelemetry-instrumentation-sagemaker", | ||
"../opentelemetry-instrumentation-replicate", | ||
"../opentelemetry-instrumentation-vertexai", | ||
"../opentelemetry-instrumentation-watsonx", | ||
"../opentelemetry-instrumentation-google-generativeai", | ||
"../opentelemetry-instrumentation-alephalpha" | ||
] | ||
frameworks = [ | ||
"../opentelemetry-instrumentation-langchain", | ||
"../opentelemetry-instrumentation-llamaindex", | ||
"../opentelemetry-instrumentation-crewai", | ||
"../opentelemetry-instrumentation-haystack", | ||
"../opentelemetry-instrumentation-transformers", | ||
"../opentelemetry-instrumentation-together" | ||
] | ||
vectorstores = [ | ||
"../opentelemetry-instrumentation-pinecone", | ||
"../opentelemetry-instrumentation-qdrant", | ||
"../opentelemetry-instrumentation-weaviate", | ||
"../opentelemetry-instrumentation-chromadb", | ||
"../opentelemetry-instrumentation-lancedb", | ||
"../opentelemetry-instrumentation-milvus", | ||
"../opentelemetry-instrumentation-marqo" | ||
] | ||
cloud = [ | ||
"../opentelemetry-instrumentation-bedrock", | ||
"../opentelemetry-instrumentation-vertexai", | ||
"../opentelemetry-instrumentation-sagemaker", | ||
"../opentelemetry-instrumentation-watsonx", | ||
"../opentelemetry-instrumentation-google-generativeai" | ||
] | ||
minimal = [ | ||
"opentelemetry-api", | ||
"opentelemetry-sdk", | ||
"opentelemetry-exporter-otlp-proto-http", | ||
"opentelemetry-exporter-otlp-proto-grpc", | ||
"colorama", | ||
"tenacity", | ||
"pydantic", | ||
"jinja2", | ||
"deprecated", | ||
"posthog", | ||
"aiohttp" | ||
] | ||
all = [ | ||
"llm", | ||
"frameworks", | ||
"vectorstores", | ||
"cloud", | ||
"logging", | ||
"requests", | ||
"sqlalchemy", | ||
"urllib3", | ||
"threading", | ||
"redis", | ||
"semantic-conventions-ai", | ||
"../opentelemetry-instrumentation-openai-agents", | ||
"../opentelemetry-instrumentation-mcp", | ||
"../opentelemetry-instrumentation-groq" | ||
] | ||
logging = ["opentelemetry-instrumentation-logging"] | ||
requests = ["opentelemetry-instrumentation-requests"] | ||
sqlalchemy = ["opentelemetry-instrumentation-sqlalchemy"] | ||
urllib3 = ["opentelemetry-instrumentation-urllib3"] | ||
threading = ["opentelemetry-instrumentation-threading"] | ||
redis = ["opentelemetry-instrumentation-redis"] | ||
semantic-conventions-ai = ["opentelemetry-semantic-conventions-ai"] | ||
mistralai = ["../opentelemetry-instrumentation-mistralai"] | ||
openai = ["../opentelemetry-instrumentation-openai"] | ||
openai-agents = ["../opentelemetry-instrumentation-openai-agents"] | ||
ollama = ["../opentelemetry-instrumentation-ollama"] | ||
anthropic = ["../opentelemetry-instrumentation-anthropic"] | ||
cohere = ["../opentelemetry-instrumentation-cohere"] | ||
crewai = ["../opentelemetry-instrumentation-crewai"] | ||
google-generativeai = ["../opentelemetry-instrumentation-google-generativeai"] | ||
pinecone = ["../opentelemetry-instrumentation-pinecone"] | ||
qdrant = ["../opentelemetry-instrumentation-qdrant"] | ||
langchain = ["../opentelemetry-instrumentation-langchain"] | ||
lancedb = ["../opentelemetry-instrumentation-lancedb"] | ||
chromadb = ["../opentelemetry-instrumentation-chromadb"] | ||
transformers = ["../opentelemetry-instrumentation-transformers"] | ||
together = ["../opentelemetry-instrumentation-together"] | ||
llamaindex = ["../opentelemetry-instrumentation-llamaindex"] | ||
milvus = ["../opentelemetry-instrumentation-milvus"] | ||
haystack = ["../opentelemetry-instrumentation-haystack"] | ||
bedrock = ["../opentelemetry-instrumentation-bedrock"] | ||
sagemaker = ["../opentelemetry-instrumentation-sagemaker"] | ||
replicate = ["../opentelemetry-instrumentation-replicate"] | ||
vertexai = ["../opentelemetry-instrumentation-vertexai"] | ||
watsonx = ["../opentelemetry-instrumentation-watsonx"] | ||
weaviate = ["../opentelemetry-instrumentation-weaviate"] | ||
alephalpha = ["../opentelemetry-instrumentation-alephalpha"] | ||
marqo = ["../opentelemetry-instrumentation-marqo"] | ||
groq = ["../opentelemetry-instrumentation-groq"] | ||
mcp = ["../opentelemetry-instrumentation-mcp"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extras reference local paths – will break for users installing from PyPI
[tool.poetry.extras]
entries like "../opentelemetry-instrumentation-openai"
point to a relative path that will not exist in a regular pip install
scenario. Extras must reference publishable package names (e.g. opentelemetry-instrumentation-openai
) rather than local paths.
This will block installation outside the monorepo.
🤖 Prompt for AI Agents
In packages/traceloop-sdk/pyproject.toml between lines 56 and 160, the
[tool.poetry.extras] section uses local relative paths like
"../opentelemetry-instrumentation-openai" which will break installations from
PyPI. Replace all these local path references with the corresponding publishable
package names such as "opentelemetry-instrumentation-openai" to ensure the
extras work correctly when installed via pip outside the monorepo.
all = [ | ||
"llm", | ||
"frameworks", | ||
"vectorstores", | ||
"cloud", | ||
"logging", | ||
"requests", | ||
"sqlalchemy", | ||
"urllib3", | ||
"threading", | ||
"redis", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Duplicate catch-all extras (all
vs full
)
Maintaining two overlapping aggregates increases maintenance cost and opens the door for drift. Pick one (preferably the one promoted in docs) and drop the other.
🤖 Prompt for AI Agents
In packages/traceloop-sdk/pyproject.toml around lines 110 to 120, there are two
overlapping catch-all extras named 'all' and 'full'. To reduce maintenance
overhead and avoid inconsistencies, choose one of these extras—preferably the
one recommended in the documentation—and remove the other from the file.
Install the SDK with all instrumentations: | ||
|
||
```bash | ||
pip install traceloop-sdk[full] | ||
``` | ||
|
||
Or, to install only the instrumentations you need (for example, OpenAI and LangChain): | ||
|
||
```bash | ||
pip install traceloop-sdk | ||
pip install traceloop-sdk[openai,langchain] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Inconsistent extra name (full
vs all
) creates confusion
README shows pip install traceloop-sdk[full]
, but the package-level README advertises [all]
and the pyproject defines both extras. Consider standardising on a single canonical name (and deprecating the other) to avoid users installing the wrong extra.
🤖 Prompt for AI Agents
In README.md lines 67 to 76, the installation instructions use the extra name
"full" while the package README and pyproject define both "full" and "all"
extras, causing confusion. Standardize on a single extra name by choosing either
"full" or "all" as the canonical name, update the README to use only that name,
and deprecate or remove the other extra from the pyproject and documentation to
prevent user errors.
Behavior:
Documentation:
Misc:
Important
Adds optional dependencies for traceloop-sdk integrations, updates documentation, and enforces dependency checks using
require_dependency
.traceloop-sdk
inpyproject.toml
for selective installation of components likellm
,frameworks
,vectorstores
, andcloud
.require_dependency
inutils/__init__.py
to raiseImportError
if a required optional dependency is missing.tracing.py
to userequire_dependency
for dependency checks.README.md
andpackages/traceloop-sdk/README.md
to include instructions for installing optional dependencies using extras.pyproject.toml
and replaces them with extras.This description was created by
for c91e24b. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Documentation
New Features
Refactor