-
Notifications
You must be signed in to change notification settings - Fork 787
feat: add optional dependencies (extras) for traceloop-sdk integration… #3240
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 restructures the SDK's dependency management by moving instrumentation packages from required to optional, organized as pip extras in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SDK
participant require_dependency
participant Instrumentor
User->>SDK: Calls init_X_instrumentor()
SDK->>require_dependency: Check for package presence
alt Package installed
require_dependency-->>SDK: Success
SDK->>Instrumentor: Import and initialize
Instrumentor-->>SDK: Instrumentation applied
else Package missing
require_dependency-->>SDK: Raise ImportError with install instructions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 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 1 minute and 7 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/traceloop/sdk/utils/__init__.py:2
- Draft comment:
Consider that the recursive implementation incameltosnake
may hit recursion limits for very long strings. An iterative or regex-based approach could improve robustness. - Reason this comment was not posted:
Comment was on unchanged code.
2. packages/traceloop-sdk/traceloop/sdk/utils/__init__.py:28
- Draft comment:
Therequire_dependency
error messages are clear; consider adding a pointer to the docs or an explicit pip install command for easier troubleshooting. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The current error message is already clear and actionable - it tells users exactly what package is missing and how to install it via the extra. Adding pip install syntax and docs link would be nice-to-have but not essential. The comment is about changed code but falls into the category of being a minor enhancement rather than fixing a real issue. The suggested additions could help some users troubleshoot more quickly. Documentation links are generally valuable. While helpful, the current message is already sufficient and clear. The docstring provides examples and the error message gives the key information needed. Delete the comment as it suggests a minor enhancement to an already clear and functional error message, rather than fixing a real issue.
3. packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py:819
- Draft comment:
Typo: The error message 'Error initializing TogetherAI instrumentor:' uses 'TogetherAI' which is inconsistent with the naming in the code ('TogetherAiInstrumentor'). Consider updating it to 'TogetherAi' for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_IIj2MFNayWZ5GLaP
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
🧹 Nitpick comments (4)
packages/traceloop-sdk/traceloop/sdk/utils/__init__.py (1)
25-47
: Expose a clear return type forrequire_dependency
.Add
-> None
to the function signature so type-checkers don’t assume an implicitAny
.-def require_dependency(package: str, extra: str = None): +def require_dependency(package: str, extra: str | None = None) -> None:packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
563-577
: Consolidate dependency-guard pattern across instrumentors.
Each function now repeats
from traceloop.sdk.utils import require_dependency
followed byrequire_dependency("<pkg>", "<extra>")
.
Importing the helper 15× adds clutter; import it once at module top (it’s a lightweight noop).Several instrumentors (Chroma, Google GenerativeAI, Transformers, etc.) still rely on
is_package_installed
instead of the new guard, leaving the enforcement strategy inconsistent. Pick one approach (preferablyrequire_dependency
) for all init_* helpers.Unifying this will shorten each function and give callers a predictable failure mode.
Also applies to: 589-603, 612-627, 632-647, 652-667, 710-725, 730-745, 806-821, 826-841, 846-861, 911-926, 931-946, 968-983, 988-1003, 1008-1023, 1099-1114, 1119-1134
packages/traceloop-sdk/README.md (1)
27-33
: Extra name mismatch (all
vsfull
).The command suggests
pip install traceloop-sdk[all]
, butpyproject.toml
defines the comprehensive extra asfull
. Rename in docs or add anall
alias to avoid user confusion.README.md (1)
67-74
: Optional: show core-only install for minimal footprintSince extras are now optional, adding a core-only install example helps users who just need the SDK without instrumentations.
Proposed insertion above the “all instrumentations” example:
+Install the SDK core only (no extra instrumentations): + +```bash +pip install traceloop-sdk +``` + Install the SDK with all instrumentations: ```bash -pip install traceloop-sdk[full] +pip install "traceloop-sdk[full]"</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between feb570b0903bdfe17f8402774fbbd9122a553067 and c91e24b5827faa34d0f6b551b99014e8a0780481. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `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) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-08-04T15:23:44.799Z</summary>
Learnt from: nina-kollman
PR: #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` </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>README.md (1)</summary> `67-77`: **Extras keys verified in pyproject.toml** All documented extras (`full`, `openai`, `langchain`) are defined exactly as such in `packages/traceloop-sdk/pyproject.toml`. No changes needed. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
"../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 undeclared path dependencies – build will fail.
[tool.poetry.extras]
lists entries such as
"../opentelemetry-instrumentation-openai"
but none of these path packages are declared in [tool.poetry.dependencies]
with optional = true
.
Poetry (and pip) resolve extras only against dependencies already present; referencing raw paths here causes resolution errors during poetry build
or pip install traceloop-sdk[full]
.
Add each instrumentation as an optional dependency first, then reference its name (not the path) in the extras list, e.g.:
[tool.poetry.dependencies]
opentelemetry-instrumentation-openai = {path = "../opentelemetry-instrumentation-openai", optional = true}
[tool.poetry.extras]
openai = ["opentelemetry-instrumentation-openai"]
llm = ["openai", ...]
Without this fix, publishing the wheel or installing with extras will error out.
🤖 Prompt for AI Agents
In packages/traceloop-sdk/pyproject.toml between lines 57 and 160, the extras
section references path dependencies directly without declaring them as optional
dependencies in the [tool.poetry.dependencies] section. To fix this, add each
instrumentation package as an optional dependency under
[tool.poetry.dependencies] with its path and optional = true, then update the
extras lists to reference these dependency names instead of raw paths. This
ensures proper resolution during build and installation.
```bash | ||
pip install traceloop-sdk[full] | ||
``` |
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.
Quote extras to avoid shell globbing and PowerShell parsing issues
Unquoted extras can break in zsh/bash (globbing of [..]) and PowerShell. Recommend quoting for cross-shell compatibility.
Apply:
-```bash
-pip install traceloop-sdk[full]
-```
+```bash
+pip install "traceloop-sdk[full]"
+```
🤖 Prompt for AI Agents
In README.md around lines 69 to 71, the pip install command uses unquoted extras
which can cause shell globbing issues in zsh/bash and parsing problems in
PowerShell. To fix this, add double quotes around the extras part in the pip
install command, changing pip install traceloop-sdk[full] to pip install
"traceloop-sdk[full]". Also ensure the code block is properly fenced with triple
backticks and the bash language identifier.
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
Also quote selective extras; add a brief cross-shell note
Same quoting concern applies here. Suggest a short note that quoting is required in PowerShell/zsh.
Apply:
-Or, to install only the instrumentations you need (for example, OpenAI and LangChain):
+Or, to install only the instrumentations you need (for example, OpenAI and LangChain):
+Tip: quote extras for zsh/PowerShell compatibility.
```bash
-pip install traceloop-sdk[openai,langchain]
+pip install "traceloop-sdk[openai,langchain]"
<details>
<summary>🤖 Prompt for AI Agents</summary>
In README.md around lines 73 to 77, the pip install command for selective extras
lacks quotes, which can cause issues in shells like PowerShell and zsh. Fix this
by adding double quotes around the package and extras list, changing pip install
traceloop-sdk[openai,langchain] to pip install
"traceloop-sdk[openai,langchain]". Also, add a brief note explaining that
quoting is necessary in PowerShell and zsh to avoid shell interpretation errors.
</details>
<!-- fingerprinting:phantom:triton:capybara -->
<!-- This is an auto-generated comment by CodeRabbit -->
update docs, and require_dependency checks
feat(instrumentation): ...
orfix(instrumentation): ...
.Important
Adds optional dependencies for
traceloop-sdk
to allow selective installation and updates documentation and code to support this feature.traceloop-sdk
inpyproject.toml
to allow selective installation of components likellm
,frameworks
,vectorstores
, andcloud
.require_dependency
function 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
Chores
Refactor