fix(google-generativeai): defer google-generativeai to allow omission from runtime dependencies#3127
fix(google-generativeai): defer google-generativeai to allow omission from runtime dependencies#3127timkpaine wants to merge 2 commits intotraceloop:mainfrom
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 136f51f in 46 seconds. Click for details.
- Reviewed
20lines of code in1files - Skipped
0files 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-google-generativeai/pyproject.toml:27
- Draft comment:
Added google-generativeai as a runtime dependency to fix the ModuleNotFoundError. Confirm that ^0.8.3 meets your requirements. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/opentelemetry-instrumentation-google-generativeai/pyproject.toml:41
- Draft comment:
Removing google-generativeai from the dev dependencies is appropriate since it’s needed at runtime. Ensure tests don’t require a separate dev-only version. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_zTK6HvfkbPlPfH6X
You can customize 
timkpaine
changed the title
WalkthroughIntroduced TYPE_CHECKING-guarded import for GenerateContentResponse and replaced runtime type annotations with forward-references; added a test ensuring importing the instrumentation package does not eagerly load Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
timkpaine
force-pushed
the
patch-1
branch
2 times, most recently
from
f10b170 to
f659a66
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py (1)
169-174: Fix F401 and make the test robust against cross-test contaminationThe import is intentionally for side effects but triggers Ruff/Flake8 F401. Also, if any other test imports google.generativeai, this test may become flaky. Import via importlib and clear any pre-existing module entry.
Apply this diff:
def test_imports(): # Ensure heavy dependencies are not accidentally imported - import opentelemetry.instrumentation.google_generativeai - - assert "google.generativeai" not in sys.modules + # Clear any pre-existing module to avoid cross-test contamination + sys.modules.pop("google.generativeai", None) + # Import the instrumentor for side effects without tripping F401 + import importlib + importlib.import_module("opentelemetry.instrumentation.google_generativeai") + + assert "google.generativeai" not in sys.modulesThis resolves Ruff/Flake8 F401 and improves test isolation. If you'd prefer to keep the direct import, add a trailing comment instead:
- import opentelemetry.instrumentation.google_generativeai # noqa: F401
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py (2)
72-85: Remove duplicate WRAPPED_METHODS definitionWRAPPED_METHODS is defined twice with identical content. Keep a single definition to avoid confusion.
Apply this diff to remove the duplicate block:
-WRAPPED_METHODS = [ - { - "package": "google.genai.models", - "object": "Models", - "method": "generate_content", - "span_name": "gemini.generate_content", - }, - { - "package": "google.genai.models", - "object": "AsyncModels", - "method": "generate_content", - "span_name": "gemini.generate_content", - }, -]
318-331: Use dynamically selected wrapped methods and detect async wrappers_instrument currently uses the constant WRAPPED_METHODS, ignoring legacy vs. new API detection implemented in _wrapped_methods(). Also, AsyncModels.generate_content should use _awrap, not _wrap.
Apply this diff:
- for wrapped_method in WRAPPED_METHODS: + for wrapped_method in self._wrapped_methods(): wrap_package = wrapped_method.get("package") wrap_object = wrapped_method.get("object") wrap_method = wrapped_method.get("method") wrap_function_wrapper( wrap_package, f"{wrap_object}.{wrap_method}", - ( - _awrap(tracer, event_logger, wrapped_method) - if wrap_method == "generate_content_async" - else _wrap(tracer, event_logger, wrapped_method) - ), + ( + _awrap(tracer, event_logger, wrapped_method) + if (wrap_method.endswith("_async") or wrap_object.startswith("Async")) + else _wrap(tracer, event_logger, wrapped_method) + ), )This ensures:
- Legacy/new client selection is respected at runtime.
- AsyncModels.generate_content gets the async wrapper.
🧹 Nitpick comments (2)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py (2)
5-5: Import typing helpers to correctly annotate streaming responsesTo reflect the actual types of streaming responses, bring in Iterable and AsyncIterable.
Apply this diff:
-from typing import Collection, TYPE_CHECKING +from typing import Collection, TYPE_CHECKING, Iterable, AsyncIterable
96-102: Correct type annotations for streaming responses (iterables vs. single response)Both helpers iterate over response; the type should be Iterable/AsyncIterable of GenerateContentResponse. This keeps runtime behavior unchanged while improving type correctness.
Apply this diff:
def _build_from_streaming_response( span, - response: "GenerateContentResponse", + response: Iterable["GenerateContentResponse"], llm_model, event_logger, ): @@ async def _abuild_from_streaming_response( - span, response: "GenerateContentResponse", llm_model, event_logger + span, response: AsyncIterable["GenerateContentResponse"], llm_model, event_logger ):Also applies to: 117-120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py(4 hunks)packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Python code must conform to Flake8 linting rules
Do not hardcode API keys in source code; read them from environment variables or a secure vault
Files:
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.pypackages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
171-171: opentelemetry.instrumentation.google_generativeai imported but unused
Remove unused import: opentelemetry.instrumentation.google_generativeai
(F401)
🪛 Flake8 (7.2.0)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
[error] 171-171: 'opentelemetry.instrumentation.google_generativeai' imported but unused
(F401)
🔇 Additional comments (2)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py (1)
1-1: LGTM: sys import is usedUsed in the assertions and sys.modules manipulation.
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py (1)
36-38: Good: TYPE_CHECKING-guarded import defers heavy dependency at runtimeThis avoids ModuleNotFoundError when google-generativeai isn’t installed, aligning with the PR goal.
nirga
changed the title
|
@timkpaine can you take a look? Looks like test and lint are failing |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py (1)
1-1: Optional: localizesysimport to the test
sysis only used intest_importsafter the refactor above. Keep imports as local as possible to minimize module import overhead.Apply this diff (safe if you adopt the change above that imports
sysinside the test):-import sys
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py(4 hunks)packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/init.py
🧰 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-google-generativeai/tests/test_generate_content.py
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
171-171: opentelemetry.instrumentation.google_generativeai imported but unused
Remove unused import: opentelemetry.instrumentation.google_generativeai
(F401)
🪛 Flake8 (7.2.0)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
[error] 171-171: 'opentelemetry.instrumentation.google_generativeai' imported but unused
(F401)
⏰ 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: Lint
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Build Packages (3.11)
🔇 Additional comments (1)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py (1)
168-168: No-op changeBlank line addition; nothing to address.
| def test_imports(): | ||
| # Ensure heavy dependencies are not accidentally imported | ||
| import opentelemetry.instrumentation.google_generativeai | ||
|
|
||
| assert "google.genai" not in sys.modules |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix unused import (F401) and harden the assertion against environment-side effects
- The explicit import of
opentelemetry.instrumentation.google_generativeaiis for side effects only and triggers Flake8/Ruff F401, which will fail CI. - The assertion only checks
google.genai, while the historical package path isgoogle.generativeai. It may miss regressions. - Checking global
sys.modulescan be flaky if other tests pre-load these modules. Compare the delta ofsys.modulesbefore/after the import instead.
Apply this diff:
-def test_imports():
- # Ensure heavy dependencies are not accidentally imported
- import opentelemetry.instrumentation.google_generativeai
-
- assert "google.genai" not in sys.modules
+def test_imports():
+ # Ensure heavy dependencies are not imported as a side-effect of instrumentation import
+ import importlib, sys
+
+ before = set(sys.modules.keys())
+ importlib.import_module("opentelemetry.instrumentation.google_generativeai")
+ after = set(sys.modules.keys())
+ newly_loaded = after - before
+
+ # Neither of the Google GenAI SDKs should be imported by the instrumentation __init__
+ assert not any(m.startswith("google.genai") for m in newly_loaded)
+ assert not any(m.startswith("google.generativeai") for m in newly_loaded)Run linters locally to confirm the F401 is gone and the test remains green:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ruff packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
flake8 packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
pytest -q packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py::test_imports -qLength of output: 309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Determine the proper ruff invocation
ruff --help | head -n 20
# Run ruff lint using the corrected command
ruff check packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
# Run flake8 lint
flake8 packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
# Execute only the specific test to ensure it still passes
pytest -q packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py::test_importsLength of output: 1699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Create a temporary file with the suggested diff snippet
cat > /tmp/snippet.py << 'EOF'
def test_imports():
# Ensure heavy dependencies are not imported as a side-effect of instrumentation import
import importlib, sys
before = set(sys.modules.keys())
importlib.import_module("opentelemetry.instrumentation.google_generativeai")
after = set(sys.modules.keys())
newly_loaded = after - before
# Neither of the Google GenAI SDKs should be imported by the instrumentation __init__
assert not any(m.startswith("google.genai") for m in newly_loaded)
assert not any(m.startswith("google.generativeai") for m in newly_loaded)
EOF
# Verify lint status with Ruff (should report no F401)
ruff check /tmp/snippet.py || true
# Verify lint status with Flake8 (should report no F401)
flake8 /tmp/snippet.py || trueLength of output: 348
Fix unused import (F401), harden the test, and adhere to style conventions
- Remove the bare
import opentelemetry.instrumentation.google_generativeai(F401). - Use
importlib.import_moduleto trigger only side-effects. - Compare
sys.modulesbefore/after import to avoid flakiness. - Break long lines and separate imports to satisfy Flake8 (E401, E501).
Apply this diff:
--- a/packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
+++ b/packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
@@ -169,7 +169,19 @@ def test_imports():
- # Ensure heavy dependencies are not accidentally imported
- import opentelemetry.instrumentation.google_generativeai
-
- assert "google.genai" not in sys.modules
+ # Ensure heavy dependencies aren’t loaded as a side effect
+ import importlib
+ import sys
+
+ before = set(sys.modules.keys())
+ importlib.import_module(
+ "opentelemetry.instrumentation.google_generativeai"
+ )
+ after = set(sys.modules.keys())
+ newly_loaded = after - before
+
+ # Neither Google GenAI SDK should appear in newly_loaded
+ for prefix in ("google.genai", "google.generativeai"):
+ assert not any(
+ m.startswith(prefix) for m in newly_loaded
+ )Run the following to confirm no lint errors and that the test passes:
ruff check packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
flake8 packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
pytest -q \
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py::test_imports🧰 Tools
🪛 Ruff (0.12.2)
171-171: opentelemetry.instrumentation.google_generativeai imported but unused
Remove unused import: opentelemetry.instrumentation.google_generativeai
(F401)
🪛 Flake8 (7.2.0)
[error] 171-171: 'opentelemetry.instrumentation.google_generativeai' imported but unused
(F401)
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-google-generativeai/tests/test_generate_content.py
around lines 169-173, the test currently uses a bare "import
opentelemetry.instrumentation.google_generativeai" which triggers an
unused-import (F401), is brittle and can flake; replace it by importing
importlib and using importlib.import_module to invoke module side-effects,
capture a snapshot of sys.modules before the import, call
importlib.import_module("opentelemetry.instrumentation.google_generativeai"),
then compare sys.modules after to assert "google.genai" was not added; also
split imports onto separate lines and wrap long lines to satisfy Flake8/ruff
(E401, E501).
There was a problem hiding this comment.
@timkpaine No problem! I understand you prefer to keep the test simple as-is.
If you do want to address the F401 unused import warning that's showing up in the linting tools, you could simply add a # noqa: F401 comment to suppress it:
def test_imports():
# Ensure heavy dependencies are not accidentally imported
import opentelemetry.instrumentation.google_generativeai # noqa: F401
assert "google.genai" not in sys.modulesBut if you're happy with the current implementation, that works too!
2 participants
xref: conda-forge/opentelemetry-instrumentation-google-generativeai-feedstock#23
openllmetry/packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.py
Line 7 in 6454263
feat(instrumentation): ...orfix(instrumentation): ....Important
Move
google-generativeaito runtime dependencies inpyproject.tomlto fix import error.google-generativeaifrom[tool.poetry.group.dev.dependencies]to[tool.poetry.dependencies]inpyproject.tomlto fixModuleNotFoundError.ModuleNotFoundErrorforgoogle.generativeai.types.generation_typesin__init__.py.This description was created by
for 136f51f. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Bug Fixes
Tests