-
Notifications
You must be signed in to change notification settings - Fork 844
fix(pinecone):PackageFix #3502
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?
fix(pinecone):PackageFix #3502
Conversation
WalkthroughThe Pinecone instrumentation is updated to use the renamed "pinecone" package instead of "pinecone-client," with the upper version bound removed. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
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 cdac7b0 in 1 minute and 10 seconds. Click for details.
- Reviewed
25lines of code in2files - Skipped
1files when reviewing. - Skipped posting
2draft 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/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py:35
- Draft comment:
Changed _instruments from 'pinecone-client' with an upper bound to 'pinecone >= 2.2.2'. Ensure this module name matches the actual import (it uses 'import pinecone') and that dropping the upper bound (<6) is intentional to avoid future incompatibilities. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. packages/opentelemetry-instrumentation-pinecone/pyproject.toml:40
- Draft comment:
Added httpx as a test dependency with version '>=0.27.0,<0.28.0'. Verify that this constraint is aligned with your testing needs and does not conflict with other dependencies. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is about a dependency change, specifically adding a new test dependency with a version constraint. The comment asks the PR author to verify alignment with testing needs and check for conflicts, which is not allowed according to the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.
Workflow ID: wflow_XLaAltmKUQ0IW8vJ
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: 0
🧹 Nitpick comments (2)
packages/opentelemetry-instrumentation-pinecone/pyproject.toml (1)
40-48: Align Pinecone package naming between instrumentation metadata and pyprojectAdding
httpxas a test-only dependency with>=0.27.0,<0.28.0looks fine, but there’s now an inconsistency around the Pinecone client:
PineconeInstrumentor.instrumentation_dependencies()now returns_instruments = ("pinecone >= 2.2.2",)(module file), i.e., it advertises a requirement on a package namedpinecone.- This
pyproject.tomlstill usespinecone-clientboth in test deps (Line 47) and in theinstrumentsextra (Line 54).If the official client package has moved from
pinecone-clienttopinecone, it would be good to:
- Update test deps and/or extras to use the same package name as
_instruments, or- Explicitly confirm that
pinecone-clientis still the correct package and adjust_instrumentsaccordingly.Otherwise, users relying on extras or automated discovery of instrumentation dependencies may see conflicting or misleading requirements.
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py (1)
35-35: Reassess_instrumentsconstraint: package name change and removed upper boundChanging
_instrumentsto("pinecone >= 2.2.2",)does two things at once:
- Renames the advertised dependency from
pinecone-clienttopinecone.- Drops the previous major-version upper bound (
<6per summary), so this instrumentation now claims compatibility with all future major versions of the client.Given that:
pyproject.tomlstill referencespinecone-clientin deps/extras, and- Future Pinecone client majors may introduce breaking API changes that this instrumentation doesn’t yet handle,
it may be safer to:
- Keep an explicit upper bound (e.g.,
("pinecone >= 2.2.2, <7",)) until compatibility with newer majors is confirmed, and- Align the package name here with whatever you decide in
pyproject.toml(eitherpineconeorpinecone-client, but consistently).This keeps dependency metadata honest and reduces the risk of silent breakage when a new major client release lands.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
packages/opentelemetry-instrumentation-pinecone/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py(1 hunks)packages/opentelemetry-instrumentation-pinecone/pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
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: 1
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-pinecone/tests/test_query.py (1)
151-179: Consider de-duplicating the repeatedserver.addressnotes in metrics loopThe three identical comments about
server.addressnot being present in metrics are accurate but a bit repetitive. You could move a single explanatory comment just above the metrics loop instead of repeating it inside each metric branch, keeping the test slightly leaner without losing clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
packages/opentelemetry-instrumentation-pinecone/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-pinecone/pyproject.toml(1 hunks)packages/opentelemetry-instrumentation-pinecone/tests/test_query.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-pinecone/tests/test_query.py
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-pinecone/tests/test_query.py (1)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
Meters(36-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
🔇 Additional comments (1)
packages/opentelemetry-instrumentation-pinecone/tests/test_query.py (1)
103-111: Clarifying comment for droppedserver.addresscheck looks goodReplacing the
server.addressassertion with a brief note keeps the test aligned with the newer Pinecone client behavior while preserving the rest of the span-attribute coverage. No changes requested here.
| [tool.poetry.group.test.dependencies] | ||
| openai = { version = "^1.31.1", extras = ["datalib"] } | ||
| openai = "^1.57.0" | ||
| pytest = "^8.2.2" | ||
| pytest-sugar = "1.0.0" | ||
| vcrpy = "^6.0.1" | ||
| pytest-recording = "^0.13.1" | ||
| opentelemetry-sdk = "^1.38.0" | ||
| opentelemetry-instrumentation-openai = { path = "../opentelemetry-instrumentation-openai", develop = true } | ||
| pinecone-client = "^5.0.0" | ||
| pinecone = "^5.0.0" | ||
|
|
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.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/opentelemetry-instrumentation-pinecone/pyproject.tomlRepository: traceloop/openllmetry
Length of output: 186
🌐 Web query:
pinecone package pypi distribution name history pinecone-client
💡 Result:
Short summary — PyPI distribution name history for Pinecone's Python SDK:
- Originally published on PyPI as "pinecone-client". [1]
- The project was renamed to "pinecone" starting with version 5.1.0; "pinecone-client" is marked deprecated and users are instructed to uninstall "pinecone-client" and install "pinecone". [2][3]
- Official docs and release notes reiterate the rename and upgrading steps (uninstall pinecone-client → pip install pinecone). [2][3]
Sources:
- "pinecone-client" project page on PyPI. [1]
- "pinecone" project page on PyPI (rename/deprecation and install instructions). [2]
- Pinecone Python SDK upgrading / GitHub release notes describing the rename (beginning v5.1.0). [3]
🌐 Web query:
pinecone python sdk version 5.0.0 release date pypi history
💡 Result:
- pinecone-client (PyPI) version 5.0.0 — uploaded (release) date: July 19, 2024. [1]
- Subsequent nearby releases: pinecone-client 5.0.1 uploaded Aug 1, 2024. The package was later renamed from pinecone-client → pinecone (SDK package rename started at v5.1.0). [1][2][3]
Sources: PyPI project pages for pinecone-client 5.0.0 and 5.0.1, and the pinecone (SDK) PyPI project. [1][2][3]
References
[1] PyPI package page — pinecone-client 5.0.0 (upload date Jul 19, 2024).
[2] PyPI package page — pinecone-client 5.0.1 (upload date Aug 1, 2024).
[3] PyPI package page — pinecone (SDK) (notes about rename to pinecone starting v5.1.0).
Fix pinecone dependency specification and align instruments extra
The test dependency pinecone = "^5.0.0" is misconfigured: version 5.0.0 was released under pinecone-client, not pinecone. The pinecone distribution name only exists from v5.1.0 onward (after the SDK rename). This constraint will fail to resolve.
Additionally, the instruments extra still references "pinecone-client", creating inconsistency with both the test dependencies and the instrumentor's expectations.
Fix both issues:
- Update test dependencies to
pinecone = "^5.1.0"(orpinecone-client = "^5.0.0"if older versions are required) - Update
[tool.poetry.extras]toinstruments = ["pinecone"]to align with the new distribution name
🤖 Prompt for AI Agents
In packages/opentelemetry-instrumentation-pinecone/pyproject.toml around lines
38 to 47, the test dependency pins `pinecone = "^5.0.0"` and the `instruments`
extra references `pinecone-client`, which is inconsistent and fails resolution
because version 5.0.0 was published under `pinecone-client` and the `pinecone`
distribution exists from 5.1.0 onward; update the test dependency to `pinecone =
"^5.1.0"` (or change to `pinecone-client = "^5.0.0"` if you must keep older SDK
naming) and change the `[tool.poetry.extras]` entry so `instruments =
["pinecone"]` to match the instrumentor and the package distribution name.
| data_point.attributes["server.address"] | ||
| == "https://gen-qa-openai-fast-90c5d9e.svc.gcp-starter.pinecone.io" | ||
| ) | ||
| # Note: server.address not available in Pinecone v5+ metrics |
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.
why do we need this comment?
Can we get the server.address from somewhere else?
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.
@nirga modified the instrumentation to fetch server.address reverted the test case changes now test passed.
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: 2
♻️ Duplicate comments (1)
packages/opentelemetry-instrumentation-pinecone/pyproject.toml (1)
46-46: Pinecone version constraint will fail to resolve.As noted in the previous review,
pinecone = "^5.0.0"is incorrect because version 5.0.0 was published under thepinecone-clientdistribution. Thepineconepackage only exists from version 5.1.0 onwards. This constraint will fail during dependency resolution.Apply this diff to fix the version constraint:
-pinecone = "^5.0.0" +pinecone = "^5.1.0"
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py (1)
82-85: Consider removing unusedkwargsparameter.The
kwargsparameter is not used in the function body. If it's not needed for API consistency with other similar functions, consider removing it to clean up the signature.Based on learnings, as per static analysis hints...
Apply this diff if
kwargsis truly unnecessary:-def _set_input_attributes(span, instance, kwargs): +def _set_input_attributes(span, instance): server_address = _get_server_address(instance) if server_address: set_span_attribute(span, SpanAttributes.SERVER_ADDRESS, server_address)Note: You'll also need to update the call site at line 159 to remove the
kwargsargument.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
packages/opentelemetry-instrumentation-pinecone/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py(3 hunks)packages/opentelemetry-instrumentation-pinecone/pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py
🧠 Learnings (1)
📚 Learning: 2025-12-02T21:09:48.690Z
Learnt from: duanyutong
Repo: traceloop/openllmetry PR: 3487
File: packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py:177-178
Timestamp: 2025-12-02T21:09:48.690Z
Learning: The opentelemetry-instrumentation-openai and opentelemetry-instrumentation-openai-agents packages must remain independent and not share code, so code duplication between them is acceptable.
Applied to files:
packages/opentelemetry-instrumentation-pinecone/pyproject.toml
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py (1)
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/utils.py (2)
dont_throw(7-29)set_span_attribute(32-36)
🪛 Ruff (0.14.8)
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py
82-82: Unused function argument: kwargs
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Lint
- GitHub Check: Test Packages (3.12)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.11)
🔇 Additional comments (3)
packages/opentelemetry-instrumentation-pinecone/pyproject.toml (1)
53-53: Instruments extra correctly updated, but depends on fixing the version constraint.The update from
["pinecone-client"]to["pinecone"]correctly reflects the package rename. However, this change depends on fixing the version constraint at line 46 to^5.1.0to ensure proper dependency resolution.packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py (2)
72-78: LGTM! Clean server address extraction.The
_get_server_addresshelper function is well-implemented:
- Safely checks for attribute existence using
hasattr- Correctly handles URLs with and without protocol prefixes
- Returns None when attributes are missing
- Protected by the
@dont_throwdecorator to prevent instrumentation failures
164-166: LGTM! Consistent use of the new helper.The update to use
_get_server_address(instance)for shared attributes is correct and consistent with the changes in_set_input_attributes. The None check ensures the attribute is only added when available.
| logger = logging.getLogger(__name__) | ||
|
|
||
| _instruments = ("pinecone-client >= 2.2.2, <6",) | ||
| _instruments = ("pinecone >= 2.2.2",) |
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.
Critical version mismatch: pinecone 2.2.2 doesn't exist.
The constraint "pinecone >= 2.2.2" is incorrect because the pinecone package only exists from version 5.1.0 onwards. Version 2.2.2 was released under pinecone-client, not pinecone. This creates a critical inconsistency:
- This line specifies
pinecone >= 2.2.2 - Line 46 in pyproject.toml specifies
pinecone = "^5.0.0"(which is also incorrect; should be^5.1.0)
The instrumentor will fail dependency validation because pinecone 2.2.2 doesn't exist.
Apply this diff to align with the correct pinecone package version range:
-_instruments = ("pinecone >= 2.2.2",)
+_instruments = ("pinecone >= 5.1.0",)🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-pinecone/opentelemetry/instrumentation/pinecone/__init__.py
around line 35, the _instruments tuple incorrectly requires "pinecone >= 2.2.2"
(a version that does not exist for the pinecone package); update the instrument
requirement to the actual package range starting at 5.1.0 (e.g., "pinecone >=
5.1.0") and also update the dependency in pyproject.toml (around line 46) from
pinecone = "^5.0.0" to pinecone = "^5.1.0" so both the instrument metadata and
project dependency match the correct package versions.
|
|
||
| [tool.poetry.group.test.dependencies] | ||
| openai = { version = "^1.31.1", extras = ["datalib"] } | ||
| openai = "^1.57.0" |
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.
🧩 Analysis chain
🌐 Web query:
openai python package version 1.57.0 latest release
💡 Result:
No — 1.57.0 is not the latest. The openai Python package latest release is 2.9.0 (uploaded Dec 4, 2025). [1][2]
Sources:
- PyPI package page (current release 2.9.0, uploaded Dec 4, 2025). [1]
- Historical info for older versions (1.57.0 referenced). [2]
🏁 Script executed:
# Check if there's context in the repo about this change
cd packages/opentelemetry-instrumentation-pinecone
cat -n pyproject.toml | head -50Repository: traceloop/openllmetry
Length of output: 186
🏁 Script executed:
# Look for any usage of datalib in the pinecone instrumentation tests
find packages/opentelemetry-instrumentation-pinecone -type f \( -name "*.py" \) -exec grep -l "datalib" {} \;Repository: traceloop/openllmetry
Length of output: 186
🏁 Script executed:
# Check git history for this change if available
git log --oneline -n 20 -- packages/opentelemetry-instrumentation-pinecone/pyproject.toml 2>/dev/null || echo "Git history not available"Repository: traceloop/openllmetry
Length of output: 186
Consider updating OpenAI version and verify extras removal.
The OpenAI dependency is pinned to version 1.57.0, which is not the latest release (current version is 2.9.0 as of December 4, 2025). Additionally, the datalib extra was removed during this update. Verify that:
- Version
1.57.0is intentionally pinned (not outdated), or consider upgrading to the latest stable version - Removing the
datalibextra does not break Pinecone instrumentation tests or required functionality
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-pinecone/pyproject.toml lines 39-39: the
OpenAI dependency is pinned to openai = "1.57.0" and the datalib extra was
removed; either confirm the pin is intentional or update to a supported recent
release (e.g., latest stable 2.x), and if upgrading reintroduce any needed
extras (datalib) or adjust code for breaking API changes; after changing the
version, run the Pinecone instrumentation unit/integration tests and CI to
ensure nothing breaks, and if tests fail, either revert to a compatible OpenAI
version or restore the datalib extra and adapt code to match the chosen OpenAI
release.
|
@nirga please review it , I have addressed your comments . Thanks ! |
feat(instrumentation): ...orfix(instrumentation): ....Important
Update Pinecone package dependency and add
httpxto test dependencies._instrumentsin__init__.pyto requirepinecone >= 2.2.2.httpxdependency inpyproject.tomlfor test group.This description was created by
for cdac7b0. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.