feat(deepseek): add DeepSeek instrumentation with R1 reasoning_content support#4226
Open
CaringNihilistic wants to merge 2 commits into
Open
feat(deepseek): add DeepSeek instrumentation with R1 reasoning_content support#4226CaringNihilistic wants to merge 2 commits into
CaringNihilistic wants to merge 2 commits into
Conversation
|
|
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesDeepSeek OpenTelemetry Instrumentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
1 task
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/opentelemetry-instrumentation-deepseek/README.md (1)
4-4: ⚡ Quick winAdd alt text to the badge image for accessibility.
The PyPI badge image is missing alt text. Consider adding descriptive alt text for screen readers and accessibility compliance.
♿ Proposed fix
- <img src="https://badge.fury.io/py/opentelemetry-instrumentation-deepseek.svg"> + <img src="https://badge.fury.io/py/opentelemetry-instrumentation-deepseek.svg" alt="PyPI version">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opentelemetry-instrumentation-deepseek/README.md` at line 4, The README's PyPI badge <img> tag is missing alt text; update the <img> element in packages/opentelemetry-instrumentation-deepseek/README.md to include a descriptive alt attribute (e.g., alt="PyPI package: opentelemetry-instrumentation-deepseek") so screen readers and accessibility tools can convey the badge meaning.packages/opentelemetry-instrumentation-deepseek/tests/traces/conftest.py (1)
82-82: 💤 Low valueOptional: Remove redundant
readerfixture parameter.The
readerparameter is redundant since themeter_providerfixture (lines 57-62) already declaresreaderas a dependency. Pytest will automatically ensurereaderis initialized beforemeter_provider. Removing it would simplify the fixture signatures without changing behavior.♻️ Simplified fixture signatures
-def instrument_legacy(reader, tracer_provider, meter_provider): +def instrument_legacy(tracer_provider, meter_provider):-def instrument_with_content(reader, tracer_provider, logger_provider, meter_provider): +def instrument_with_content(tracer_provider, logger_provider, meter_provider):-def instrument_with_no_content(reader, tracer_provider, logger_provider, meter_provider): +def instrument_with_no_content(tracer_provider, logger_provider, meter_provider):Also applies to: 95-95, 114-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opentelemetry-instrumentation-deepseek/tests/traces/conftest.py` at line 82, Remove the redundant reader fixture parameter from the affected fixture functions; specifically, update instrument_legacy(reader, tracer_provider, meter_provider) and the other two fixture definitions referenced (the fixtures at the other noted locations) to drop the reader parameter since meter_provider already depends on reader and pytest will initialize it automatically; adjust the function signatures (e.g., instrument_legacy and the two other fixture function names shown in the diff) and any internal references to the removed parameter so the fixtures continue to use tracer_provider and meter_provider only.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/event_models.py`:
- Around line 18-22: The TypedDict field `role` in CompletionMessage is
incorrectly given a default (`role: str = "assistant"`); remove the default so
the key is a required string (`role: str`) because codepaths pass it explicitly,
or if you intend it optional switch to `role: NotRequired[str]` (importing
NotRequired from typing_extensions for compatibility). Update the
CompletionMessage definition accordingly to avoid TypedDict/typing-tool issues
(refer to the CompletionMessage TypedDict in event_models.py).
---
Nitpick comments:
In `@packages/opentelemetry-instrumentation-deepseek/README.md`:
- Line 4: The README's PyPI badge <img> tag is missing alt text; update the
<img> element in packages/opentelemetry-instrumentation-deepseek/README.md to
include a descriptive alt attribute (e.g., alt="PyPI package:
opentelemetry-instrumentation-deepseek") so screen readers and accessibility
tools can convey the badge meaning.
In `@packages/opentelemetry-instrumentation-deepseek/tests/traces/conftest.py`:
- Line 82: Remove the redundant reader fixture parameter from the affected
fixture functions; specifically, update instrument_legacy(reader,
tracer_provider, meter_provider) and the other two fixture definitions
referenced (the fixtures at the other noted locations) to drop the reader
parameter since meter_provider already depends on reader and pytest will
initialize it automatically; adjust the function signatures (e.g.,
instrument_legacy and the two other fixture function names shown in the diff)
and any internal references to the removed parameter so the fixtures continue to
use tracer_provider and meter_provider only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 081db5d1-e617-42c5-b0c2-8bdaca29d911
⛔ Files ignored due to path filters (3)
packages/opentelemetry-instrumentation-deepseek/uv.lockis excluded by!**/*.lockpackages/sample-app/uv.lockis excluded by!**/*.lockpackages/traceloop-sdk/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.cz.tomlpackages/opentelemetry-instrumentation-deepseek/.python-versionpackages/opentelemetry-instrumentation-deepseek/README.mdpackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/__init__.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/config.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/event_emitter.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/event_models.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/span_utils.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/utils.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/version.pypackages/opentelemetry-instrumentation-deepseek/poetry.tomlpackages/opentelemetry-instrumentation-deepseek/project.jsonpackages/opentelemetry-instrumentation-deepseek/pyproject.tomlpackages/opentelemetry-instrumentation-deepseek/tests/__init__.pypackages/opentelemetry-instrumentation-deepseek/tests/data/.gitkeeppackages/opentelemetry-instrumentation-deepseek/tests/traces/conftest.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_event_emitter.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_finish_reasons.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_init.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_span_utils.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_utils.pypackages/sample-app/pyproject.tomlpackages/traceloop-sdk/pyproject.toml
…t support Adds opentelemetry-instrumentation-deepseek package for tracing DeepSeek API calls. Closes traceloop#2597. DeepSeek has no native tracing support in OpenLLMetry. Users calling DeepSeek-R1 also lose visibility into reasoning_content (chain-of-thought output) which is not captured by the existing OpenAI instrumentation. - Patches openai.resources.chat.completions (DeepSeek is OpenAI-compatible) - Only activates when base_url contains "deepseek" via _is_deepseek_client() - Captures reasoning_content as gen_ai.deepseek.reasoning_content span attribute - Accumulates reasoning_content across streaming chunks - Follows the Groq instrumentation structure exactly
CaringNihilistic
force-pushed
the
feat/deepseek-instrumentation-2597
branch
from
abef5e8 to
4ec6149
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/span_utils.py`:
- Around line 259-308: The set_response_attributes function lacks exception
handling protection and can fail during execution, particularly when
json.dumps() is called on line 305 if the message content contains
non-JSON-serializable data. Add the `@dont_throw` decorator to the
set_response_attributes function definition to ensure that any exceptions raised
during execution are caught and logged without crashing the instrumentation,
similar to how set_model_response_attributes is protected.
- Around line 175-194: The `set_streaming_response_attributes` function is
missing the `@dont_throw` decorator which prevents exceptions from propagating
to user code. The `json.dumps([message])` call can raise exceptions if the
message contains non-serializable content, and without the decorator these will
crash the user's application. Add the `@dont_throw` decorator above the function
definition, matching the pattern used in the related functions
`set_input_attributes` and `set_model_input_attributes`, to ensure any
exceptions during serialization or span attribute operations are caught and
suppressed rather than propagated.
- Around line 197-210: The function set_model_streaming_response_attributes
lacks the `@dont_throw` decorator that is used for consistency and safety across
other instrumentation functions. Add the `@dont_throw` decorator above the
function definition to ensure that any unexpected errors (such as AttributeError
when accessing usage.completion_tokens, usage.prompt_tokens, or
usage.total_tokens) are caught and handled gracefully, maintaining
instrumentation resilience without interrupting the main application flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ecf1e1f-28ee-43fe-b1a6-5abebe1e58e9
⛔ Files ignored due to path filters (3)
packages/opentelemetry-instrumentation-deepseek/uv.lockis excluded by!**/*.lockpackages/sample-app/uv.lockis excluded by!**/*.lockpackages/traceloop-sdk/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.cz.tomlpackages/opentelemetry-instrumentation-deepseek/.python-versionpackages/opentelemetry-instrumentation-deepseek/README.mdpackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/__init__.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/config.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/event_emitter.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/event_models.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/span_utils.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/utils.pypackages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/version.pypackages/opentelemetry-instrumentation-deepseek/poetry.tomlpackages/opentelemetry-instrumentation-deepseek/project.jsonpackages/opentelemetry-instrumentation-deepseek/pyproject.tomlpackages/opentelemetry-instrumentation-deepseek/tests/__init__.pypackages/opentelemetry-instrumentation-deepseek/tests/data/.gitkeeppackages/opentelemetry-instrumentation-deepseek/tests/traces/conftest.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_event_emitter.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_finish_reasons.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_init.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_span_utils.pypackages/opentelemetry-instrumentation-deepseek/tests/traces/test_utils.pypackages/sample-app/pyproject.tomlpackages/traceloop-sdk/pyproject.toml
✅ Files skipped from review due to trivial changes (4)
- packages/opentelemetry-instrumentation-deepseek/.python-version
- packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/version.py
- .cz.toml
- packages/opentelemetry-instrumentation-deepseek/project.json
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/opentelemetry-instrumentation-deepseek/README.md
- packages/traceloop-sdk/pyproject.toml
- packages/sample-app/pyproject.toml
- packages/opentelemetry-instrumentation-deepseek/poetry.toml
- packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/config.py
- packages/opentelemetry-instrumentation-deepseek/tests/traces/test_finish_reasons.py
- packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/event_models.py
- packages/opentelemetry-instrumentation-deepseek/tests/traces/test_utils.py
- packages/opentelemetry-instrumentation-deepseek/pyproject.toml
- packages/opentelemetry-instrumentation-deepseek/tests/traces/test_span_utils.py
- packages/opentelemetry-instrumentation-deepseek/tests/traces/test_event_emitter.py
- packages/opentelemetry-instrumentation-deepseek/tests/traces/conftest.py
- packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/utils.py
- packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/init.py
- packages/opentelemetry-instrumentation-deepseek/opentelemetry/instrumentation/deepseek/event_emitter.py
- packages/opentelemetry-instrumentation-deepseek/tests/traces/test_init.py
Author
|
Hi team, CLA is now signed by all committers. Could a maintainer |
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.
What
Adds
opentelemetry-instrumentation-deepseekpackage for tracing DeepSeek API calls.Why
Closes #2597. DeepSeek has no native tracing support in OpenLLMetry. Users calling
DeepSeek-R1 also lose visibility into
reasoning_content(chain-of-thought output)which is not captured by the existing OpenAI instrumentation.
How
openai.resources.chat.completions(DeepSeek is OpenAI-compatible)base_urlcontains "deepseek" via_is_deepseek_client()reasoning_contentasgen_ai.deepseek.reasoning_contentspan attributereasoning_contentacross streaming chunksTesting
121 tests passing, ruff clean.
Checklist
Summary by CodeRabbit