fix(cohere): add v2 api instrumentation#3378
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds content-specific span attribute handlers, explicit span lifecycle control (sync/async), streaming processors for Cohere v1/v2 (sync+async), response normalization/utilities, module-aware dynamic wrapping, many test cassettes/tests, and test dependency bumps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Wrapper as Instrumentation Wrapper
participant Cohere as Cohere SDK
participant Tracer as OTel Tracer
App->>Wrapper: call Client.method(...)/AsyncClient.method(...)
Wrapper->>Tracer: start_span(span_name, attrs: LLM_SYSTEM, LLM_REQUEST_TYPE)
Wrapper->>Wrapper: set_span_request_attributes(kwargs)
Wrapper->>Wrapper: set_input_content_attributes(kwargs)
alt streaming method
Wrapper->>Cohere: invoke chat_stream(...)
loop chunks/events
Cohere-->>Wrapper: chunk/event
Wrapper-->>App: yield chunk
Wrapper->>Wrapper: accumulate into final_response
end
Wrapper->>Wrapper: set_span_response_attributes(final_response)
Wrapper->>Tracer: end span
else non-streaming
Wrapper->>Cohere: invoke method(...)
Cohere-->>Wrapper: response
Wrapper->>Wrapper: set_span_response_attributes(response)
Wrapper->>Wrapper: set_response_content_attributes(response)
Wrapper->>Tracer: end span
end
Note over Wrapper,Tracer: on exception → record_exception, set_status(ERROR), end span, re-raise
sequenceDiagram
autonumber
actor App
participant Wrapper as Stream Wrapper (v2)
participant Cohere as Cohere v2 SSE
participant Acc as Accumulator
App->>Wrapper: chat(stream=True, messages, tools)
Wrapper->>Cohere: request SSE stream
Cohere-->>Wrapper: message-start / content-start
loop deltas
Cohere-->>Wrapper: content-delta / tool-plan-delta / tool-call-*
Wrapper->>Acc: _accumulate_stream_item(delta)
Wrapper-->>App: yield delta
end
Cohere-->>Wrapper: content-end / message-end
Wrapper->>Acc: finalize final_response
Wrapper->>Wrapper: set_span_response_attributes(final_response)
Wrapper->>Tracer: end span (or set ERROR if exception)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 96db843 in 2 minutes and 36 seconds. Click for details.
- Reviewed
5671lines of code in25files - Skipped
1files when reviewing. - Skipped posting
14draft 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-cohere/opentelemetry/instrumentation/cohere/streaming.py:119
- Draft comment:
Swallowing exceptions silently in _accumulate_stream_item (using 'except Exception: pass') may hide errors. Consider logging the exception details. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The code is deliberately designed to not throw errors - the @dont_throw decorator and try/except blocks are intentional. This is streaming response processing code where we want to continue processing even if one chunk fails. The final response has an error field that can capture errors. Logging every parsing error could be noisy and unhelpful since errors are expected in streaming processing. Silent exception handling could hide real bugs or issues that should be investigated. Without any logging, debugging problems could be more difficult. The code is explicitly designed to be resilient to errors in stream processing. The error handling strategy appears intentional rather than accidental. The final response object has a dedicated error field for capturing important errors. The comment should be deleted. The exception handling appears to be an intentional design choice for streaming resilience rather than a bug or oversight.
2. packages/opentelemetry-instrumentation-cohere/tests/test_chat.py:71
- Draft comment:
Inconsistent assertion found for LLM_USAGE_PROMPT_TOKENS; one test expects 58 tokens while other tests expect 7. Verify that this discrepancy is intentional and correctly reflects API behavior. - 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/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py:113
- Draft comment:
Typo alert: The async stream process function is named "aprocess_chat_v1_streaming_response". Consider whether adding an underscore (e.g. "a_process_chat_v1_streaming_response") would improve clarity and consistency with the synchronous version. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% 1. The function is imported from another module, so renaming it here would require changes in other files. 2. The 'a' prefix without underscore is a common convention in Python for async variants. 3. The current name is clear and consistent with Python async naming patterns. 4. The comment is about style rather than a substantive issue. The comment raises a valid point about naming consistency, and underscores in function names are common in Python. However, the 'a' prefix without underscore is an established pattern for async variants, and changing an imported function name would require cross-file changes for minimal benefit. Delete the comment as it suggests a style change to an imported function that follows acceptable Python conventions and would require cross-file changes.
4. packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py:230
- Draft comment:
Typo: There's an extra closing quote at the end of the comment in this line. Consider removing the extra " after '(possibly pre-prompted)'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While this is technically correct - there is an extra quote - it's an extremely minor issue in a comment. Comments are not executed code and this doesn't affect functionality at all. The meaning of the comment is still perfectly clear. This feels like unnecessary nitpicking that doesn't improve code quality in any meaningful way. The comment is factually correct - there is an extra quote. And some might argue that even small issues should be fixed for perfect code quality. But we need to focus on meaningful improvements. Nitpicking about comment formatting is more likely to be annoying than helpful. Delete this comment. While technically correct, it's an extremely minor formatting issue in a comment that doesn't impact functionality or readability.
5. packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml:117
- Draft comment:
Typographical suggestion: The text "''s" appears to be an incorrect contraction. Did you mean to use "here's" instead? - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a test cassette file that records actual API responses. The "''s" is part of the actual response from the API - it's not code we wrote, but rather data we received. Making style suggestions about API response content doesn't make sense since we can't change what the API returns. Additionally, this is a test file meant to capture real responses. Could this unusual formatting actually be a bug in how we're recording the API responses? Maybe it deserves investigation? No - looking at the full context, this is clearly part of a streaming response where text is being returned piece by piece. The formatting is exactly as received from the API and is working as intended. Delete this comment. We should not suggest changes to recorded API responses in test cassettes, as they need to exactly match what the API returns.
6. packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml:437
- Draft comment:
Typographical suggestion: The fragment "''t" seems off. Consider using the proper contraction (e.g. "shan't") if that was intended. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a test cassette file that records actual API responses. The unusual formatting "''t" is part of the API's response data and is intentionally capturing how the API returns the text. Suggesting changes to the API's response format would be incorrect since we want to preserve the exact response for testing purposes. Maybe the unusual formatting indicates a bug in the API that should be reported? Maybe this formatting could cause issues in the tests? Even if the formatting is unusual, this is a test cassette that should accurately record the API's behavior. If there are issues with the API's response format, that should be handled separately from these tests. Delete the comment since it suggests modifying recorded API response data in a test cassette, which would defeat the purpose of having accurate test recordings.
7. packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy.yaml:4
- Draft comment:
Typographical error: There appears to be an extra space between "the" and "2010" in the text. Please remove the extra space. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test fixture file that records an API interaction for testing purposes. The extra space is in the request data that gets sent to the API. Since this is test data, the exact formatting of the text, including spaces, could potentially matter for matching the recorded response. We should be very careful about modifying test fixtures unless we're sure it won't break tests. The comment points out a real issue, but is it actually a problem? The API seems to handle the extra space fine since the response data shows the text normalized with a single space. Looking at the response data (line 43), the API normalizes the spaces anyway, so the extra space in the request doesn't affect functionality. This is a purely cosmetic issue in test data. The comment should be deleted. While technically correct about the extra space, this is a minor formatting issue in test data that doesn't affect functionality since the API normalizes spaces anyway.
8. packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy_async.yaml:4
- Draft comment:
Typographical error: There is an extra space between 'At the' and '2010' in the request body (line 4). Please remove the additional space. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test fixture file that contains recorded API interactions for testing purposes. The extra space is technically present, but does it actually matter? The response data shows the API normalized the text anyway by removing the extra space. Since this is test data and the API handles the space gracefully, pointing out this minor formatting issue seems overly pedantic and not valuable. The space could potentially cause issues if exact string matching is being done in tests. The consistency of test data can be important. While consistency is good, this is clearly not causing any actual problems since the API normalizes the text. The comment is focusing on an insignificant formatting detail that has no impact on functionality. Delete this comment as it points out a trivial formatting issue in test fixture data that has no functional impact.
9. packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy.yaml:5
- Draft comment:
Typographical error: There appears to be an extra space in the text "At the 2010 United States Census" on line 5. Consider replacing "At the 2010" with "At the 2010". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is test data that's likely captured from real API interactions. The extra space is in a JSON string that's part of the test fixture. Since this is test data meant to validate API behavior, the exact formatting of the text content (including extra spaces) is not relevant to the functionality being tested. The comment is focusing on a purely cosmetic issue in test data. Maybe the extra space could cause issues with text matching in tests? Maybe consistency in test data is important for maintainability? The tests are likely checking API behavior and response handling, not exact string matching. The extra space doesn't affect the semantic meaning or the test's ability to verify the correct functionality. Delete this comment as it's pointing out a trivial formatting issue in test data that doesn't impact functionality or test effectiveness.
10. packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy_async.yaml:5
- Draft comment:
Typographical error: There is a double space between "At the" and "2010" in the document text on this line. Consider reducing it to a single space. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is test data in a cassette file used for recording API interactions. The double space is in example text that's being sent to test an API. The spacing in test data is irrelevant as long as the tests pass. This seems like an extremely minor formatting issue that doesn't impact functionality. Maybe consistent formatting is important for test data readability? Maybe the double space could cause subtle test failures? The double space is part of a JSON string being sent to an API - it won't affect test outcomes since it's just example text. This level of formatting nitpicking on test data isn't valuable. Delete this comment as it's an extremely minor formatting issue in test data that doesn't impact functionality or readability in any meaningful way.
11. packages/opentelemetry-instrumentation-cohere/tests/test_embed.py:12
- Draft comment:
Typographical error: There is an extra space in "At the 2010 United States Census". Please change it to "At the 2010 United States Census". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is test data being sent to an embedding API. The extra space is likely irrelevant to the functionality being tested. The tests are checking the correct handling of API calls, responses, and telemetry, not the specific content of the text. The presence or absence of an extra space won't affect the test's ability to verify the core functionality. The extra space could be a genuine mistake that makes the test data less clean and professional. Consistent formatting in test data might make the tests more readable. While clean test data is nice to have, this is an extremely minor formatting issue that doesn't impact the test's effectiveness or readability in any meaningful way. The comment should be deleted as it addresses a trivial formatting issue in test data that doesn't impact functionality or test effectiveness.
12. packages/opentelemetry-instrumentation-cohere/tests/test_embed.py:71
- Draft comment:
Typographical error: There is an extra space in "At the 2010 United States Census". Please change it to "At the 2010 United States Census". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test file and the extra space is in test data. The space doesn't affect the functionality being tested (embedding API instrumentation). The tests are passing with the current data. The extra space might even be intentional to test how the API handles various text formats. We should focus on actual code issues rather than cosmetic issues in test data. The extra space could be considered a data quality issue, and consistent, clean test data might be valuable for future test maintenance. While clean data is good, this is an extremely minor formatting issue in test data that doesn't impact the test's effectiveness or readability. The comment violates the rule about not making purely informative or unimportant comments. Delete the comment as it addresses a trivial formatting issue in test data that doesn't affect functionality or test effectiveness.
13. packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py:214
- Draft comment:
Typographical issue: There's a double space in the string "At the 2010 United States Census, Carson City had a population of 55,274." Please remove the extra space. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is test data that's being used consistently across multiple test cases. The double space, while technically a typo, appears to be intentionally copied to maintain consistency. Changing it would not affect functionality. In test data, exact string matching is often important for assertions, so we should be cautious about changing strings unless there's a clear benefit. The double space could be considered a real typo that should be fixed. Consistent doesn't necessarily mean correct. While true, this is test data where the exact string content doesn't matter as long as it's consistent. The tests are about reranking functionality, not string correctness. The comment should be deleted as it suggests a trivial change to test data that would not improve code quality or fix any real issues.
14. packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py:295
- Draft comment:
Typographical issue: Detected a double space in the string "At the 2010 United States Census, Carson City had a population of 55,274." Please remove the extra space. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is test data that's being used consistently across multiple test cases. The double space, while technically a typo, is part of the test input data. Changing it would not affect functionality. In fact, changing it might make the tests less realistic since real-world text often contains such imperfections. The consistency of the double space across test cases suggests it might even be intentional. The double space is indeed a typographical error, and generally we want clean, well-formatted code. Could fixing it improve readability? While clean code is important, this is test data that simulates real-world input. Having slightly messy test data can actually be beneficial for testing robustness. The change would be purely cosmetic and doesn't improve test quality. The comment should be deleted. The double space is in test data, appears consistently across test cases, and fixing it provides no meaningful benefit while potentially making the tests less realistic.
Workflow ID: wflow_2BbpbeluZuYYdyNM
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history.yaml (1)
1-165: Add VCR scrubbing for volatile headers and dynamic IDspackages/opentelemetry-instrumentation-cohere/tests/conftest.py: vcr_config() currently only filters "authorization" (line 122) — add additional filter_headers and a before_record hook to redact volatile headers and dynamic values (suggested: user-agent, x-fern-sdk-version, x-fern-language, date, x-debug-trace-id, x-envoy-upstream-service-time, num_tokens, num_chars, x-api-warning) and redact tool_call IDs / UUIDs in request/response bodies.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming.yaml (1)
1-659: Replace deprecated model "command" with Cohere's recommended v2 model "command-a-03-2025".Update the cassette(s) (e.g. packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming.yaml) to use model: "command-a-03-2025" and re-record tests — "command" is deprecated and will be removed on September 15, 2025.
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (1)
167-177: Harden tool-call accumulation and reset between calls.Avoid KeyError when function missing and prevent state bleed across tool calls.
elif item_dict.get("type") == "tool-call-delta": message = (item_dict.get("delta") or {}).get("message") or {} new_arguments = ((message.get("tool_calls") or {}).get("function") or {}).get("arguments") if new_arguments: - existing_arguments = (current_tool_call_item.get("function") or {}).get("arguments") - current_tool_call_item["function"]["arguments"] = (existing_arguments or "") + new_arguments + current_tool_call_item.setdefault("function", {}) + existing_arguments = current_tool_call_item["function"].get("arguments") + current_tool_call_item["function"]["arguments"] = (existing_arguments or "") + new_arguments elif item_dict.get("type") == "tool-call-end": - final_response["message"]["tool_calls"].append({**current_tool_call_item}) + import copy as _copy + final_response["message"]["tool_calls"].append(_copy.deepcopy(current_tool_call_item)) + # reset for next tool call + current_tool_call_item.clear() + current_tool_call_item.update({"id": "", "type": "function", "function": {"name": "", "arguments": "", "description": ""}})
🧹 Nitpick comments (32)
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy.yaml (4)
30-34: Scrub IDs and volatile headers to avoid leakage and cassette churn.Redact response "id", x-debug-trace-id, Date, Content-Length, Via, Server, x‑envoy‑upstream‑service‑time (and similar) in cassettes. These aren’t needed for replay, can be considered sensitive ops metadata, and cause noisy diffs.
Apply minimal redactions:
- string: "{\"id\":\"83e3e297-264b-478e-9b22-5058386292ed\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"Arrrr, + string: "{\"id\":\"<redacted-id>\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"Arrrr, @@ - Content-Length: - - '552' + Content-Length: + - '<redacted>' @@ - Via: - - 1.1 google + Via: + - '<redacted>' @@ - date: - - Sun, 14 Sep 2025 01:50:19 GMT + date: + - '<redacted>' @@ - server: - - envoy + server: + - '<redacted>' @@ - x-debug-trace-id: - - 3ea490a581ea0c90e48fbdccabff104b + x-debug-trace-id: + - '<redacted>' @@ - x-envoy-upstream-service-time: - - '2637' + x-envoy-upstream-service-time: + - '<redacted>'Prefer doing this centrally via VCR hooks (before_record[_response]) to auto-scrub across all cassettes. I can draft the hook if helpful.
Also applies to: 38-41, 49-49, 59-59, 67-70
52-55: Optional: redact num_chars/num_tokens to reduce brittle diffs.Token/char counts are non-essential for replay and often change. Consider replacing with placeholders.
- num_chars: - - '324' + num_chars: + - '<redacted>' - num_tokens: - - '95' + num_tokens: + - '<redacted>'
18-25: Optional: scrub request identifying headers (UA, SDK version, host).To avoid churn when SDK updates, filter user-agent, x-fern-* and host in recorded requests.
- user-agent: - - cohere/5.18.0 + user-agent: + - '<redacted>' @@ - x-fern-sdk-version: - - 5.18.0 + x-fern-sdk-version: + - '<redacted>' @@ - host: - - api.cohere.com + host: + - '<redacted>'Also applies to: 16-17
64-66: Legacy model is being removed on September 15, 2025 — keep this cassette “frozen”.x-api-warning shows model 'command' deprecation effective 2025‑09‑15. Treat this as a legacy test; avoid re‑recording, and add a separate cassette using a supported model to ensure forward‑compat coverage.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml (1)
544-546: Scrub x-debug-trace-id in cassettes.Trace/diagnostic IDs shouldn’t be committed. Recommend redacting in fixtures and adding a global VCR header filter.
Apply immediate redaction here (still prefer a global filter):
- x-debug-trace-id: - - bc877ec6699cd83aa9c7d1952c3fbb20 + x-debug-trace-id: + - "<REDACTED>"packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml (3)
30-41: Redact provider-generated IDs in cassette body (response_id, generation_id).Unique identifiers aren’t secrets per se, but we avoid committing provider traceables to reduce risk and churn. Replace with stable placeholders.
- string: '{"response_id":"ea2d074c-4f25-47cb-bef8-b00dc2ae991b","text":"Arrrr, + string: '{"response_id":"REDACTED","text":"Arrrr, ... - find a new treasure trove before then!","generation_id":"41bd7ae5-511d-449f-a64d-dd54e5d697e4","chat_history":[{"role":"USER","message":"Tell + find a new treasure trove before then!","generation_id":"REDACTED","chat_history":[{"role":"USER","message":"Tell
56-57: Scrub volatile headers (Date, x-debug-trace-id, x-envoy-upstream-service-time).These are non-deterministic and unnecessary for playback; scrubbing reduces cassette churn and accidental leakage. Aligns with our “no sensitive data in cassettes” guidance.
- date: - - Sun, 14 Sep 2025 01:23:07 GMT + date: + - REDACTED ... - x-debug-trace-id: - - 37d8ceb858f86160192181d6ece63b36 + x-debug-trace-id: + - REDACTED ... - x-envoy-upstream-service-time: - - '2401' + x-envoy-upstream-service-time: + - REDACTEDAlso applies to: 74-77
6-25: Consider sanitizing additional low-value, high-churn headers.Alt-Svc, Via, cache-control variants, pragma, vary, and Accept-Encoding add noise without test value. Safe to scrub or drop at record time.
Also applies to: 42-69
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py (2)
52-61: Tighten exception handling and cap payload size in dump_object to prevent log bloat.
Avoid broadException, add size cap, and use safer JSON fallback.-def dump_object(obj): - try: - if hasattr(obj, "model_dump_json") and callable(obj.model_dump_json): - return obj.model_dump_json() - except Exception: - pass - try: - return json.dumps(obj) - except Exception: - return "" +def dump_object(obj): + # Prefer model_dump_json when available + try: + if hasattr(obj, "model_dump_json") and callable(obj.model_dump_json): + s = obj.model_dump_json() + return s if len(s) <= 8192 else s[:8192] + "…" + except (AttributeError, TypeError, ValueError): + pass + # Fallback to JSON with a safe default + try: + s = json.dumps(obj, default=str, ensure_ascii=False) + return s if len(s) <= 8192 else s[:8192] + "…" + except (TypeError, ValueError): + # Final fallback: repr + s = repr(obj) + return s if len(s) <= 8192 else s[:8192] + "…"
64-77: Narrow excepts in to_dict and handle common containers more defensively.
Reduce blind catches; prefer predictable fallbacks.-def to_dict(obj): - try: - if hasattr(obj, "model_dump") and callable(obj.model_dump): - return obj.model_dump() - except Exception: - pass - if isinstance(obj, dict): - return obj - if dataclasses.is_dataclass(obj): - return dataclasses.asdict(obj) - try: - return dict(obj) - except Exception: - return obj +def to_dict(obj): + try: + if hasattr(obj, "model_dump") and callable(obj.model_dump): + return obj.model_dump() + except (AttributeError, TypeError, ValueError): + pass + if isinstance(obj, dict): + return obj + if dataclasses.is_dataclass(obj): + return dataclasses.asdict(obj) + # Last attempts: mapping-like or object __dict__ + try: + return dict(obj) + except (TypeError, ValueError): + return getattr(obj, "__dict__", obj)packages/opentelemetry-instrumentation-cohere/tests/conftest.py (4)
55-58: Close async client to avoid connector leaks.Make this an async-yield fixture and close the client.
@pytest.fixture -def async_cohere_client(): - return cohere.AsyncClient(os.environ.get("COHERE_API_KEY")) +async def async_cohere_client(): + client = cohere.AsyncClient(os.environ.get("COHERE_API_KEY")) + try: + yield client + finally: + if hasattr(client, "aclose"): + await client.aclose() + elif hasattr(client, "close"): + client.close()
60-63: Close v2 sync client to free resources.Yield and close after use.
@pytest.fixture def cohere_client_v2(): - return cohere.ClientV2(os.environ.get("COHERE_API_KEY")) + client = cohere.ClientV2(os.environ.get("COHERE_API_KEY")) + try: + yield client + finally: + if hasattr(client, "close"): + client.close()
65-68: Close async v2 client as well.Same pattern as async v1.
@pytest.fixture -def async_cohere_client_v2(): - return cohere.AsyncClientV2(os.environ.get("COHERE_API_KEY")) +async def async_cohere_client_v2(): + client = cohere.AsyncClientV2(os.environ.get("COHERE_API_KEY")) + try: + yield client + finally: + if hasattr(client, "aclose"): + await client.aclose() + elif hasattr(client, "close"): + client.close()
120-123: Reduce cassette churn: filter dynamic headers in VCR.Mask/remove headers like date, x-debug-trace-id, x-envoy-upstream-service-time to keep diffs stable.
@pytest.fixture(scope="module") def vcr_config(): - return {"filter_headers": ["authorization"]} + return { + "filter_headers": [ + "authorization", + "x-debug-trace-id", + "x-envoy-upstream-service-time", + "date", + "alt-svc", + "via", + "server", + "user-agent", + ], + "before_record_request": before_record_request, + "before_record_response": before_record_response, + }Add these helpers anywhere in this file:
def before_record_request(request): headers = request.get("headers", {}) for h in ["x-debug-trace-id", "x-envoy-upstream-service-time", "date", "alt-svc", "via", "user-agent"]: headers.pop(h, None) return request def before_record_response(response): headers = response.get("headers", {}) for h in ["x-debug-trace-id", "x-envoy-upstream-service-time", "date", "alt-svc", "via", "server"]: headers.pop(h, None) return responsepackages/opentelemetry-instrumentation-cohere/tests/test_embed.py (2)
7-9: Silence Ruff ARG001 for fixture-only parameter.instrument_legacy is intentionally unused but required to activate instrumentation via fixture. Add a no-op reference to placate Ruff.
def test_cohere_v2_embed_legacy( span_exporter, log_exporter, instrument_legacy, cohere_client_v2 ): + _ = instrument_legacy # noqa: ARG001
64-68: Same Ruff ARG001 in async test.Add a no-op reference.
async def test_cohere_v2_embed_legacy_async( span_exporter, log_exporter, instrument_legacy, async_cohere_client_v2 ): + _ = instrument_legacy # noqa: ARG001packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py (2)
207-210: Silence Ruff ARG001 for fixture-only parameter.def test_cohere_v2_rerank_legacy( span_exporter, log_exporter, instrument_legacy, cohere_client_v2 ): + _ = instrument_legacy # noqa: ARG001
289-291: Silence Ruff ARG001 for async fixture-only parameter.async def test_cohere_v2_rerank_legacy_async( span_exporter, log_exporter, instrument_legacy, async_cohere_client_v2 ): + _ = instrument_legacy # noqa: ARG001packages/opentelemetry-instrumentation-cohere/tests/test_chat.py (2)
1-1: Silence repeated Ruff ARG001 warnings in this test module.Given many fixture-only parameters, prefer a single file-level directive.
+# ruff: noqa: ARG001 import json
271-374: Reduce assertion duplication with small helpers.The repeated blocks validating request functions and tool_calls can be centralized to cut noise and ease maintenance.
+# Helpers to reduce assertion duplication +def assert_request_functions(span, tools, prefix=SpanAttributes.LLM_REQUEST_FUNCTIONS): + for i, tool in enumerate(tools): + fn = tool["function"] + assert span.attributes.get(f"{prefix}.{i}.name") == fn["name"] + assert span.attributes.get(f"{prefix}.{i}.description") == fn["description"] + import json as _json + assert _json.loads(span.attributes.get(f"{prefix}.{i}.parameters")) == fn["parameters"] + +def assert_tool_call(span, idx, tc): + assert span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.tool_calls.{idx}.id") == tc.id + assert span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.tool_calls.{idx}.name") == tc.function.name + assert span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.tool_calls.{idx}.arguments") == tc.function.argumentsThen replace the repeated assertions with calls to these helpers.
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py (3)
80-89: Duplicate write of LLM_REQUEST_FUNCTIONS.parameters.parameters is set twice; drop the duplicate.
_set_span_attribute( span, f"{SpanAttributes.LLM_REQUEST_FUNCTIONS}.{index}.parameters", dump_object(function.get("parameters")), ) - _set_span_attribute( - span, - f"{SpanAttributes.LLM_REQUEST_FUNCTIONS}.{index}.parameters", - dump_object(function.get("parameters")), - )
148-156: Support both max_tokens_to_sample and max_tokens.Cohere v2 uses max_tokens; v1 often used max_tokens_to_sample. Record whichever is provided.
- _set_span_attribute( - span, GEN_AI_REQUEST_MAX_TOKENS, kwargs.get("max_tokens_to_sample") - ) + _set_span_attribute( + span, GEN_AI_REQUEST_MAX_TOKENS, kwargs.get("max_tokens", kwargs.get("max_tokens_to_sample")) + )
171-179: Guard against None response.Avoid to_dict(None) and no-op early.
def set_span_response_attributes(span, response): if not span.is_recording(): return - - response_dict = to_dict(response) + if response is None: + return + response_dict = to_dict(response)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (5)
77-84: Don’t swallow exceptions silently; log at debug.Replace bare except/pass with debug logging to aid diagnostics.
- try: - _accumulate_stream_item(item, current_content_item, current_tool_call_item, final_response) - except Exception: - pass + try: + _accumulate_stream_item(item, current_content_item, current_tool_call_item, final_response) + except Exception as e: + import logging as _logging + _logging.getLogger(__name__).debug("Failed to accumulate v2 stream item: %s", e, exc_info=True)
118-123: Same: add debug logging instead of swallowing.- try: - _accumulate_stream_item(item, current_content_item, current_tool_call_item, final_response) - except Exception: - pass + try: + _accumulate_stream_item(item, current_content_item, current_tool_call_item, final_response) + except Exception as e: + import logging as _logging + _logging.getLogger(__name__).debug("Failed to accumulate v2 async stream item: %s", e, exc_info=True)
85-86: Remove stray print.Avoid console noise in libraries.
- print(f"final_response: {final_response}")
86-91: Guard event emission on event_logger presence (v2 sync).Prevent potential None deref if instrumented without event logger.
- if should_emit_events(): + if should_emit_events() and event_logger is not None: emit_response_events(event_logger, llm_request_type, final_response)
124-129: Guard event emission on event_logger presence (v2 async).- if should_emit_events(): + if should_emit_events() and event_logger is not None: emit_response_events(event_logger, llm_request_type, final_response)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (4)
202-205: Event emission may miss v2 chat inputs.
emit_input_event expects message (v1), but v2 passes messages. Consider emitting events for v2 messages as well.Would you like a small PR to extend emit_input_event to handle messages (list) for v2 while retaining v1?
179-181: Silence Ruff ARG001 on unused instance params.
Prefix unused parameters with underscores to satisfy lints without changing behavior.Apply this diff:
@@ - def wrapper(wrapped, instance, args, kwargs): - return func(tracer, event_logger, to_wrap, wrapped, instance, args, kwargs) + def wrapper(wrapped, _instance, args, kwargs): + return func(tracer, event_logger, to_wrap, wrapped, _instance, args, kwargs) @@ -def _wrap( +def _wrap( tracer: Tracer, event_logger: Union[EventLogger, None], to_wrap, wrapped, - instance, + _instance, args, kwargs, ): @@ -async def _awrap( +async def _awrap( tracer: Tracer, event_logger: Union[EventLogger, None], to_wrap, wrapped, - instance, + _instance, args, kwargs, ):Also applies to: 216-224, 264-273
329-340: Capture exception context when instrumentation binding fails.
Adds exc_info to debug logs; helps diagnose version drift or API changes.Apply this diff:
@@ - except (ImportError, ModuleNotFoundError, AttributeError): - logger.debug(f"Failed to instrument {wrap_module}.{wrap_object}.{wrap_method}") + except (ImportError, ModuleNotFoundError, AttributeError): + logger.debug( + f"Failed to instrument {wrap_module}.{wrap_object}.{wrap_method}", + exc_info=True, + ) @@ - except (ImportError, ModuleNotFoundError, AttributeError): - logger.debug(f"Failed to instrument {wrap_module}.{wrap_object}.{wrap_method}") + except (ImportError, ModuleNotFoundError, AttributeError): + logger.debug( + f"Failed to instrument {wrap_module}.{wrap_object}.{wrap_method}", + exc_info=True, + )Also applies to: 341-353
354-376: Minor: silence Ruff ARG002 and add exc_info during uninstrument failures.
Keeps logs useful and lints quiet.Apply this diff:
-def _uninstrument(self, **kwargs): +def _uninstrument(self, **_kwargs): @@ - except (ImportError, ModuleNotFoundError, AttributeError): - logger.debug(f"Failed to uninstrument {wrap_module}.{wrap_object}.{wrap_method}") + except (ImportError, ModuleNotFoundError, AttributeError): + logger.debug( + f"Failed to uninstrument {wrap_module}.{wrap_object}.{wrap_method}", + exc_info=True, + ) @@ - except (ImportError, ModuleNotFoundError, AttributeError): - logger.debug(f"Failed to uninstrument {wrap_module}.{wrap_object}.{wrap_method}") + except (ImportError, ModuleNotFoundError, AttributeError): + logger.debug( + f"Failed to uninstrument {wrap_module}.{wrap_object}.{wrap_method}", + exc_info=True, + )
📜 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-cohere/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py(7 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py(5 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py(1 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py(2 hunks)packages/opentelemetry-instrumentation-cohere/pyproject.toml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_content_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_no_content_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy_async.yaml(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/conftest.py(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/test_chat.py(4 hunks)packages/opentelemetry-instrumentation-cohere/tests/test_embed.py(1 hunks)packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/cassettes/**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Never commit secrets or PII in VCR cassettes; scrub sensitive data
Files:
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_content_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_no_content_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming_async.yamlpackages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml
**/*.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-cohere/tests/test_embed.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.pypackages/opentelemetry-instrumentation-cohere/tests/conftest.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.pypackages/opentelemetry-instrumentation-cohere/tests/test_chat.pypackages/opentelemetry-instrumentation-cohere/tests/test_rerank.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py
🧬 Code graph analysis (5)
packages/opentelemetry-instrumentation-cohere/tests/test_embed.py (3)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
SpanAttributes(64-261)packages/opentelemetry-instrumentation-cohere/tests/conftest.py (3)
instrument_legacy(71-79)cohere_client_v2(61-62)async_cohere_client_v2(66-67)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (3)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py (4)
dont_throw(13-35)should_send_prompts(38-41)to_dict(64-77)should_emit_events(44-49)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py (2)
set_span_response_attributes(171-249)_set_span_chat_response(252-293)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
SpanAttributes(64-261)
packages/opentelemetry-instrumentation-cohere/tests/test_chat.py (1)
packages/opentelemetry-instrumentation-cohere/tests/conftest.py (7)
instrument_legacy(71-79)cohere_client_v2(61-62)cohere_client(51-52)async_cohere_client(56-57)instrument_with_content(83-95)instrument_with_no_content(99-111)async_cohere_client_v2(66-67)
packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py (2)
packages/opentelemetry-instrumentation-cohere/tests/conftest.py (3)
instrument_legacy(71-79)cohere_client_v2(61-62)async_cohere_client_v2(66-67)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
SpanAttributes(64-261)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py (5)
dump_object(52-61)should_send_prompts(38-41)to_dict(64-77)should_emit_events(44-49)dont_throw(13-35)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (2)
LLMRequestTypeValues(293-298)SpanAttributes(64-261)
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-cohere/tests/test_embed.py
8-8: Unused function argument: instrument_legacy
(ARG001)
67-67: Unused function argument: instrument_legacy
(ARG001)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py
56-57: try-except-pass detected, consider logging the exception
(S110)
56-56: Do not catch blind exception: Exception
(BLE001)
60-60: Do not catch blind exception: Exception
(BLE001)
68-69: try-except-pass detected, consider logging the exception
(S110)
68-68: Do not catch blind exception: Exception
(BLE001)
76-76: Do not catch blind exception: Exception
(BLE001)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py
81-82: try-except-pass detected, consider logging the exception
(S110)
81-81: Do not catch blind exception: Exception
(BLE001)
120-121: try-except-pass detected, consider logging the exception
(S110)
120-120: Do not catch blind exception: Exception
(BLE001)
packages/opentelemetry-instrumentation-cohere/tests/test_chat.py
90-90: Unused function argument: instrument_legacy
(ARG001)
134-134: Unused function argument: instrument_legacy
(ARG001)
176-176: Unused function argument: instrument_legacy
(ARG001)
226-226: Unused function argument: instrument_legacy
(ARG001)
418-418: Unused function argument: instrument_legacy
(ARG001)
578-578: Unused function argument: instrument_legacy
(ARG001)
616-616: Unused function argument: instrument_with_content
(ARG001)
646-646: Unused function argument: instrument_with_no_content
(ARG001)
676-676: Unused function argument: instrument_legacy
(ARG001)
719-719: Unused function argument: instrument_legacy
(ARG001)
770-770: Unused function argument: instrument_legacy
(ARG001)
922-922: Unused function argument: instrument_legacy
(ARG001)
packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py
209-209: Unused function argument: instrument_legacy
(ARG001)
290-290: Unused function argument: instrument_legacy
(ARG001)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py
270-270: Unused function argument: instance
(ARG001)
354-354: Unused method argument: kwargs
(ARG002)
⏰ 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.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (28)
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml (4)
3-4: AI summary mismatch: cassette uses model "command", not "command-r".The AI summary mentions "command-r", but the request body here sets "model": "command".
1-2: LGTM: cassette structure and SSE headers look correct.Good: POST /v2/chat, text/event-stream, 200 OK, and streaming content captured.
Also applies to: 27-28, 527-550
60-64: SSE JSON validated — no hard-wrapped data lines detected.
Provided validator printed "OK"; no action required.
1-551: Repo‑wide cassette secrets/PII sweep required.I ran the scan against packages/opentelemetry-instrumentation-cohere/tests/cassettes (rg produced no output). Absence of output isn't proof of safety — run a repo‑wide sweep from the repo root:
# repo-wide search for common secret patterns (exclude node_modules/vendor) rg -n --hidden -S -C1 -g '!.git/**' -g '!**/node_modules/**' -g '!**/vendor/**' \ -e 'Authorization:' -e 'Bearer\s+[A-Za-z0-9._-]+' -e 'X-Api-Key' -e 'x-api-key' \ -e 'api_key' -e 'access_token' -e 'client_secret' -e 'AWS_SECRET_ACCESS_KEY' . # search only VCR cassettes (yaml/yml/json) rg -n --hidden -S -C1 -g '**/cassettes/**/*.{yaml,yml,json}' \ -e 'Authorization:' -e 'Bearer\s+[A-Za-z0-9._-]+' -e 'X-Api-Key' -e 'api_key' \ -e 'BEGIN RSA PRIVATE KEY' -e '-----BEGIN PRIVATE KEY-----'If matches are found: remove/scrub the cassette (use VCR scrubbers or delete the file), and rotate/revoke any exposed credentials. Follow repo policy: never commit secrets or PII in /cassettes/**/*.{yaml,yml,json}.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml (1)
24-25: Verify cassette header matchers or re-record with Cohere ^5.18.xSandbox couldn't access repository files; unable to confirm. Check packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml (lines 24–25): if the recorded header
x-fern-sdk-version: 5.6.1is included in the cassette's match_on, remove/ignore that header from matchers or re-record the cassette with the bumped Cohere SDK (^5.18.x) to avoid playback breakage.packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py (2)
1-2: Imports LGTM.
Required for new helpers; no conflicts.
52-77: Privacy guardrail — confirm dump_object/to_dict call sites are only executed when content tracing is enabled.Search failed (opentelemetry/instrumentation/cohere not found), so I couldn't verify. Run from the repo root and confirm each match is gated by content-tracing flags:
rg -n -C3 "dump_object(|to_dict("
rg -n -C3 "TRACELOOP_TRACE_CONTENT|override_enable_content_tracing"If any call site is unguarded, gate it or avoid logging content to prevent PII exposure.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy.yaml (1)
1-77: Rerank cassette LGTM.
No sensitive headers recorded; payload size reasonable.packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history_async.yaml (1)
1-164: Cassette otherwise looks consistent and scrubbed.
Deprecation warnings and usage fields are fine.packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy_async.yaml (1)
1-93: Embed cassette LGTM.
No secrets; payload sizes acceptable for repo.packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_content_async.yaml (1)
1-84: Cassette appears scrubbed — verify repo‑wide header filters.
rg returned no matches; absence of matches is inconclusive. Confirm repository-level VCR/cassette header-filtering (Authorization / Bearer / api[-_]key / x-api-key / Set-Cookie / Cookie) is enforced and provide the config or CI check that implements it.packages/opentelemetry-instrumentation-cohere/pyproject.toml (2)
39-39: Cohere bumped to ^5.18.0 — PyPI 5.18.0 present and python_requires = >=3.9,<4.0.
Matches our 3.9–<4 CI range.
45-45: OK to keep pytest-asyncio = ^1.2.0. PyPI shows pytest-asyncio 1.2.0 is published; pytest‑8 compatibility was declared in v0.23.5 and recent releases constrain pytest to >=8.2,<9. (newreleases.io)packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_no_content_async.yaml (1)
1-84: Cassette OK — header drift is informational; no playback break detected.vcr_config only filters "authorization" (packages/opentelemetry-instrumentation-cohere/tests/conftest.py:121) and there are no custom VCR matchers ('match_on' / 'register_matcher') in the cohere tests, so recorded headers (including x-fern-sdk-version) are not used for matching — re-record or relax header matching only if you intentionally enable header-based matching.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming_async.yaml (1)
1-283: LGTM: good streaming + tool-calls coverage, no secrets observed.Cassette looks clean (no Authorization header, no PII). Exercises v2 streaming/tool-calls well.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming.yaml (1)
1-249: LGTM: deterministic v2 tool-call stream.Looks properly scrubbed and representative; fine to merge.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_rerank/test_cohere_v2_rerank_legacy_async.yaml (1)
1-77: LGTM: v2 rerank async cassette is clean and minimal.Good token accounting; no secrets present.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming_async.yaml (1)
1-376: LGTM: v1 streaming path covered; no sensitive headers recorded.All good for playback.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_embed/test_cohere_v2_embed_legacy.yaml (1)
1-93: LGTM: v2 embed cassette is tidy.No secrets/PII; deterministic payload.
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming.yaml (1)
1-300: LGTM: legacy v1 streaming cassette looks fine.Safe to merge.
packages/opentelemetry-instrumentation-cohere/tests/conftest.py (2)
55-68: Async fixtures confirmed — no action required.
All references to async_cohere_client / async_cohere_client_v2 are in tests annotated with @pytest.mark.asyncio; no synchronous tests reference these fixtures.
55-68: Confirmed — AsyncClient/AsyncClientV2 use aclose() and support async context manager.
Cohere Python SDK v5.18.0 async clients wrap httpx.AsyncClient, expose aclose(), and supportasync with; the fixtures are correct.packages/opentelemetry-instrumentation-cohere/tests/test_embed.py (1)
32-41: LGTM: Core span assertions look correct for v2 embed (legacy).Attributes validated here match the instrumentation contract (system, request type/model, prompt shape). Keep as-is.
packages/opentelemetry-instrumentation-cohere/tests/test_rerank.py (1)
229-241: LGTM: v2 rerank (legacy) span assertions are consistent.Prompt indexing (docs as system, query as user) and model/type checks align with span_utils behavior.
packages/opentelemetry-instrumentation-cohere/tests/test_chat.py (1)
13-48: LGTM: Tools schema mirrors Cohere v2 expectations.Tool definitions (names/descriptions/parameters) look correct for tool_calls coverage.
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (3)
14-24: LGTM: import reshuffle aligns with the new content/streaming responsibilities.
No issues spotted with the new imports (content attribute handlers, span-level helpers, streaming processors, and use_span).Also applies to: 37-37
189-196: LGTM: request-type mapping covers v1/v2 and streaming.
The mapping of chat/chat_stream→CHAT, generate/generate_stream→COMPLETION, rerank, embed looks correct.
42-42: Verify supported cohere versions for v2 coverage.
Range is "cohere >=4.2.7, <6". If v2 APIs require >=5.x, consider bumping lower bound or adding a note to docs/tests.Would you like me to scan the tests/lockfiles to confirm the minimum supported cohere version and propose an update?
| if to_wrap.get("stream_process_func"): | ||
| return to_wrap.get("stream_process_func")(span, event_logger, llm_request_type, response) | ||
|
|
There was a problem hiding this comment.
Streaming span lifecycle can leak on early-cancel; verify v2 error branch ends the span.
If the caller stops consuming the stream early, process_chat_* may never run its trailer and span may not end. Also, the v2 streaming processor sets ERROR but doesn’t end the span in the error path.
- In streaming.py, wrap the yield loop in try/finally and end the span in finally when final_response is present or on cancel.
- Also end the span in the error branch for v2 processors.
Example (streaming.py) pattern:
def process_chat_v2_streaming_response(span, event_logger, llm_request_type, response):
final_response = {...}
try:
for item in response:
span.add_event(name=f"{SpanAttributes.LLM_CONTENT_COMPLETION_CHUNK}")
_accumulate_stream_item(..., final_response)
yield item
except Exception as e:
if span.is_recording():
span.set_status(Status(StatusCode.ERROR, str(e)))
span.record_exception(e)
raise
finally:
# set attributes only if we have something to set; still end the span
if final_response:
set_span_response_attributes(span, final_response)
if should_emit_events():
emit_response_events(event_logger, llm_request_type, final_response)
elif should_send_prompts():
_set_span_chat_response(span, final_response)
if span.status is None or span.status.status_code == StatusCode.UNSET:
span.set_status(Status(StatusCode.OK))
span.end()| ''command'' is deprecated and will be removed September 15 2025. Please consider | ||
| upgrading to a newer model to avoid future service disruptions"],"billed_units":{"input_tokens":7,"output_tokens":80},"tokens":{"input_tokens":69,"output_tokens":81}}}' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Impending deprecation: model "command" removal dated September 15, 2025. Ensure tests never attempt live re-record.
If VCR ever re-records, this path will 404/410 after Sep 15, 2025. Confirm CI forces record=none and that we don’t depend on live calls for this legacy path.
Also applies to: 72-73
🏁 Script executed:
#!/bin/bash
# Verify VCR is not re-recording legacy cassettes and locate any remaining uses of the deprecated model.
set -euo pipefail
echo "Searching for pytest-vcr record modes and VCR configs..."
rg -nC2 -P '\brecord\s*=\s*["'\'']?(none|all|once|new_episodes)["'\'']?' -g '!**/node_modules/**'
echo
echo "Looking for global vcr_config fixtures or pytest.ini settings..."
rg -nC2 -P 'def\s+vcr_config|vcr=\{|\[pytest\]|\bvcr_cassette_library_dir\b' -g '!**/node_modules/**'
echo
echo "Checking for usages of deprecated Cohere model 'command' across tests and cassettes..."
rg -n -C2 -P "\bcommand\b" tests packages | sed 's/^/HIT: /'Length of output: 205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "--- root listing ---"
ls -la | sed -n '1,120p' || true
cassette="packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml"
echo
echo "--- check cassette file: $cassette ---"
if [ -f "$cassette" ]; then
echo "FOUND: $cassette"
sed -n '1,160p' "$cassette" || true
else
echo "MISSING: $cassette"
fi
echo
echo "--- list cassettes under cohere package (fallback-safe) ---"
if command -v fd >/dev/null 2>&1; then
fd -H -e yaml -c never --hidden packages/opentelemetry-instrumentation-cohere/tests/cassettes || true
else
find packages -type f -path "*/tests/cassettes/*.yaml" -print 2>/dev/null || true
fi
echo
echo "--- search repo for pytest-vcr / pytest-recording record flags and pytest addopts ---"
rg -n -S -uu --hidden -F -- '--vcr-record' || echo "no --vcr-record found"
rg -n -S -uu --hidden -F -- '--record-mode' || echo "no --record-mode found"
rg -n -S -uu --hidden -P '\baddopts\b' || echo "no addopts occurrences"
echo
echo "--- search CI/workflow files for pytest invocations and vcr flags ---"
rg -n -S -uu --hidden -g '.github/**' 'pytest' || echo "no pytest in .github"
rg -n -S -uu --hidden -g '.github/**' '--vcr-record' || echo "no --vcr-record in .github"
echo
echo "--- search for vcr fixtures / cassette dir config in tests/conftest.py or similar ---"
rg -n -S -uu --hidden -C2 'def\s+vcr_config|vcr_cassette_library_dir|vcr_cassette_dir|vcr_config' || echo "no vcr_config/vcr_cassette_dir found"
echo
echo "--- search tests and cassettes for the word 'command' (model) and 'cohere' ---"
rg -n -S -uu --hidden -C2 -- '\"command\"|\bcommand\b|cohere' tests packages || echo "no 'command' or 'cohere' matches in tests/packages"
echo
echo "--- full-repo 'command' scan (case-sensitive) ---"
rg -n -S -uu --hidden -- '\"command\"|\bcommand\b' || echo "no 'command' matches in repo"Length of output: 16786
Enforce VCR record=none in CI — legacy Cohere "command" cassette expires September 15, 2025
- Evidence: packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml contains "model":"command" and an API warning about removal on September 15, 2025; packages/opentelemetry-instrumentation-cohere/tests/conftest.py defines vcr_config but does not set a record mode; no global pytest addopts or --record-mode found in CI workflows.
- Action: Update CI (.github/workflows/ci.yml test job) to set VCRPY_RECORD_MODE=none or run pytest with --record-mode=none, or remove/replace the legacy cassette before September 15, 2025.
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml
lines 40-41: this cassette uses the deprecated "command" model and will expire
on Sept 15, 2025, so update CI to prevent VCR from attempting to re-record it;
modify .github/workflows/ci.yml test job to set VCRPY_RECORD_MODE=none (or run
pytest with --record-mode=none) so cassettes are only replayed in CI, or
alternatively remove/replace the legacy cassette with one using a supported
model before that date.
| body: '{"model": "command", "messages": [{"role": "user", "content": "Tell me | ||
| a joke, pirate style"}], "stream": true}' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Deprecated model in cassette will time-bomb tests; re-record with supported model.
Response warns model "command" is deprecated and removed on September 15, 2025. Re-record this cassette with a supported v2 model (e.g., command‑r) to avoid brittle replays and future re-record failures.
Run to find other affected cassettes:
Also applies to: 541-543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --no-heading -C1 '"model":\s*"command"' packages/opentelemetry-instrumentation-cohere/tests/cassettes
rg -n --no-heading -C1 'x-api-warning:.*deprecated' packages/opentelemetry-instrumentation-cohere/tests/cassettesLength of output: 6791
Replace deprecated "command" model in test cassettes — re-record with a supported v2 model.
Multiple cassettes record requests using "model": "command" (deprecated); migrate to a supported v2 model to avoid future test failures. (docs.cohere.com)
Affected cassettes (found by ripgrep):
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_completion/test_cohere_completion_with_events_with_no_content.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_content.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_no_content_async.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_content_async.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming_async.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_async.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy_with_streaming.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_legacy.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_completion/test_cohere_completion_with_events_with_content.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_completion/test_cohere_completion_legacy.yaml
- packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_chat_with_events_with_no_content.yaml
Action (one-line): Re-record these cassettes replacing "model": "command" with a current Cohere model (e.g., "command-r" or "command-a") and commit the new cassettes; then run the test suite. (docs.cohere.com)
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_streaming_async.yaml
(lines 3-4): the cassette records a deprecated model ("command"); re-record this
cassette (and the other affected cassettes listed in the review) by updating the
request payload to use a supported v2 model such as "command-r" or "command-a",
regenerate the HTTP cassette(s) using the test recorder so responses align with
the new model, commit the updated cassette files, and run the full test suite to
ensure no failures.
| "get_weather_mznfyh090g90", "content": "4:20 PM"}, {"role": "tool", "tool_call_id": | ||
| "get_time_eksdqe8f9g4y", "content": "Sunny 20 degrees Celsius"}], "tools": [{"type": | ||
| "function", "function": {"name": "get_weather", "description": "Get the current | ||
| weather in a given location", "parameters": {"type": "object", "properties": | ||
| {"location": {"type": "string", "description": "The city and state, e.g. San | ||
| Francisco, CA"}}, "required": ["location"]}}}, {"type": "function", "function": | ||
| {"name": "get_time", "description": "Get the current time in a given location", | ||
| "parameters": {"type": "object", "properties": {"location": {"type": "string", | ||
| "description": "The city and state, e.g. San Francisco, CA"}}, "required": ["location"]}}}], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potentially swapped tool outputs in async history step.
Tool results seem inverted: weather id carries time and time id carries weather. Consider fixing test data or re-recording.
🏁 Script executed:
#!/bin/bash
# Show the mismatched lines for quick inspection
sed -n '70,105p' packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history_async.yamlLength of output: 1719
Fix swapped tool outputs in async cassette
get_weather (id get_weather_mznfyh090g90) contains "4:20 PM" and get_time (id get_time_eksdqe8f9g4y) contains "Sunny 20 degrees Celsius" — swap these tool output entries in packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_history_async.yaml (lines ~84–92) or re-record the cassette.
| body: '{"model": "command-r", "messages": [{"role": "user", "content": "What is | ||
| the weather and current time in Tokyo?"}], "tools": [{"type": "function", "function": | ||
| {"name": "get_weather", "description": "Get the current weather in a given location", | ||
| "parameters": {"type": "object", "properties": {"location": {"type": "string", | ||
| "description": "The city and state, e.g. San Francisco, CA"}}, "required": ["location"]}}}, | ||
| {"type": "function", "function": {"name": "get_time", "description": "Get the | ||
| current time in a given location", "parameters": {"type": "object", "properties": | ||
| {"location": {"type": "string", "description": "The city and state, e.g. San | ||
| Francisco, CA"}}, "required": ["location"]}}}], "stream": true}' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Heads-up: model 'command-r' deprecation (removal set for September 15, 2025).
Re-records after that date will fail. Consider switching to a supported model before next re-record.
🌐 Web query:
Cohere model deprecations as of September 2025 — what replaces "command-r" for chat + tools?
💡 Result:
Short answer: Cohere's "command-r" chat models have been deprecated and replaced by the 08-2024 family — use "command-r-08-2024" (and "command-r-plus-08-2024" for the + tier) for chat and tools. [1][2][3]
Sources:
- Cohere deprecations page (deprecation history). [1]
- OCI / Cohere Command R 08-2024 release notes (chat/tool replacement). [2]
- GitHub Models changelog noting migration to Command R 08-2024 (and R+ 08-2024). [3]
Replace deprecated model 'command-r' with 'command-r-08-2024' (use 'command-r-plus-08-2024' for + tier)
Cohere's 08-2024 family replaces "command-r"; re-records will fail after September 15, 2025 — update this cassette before the next re-record.
File: packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming_async.yaml (lines 3-11)
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-cohere/tests/cassettes/test_chat/test_cohere_v2_chat_legacy_with_tool_calls_and_streaming_async.yaml
around lines 3 to 11, the cassette uses the deprecated model name "command-r";
update the body JSON to use "command-r-08-2024" (or "command-r-plus-08-2024" if
this test should target the + tier) by replacing the "model": "command-r" entry
accordingly and ensure the cassette is re-recorded after updating the model
string.
| interactions: | ||
| - request: | ||
| body: '{"model": "command", "messages": [{"role": "user", "content": "Tell me | ||
| a joke, pirate style"}], "stream": false}' | ||
| headers: | ||
| accept: | ||
| - '*/*' | ||
| accept-encoding: | ||
| - gzip, deflate | ||
| connection: | ||
| - keep-alive | ||
| content-length: | ||
| - '112' | ||
| content-type: | ||
| - application/json | ||
| host: | ||
| - api.cohere.com | ||
| user-agent: | ||
| - cohere/5.18.0 | ||
| x-fern-language: | ||
| - Python | ||
| x-fern-sdk-name: | ||
| - cohere | ||
| x-fern-sdk-version: | ||
| - 5.18.0 | ||
| method: POST | ||
| uri: https://api.cohere.com/v2/chat |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify global header filtering for secrets.
No Authorization header here, good. Please confirm VCR config filters Authorization and API keys across all cassettes.
🏁 Script executed:
#!/bin/bash
# Verify no auth secrets are recorded and that filtering is configured.
rg -nI -C2 -g 'packages/**/cassettes/**' -e 'Authorization' -e 'api_key' -e 'Bearer ' -e 'COHERE_API_KEY'
rg -nI -C3 -e 'filter_headers' -e 'before_record' -e 'before_record_response' -e 'placeholders' -g 'packages/**/tests/**'Length of output: 60355
Critical: recorded API keys found in cassettes — scrub secrets and enforce global VCR filtering
Ripgrep found API keys in cassette request bodies (examples: apikey=test_api_key; "api_key": "phc_C44vUK9R1J6HYVdfJarTEPqVAoRPJzMXzFcj8PIrJgP"). Tests have per-module vcr_config fixtures but filtering is inconsistent (many only filter "authorization").
- Remove/rotate any real credentials in committed cassettes under packages//cassettes/ (search for api_key / apikey / secret_access_key / phc_*).
- Add a central, autouse vcr_config (top-level tests/conftest.py) that sets filter_headers for authorization plus common API-key headers (api-key, x-api-key, x-openai-api-key, x-cohere-api-key, etc.) and implement before_record_request/response to scrub body fields (apikey, api_key, secret_access_key) and insert placeholders.
- Sanitize existing cassettes (replace tokens with FILTERED), commit the sanitized files, and re-run rg to confirm no remaining real tokens.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (6)
155-157: Append a copy; then reset accumulator.Avoid mutating previously appended content items.
- elif item_dict.get("type") == "content-end": - final_response["message"]["content"].append(current_content_item) + elif item_dict.get("type") == "content-end": + import copy as _copy + final_response["message"]["content"].append(_copy.deepcopy(current_content_item)) + current_content_item.clear()
21-41: Guard missing stream-end; end span; decorate with dont_throw.Handle final_response None, avoid emitting with None, and guard event_logger in event mode. Also decorate for parity.
- def process_chat_v1_streaming_response(span, event_logger, llm_request_type, response): +@dont_throw +def process_chat_v1_streaming_response(span, event_logger, llm_request_type, response): @@ - set_span_response_attributes(span, final_response) - if should_emit_events(): - emit_response_events(event_logger, llm_request_type, final_response) - elif should_send_prompts(): - _set_span_chat_response(span, final_response) - span.set_status(Status(StatusCode.OK)) + if final_response is not None: + set_span_response_attributes(span, final_response) + if should_emit_events() and event_logger is not None: + emit_response_events(event_logger, llm_request_type, final_response) + elif should_send_prompts(): + _set_span_chat_response(span, final_response) + span.set_status(Status(StatusCode.OK)) + else: + span.set_status(Status(StatusCode.ERROR, "stream terminated without final response")) span.end()
43-63: Same guards/decorator for async v1 streamer.-async def aprocess_chat_v1_streaming_response(span, event_logger, llm_request_type, response): +@dont_throw +async def aprocess_chat_v1_streaming_response(span, event_logger, llm_request_type, response): @@ - set_span_response_attributes(span, final_response) - if should_emit_events(): - emit_response_events(event_logger, llm_request_type, final_response) - elif should_send_prompts(): - _set_span_chat_response(span, final_response) - span.set_status(Status(StatusCode.OK)) + if final_response is not None: + set_span_response_attributes(span, final_response) + if should_emit_events() and event_logger is not None: + emit_response_events(event_logger, llm_request_type, final_response) + elif should_send_prompts(): + _set_span_chat_response(span, final_response) + span.set_status(Status(StatusCode.OK)) + else: + span.set_status(Status(StatusCode.ERROR, "stream terminated without final response")) span.end()
67-76: Avoid mutating DEFAULT_MESSAGE; initialize accumulators safely.final_response["message"] references a module‑level dict; later mutations leak across requests. Also initialize accumulators with expected nested keys to avoid KeyError.
- final_response = { + import copy as _copy + final_response = { "finish_reason": None, - "message": DEFAULT_MESSAGE, + "message": _copy.deepcopy(DEFAULT_MESSAGE), "usage": {}, "id": "", "error": None, } - current_content_item = {} - current_tool_call_item = {} + current_content_item = {"type": "text", "thinking": None, "text": ""} + current_tool_call_item = { + "id": "", + "type": "function", + "function": {"name": "", "arguments": "", "description": ""}, + }
85-90: Always end span; guard event_logger; record proper exception.- set_span_response_attributes(span, final_response) - if should_emit_events(): - emit_response_events(event_logger, llm_request_type, final_response) + set_span_response_attributes(span, final_response) + if should_emit_events() and event_logger is not None: + emit_response_events(event_logger, llm_request_type, final_response) @@ - if final_response.get("error"): - span.set_status(Status(StatusCode.ERROR, final_response.get("error"))) - span.record_exception(final_response.get("error")) - else: - span.set_status(Status(StatusCode.OK)) - span.end() + if final_response.get("error"): + err = final_response.get("error") + span.set_status(Status(StatusCode.ERROR, str(err))) + # record as Exception if it's a string + span.record_exception(err if isinstance(err, BaseException) else Exception(str(err))) + else: + span.set_status(Status(StatusCode.OK)) + span.end()Also applies to: 91-97
99-107: Same fixes for async v2.- final_response = { + import copy as _copy + final_response = { "finish_reason": None, - "message": DEFAULT_MESSAGE, + "message": _copy.deepcopy(DEFAULT_MESSAGE), "usage": {}, "id": "", "error": None, } @@ - if should_emit_events(): - emit_response_events(event_logger, llm_request_type, final_response) + if should_emit_events() and event_logger is not None: + emit_response_events(event_logger, llm_request_type, final_response) @@ - if final_response.get("error"): - span.set_status(Status(StatusCode.ERROR, final_response.get("error"))) - span.record_exception(final_response.get("error")) - else: - span.set_status(Status(StatusCode.OK)) - span.end() + if final_response.get("error"): + err = final_response.get("error") + span.set_status(Status(StatusCode.ERROR, str(err))) + span.record_exception(err if isinstance(err, BaseException) else Exception(str(err))) + else: + span.set_status(Status(StatusCode.OK)) + span.end()Also applies to: 123-134
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (2)
234-241: Exception can be swallowed when span isn’t recording (sync).Always re‑raise; end span even when not recording.
- except Exception as e: - if span.is_recording(): - span.set_status(Status(StatusCode.ERROR, str(e))) - span.record_exception(e) - span.end() - raise + except Exception as e: + if span.is_recording(): + span.set_status(Status(StatusCode.ERROR, str(e))) + span.record_exception(e) + # We started the span; end it even if not recording. + span.end() + raise
281-287: Same exception swallowing bug (async).- except Exception as e: - if span.is_recording(): - span.set_status(Status(StatusCode.ERROR, str(e))) - span.record_exception(e) - raise + except Exception as e: + if span.is_recording(): + span.set_status(Status(StatusCode.ERROR, str(e))) + span.record_exception(e) + raise
🧹 Nitpick comments (7)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py (4)
94-103: Avoid None in len(kwargs.get('documents')).len(None) will raise. Cache with a default.
- _set_span_attribute( - span, - f"{GEN_AI_PROMPT}.{len(kwargs.get('documents'))}.role", - "user", - ) + docs = kwargs.get("documents") or [] + _set_span_attribute( + span, + f"{GEN_AI_PROMPT}.{len(docs)}.role", + "user", + ) _set_span_attribute( span, - f"{GEN_AI_PROMPT}.{len(kwargs.get('documents'))}.content", + f"{GEN_AI_PROMPT}.{len(docs)}.content", kwargs.get("query"), )
110-116: Guard EMBEDDING texts None case.If inputs/texts are both missing, iterating texts fails.
- if not inputs: - texts = kwargs.get("texts") - inputs = [ - {"type": "text", "text": text} for text in texts - ] + if not inputs: + texts = kwargs.get("texts") or [] + inputs = [{"type": "text", "text": text} for text in texts]
197-245: Token usage set multiple times; choose one precedence.You set usage from meta.billed_units and again from usage.billed_units. This overwrites and can confuse backends. Prefer one precedence and skip the other if already set.
299-301: Unify completion prefix for RERANK response.Chat paths use GEN_AI_COMPLETION.; rerank uses legacy LLM_COMPLETIONS.. For consistency with the semantic conventions learning, prefer GEN_AI_COMPLETION.*.
- prefix = f"{SpanAttributes.LLM_COMPLETIONS}.{idx}" + prefix = f"{GEN_AI_COMPLETION}.{idx}"packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (3)
81-83: Don’t swallow exceptions during accumulation.Blind except/ pass hides data bugs. At least record the exception.
- try: + try: _accumulate_stream_item(item, current_content_item, current_tool_call_item, final_response) - except Exception: - pass + except Exception as e: + span.record_exception(e)Also applies to: 119-121
166-171: Defensive structure for tool-call deltas.Ensure nested dicts exist before concatenation.
- elif item_dict.get("type") == "tool-call-delta": - message = (item_dict.get("delta") or {}).get("message") or {} - new_arguments = ((message.get("tool_calls") or {}).get("function") or {}).get("arguments") - if new_arguments: - existing_arguments = (current_tool_call_item.get("function") or {}).get("arguments") - current_tool_call_item["function"]["arguments"] = (existing_arguments or "") + new_arguments + elif item_dict.get("type") == "tool-call-delta": + message = (item_dict.get("delta") or {}).get("message") or {} + tool_calls = message.get("tool_calls") or {} + func = (tool_calls.get("function") or {}) + new_arguments = func.get("arguments") + if new_arguments: + current_tool_call_item.setdefault("function", {}) + existing_arguments = current_tool_call_item["function"].get("arguments") + current_tool_call_item["function"]["arguments"] = (existing_arguments or "") + new_arguments
136-176: Consider try/finally to end spans on early cancel.If the caller stops consuming early, spans may never end. Wrap the loop in try/finally and finalize in finally (only emit/attribute when you have a final_response). Same for async variant.
📜 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 selected for processing (3)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py(7 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py(5 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py(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-cohere/opentelemetry/instrumentation/cohere/__init__.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: traceloop/openllmetry#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
🧬 Code graph analysis (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py (5)
dump_object(52-61)should_send_prompts(38-41)to_dict(64-77)should_emit_events(44-49)dont_throw(13-35)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (2)
LLMRequestTypeValues(293-298)SpanAttributes(64-261)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (3)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py (4)
dont_throw(13-35)should_send_prompts(38-41)to_dict(64-77)should_emit_events(44-49)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py (2)
set_span_response_attributes(166-244)_set_span_chat_response(247-288)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
SpanAttributes(64-261)
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py
258-258: Unused function argument: instance
(ARG001)
342-342: Unused method argument: kwargs
(ARG002)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py
81-82: try-except-pass detected, consider logging the exception
(S110)
81-81: Do not catch blind exception: Exception
(BLE001)
119-120: try-except-pass detected, consider logging the exception
(S110)
119-119: Do not catch blind exception: Exception
(BLE001)
⏰ 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). (4)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
🔇 Additional comments (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (2)
219-228: LGTM on span construction and semconv tags.SpanKind, system, and request type attributes look correct and align with semconv.
316-341: Resilient wrapping/unwrapping.The import guards and debug logs around wrap/unwrap are good for optional deps.
| if should_send_prompts() and not should_emit_events(): | ||
| if llm_request_type == LLMRequestTypeValues.COMPLETION: | ||
| _set_span_attribute(span, f"{SpanAttributes.LLM_PROMPTS}.0.role", "user") | ||
| _set_span_attribute(span, f"{GEN_AI_PROMPT}.0.role", "user") | ||
| _set_span_attribute( | ||
| span, f"{SpanAttributes.LLM_PROMPTS}.0.content", kwargs.get("prompt") | ||
| span, f"{GEN_AI_PROMPT}.0.content", kwargs.get("prompt") | ||
| ) | ||
| elif llm_request_type == LLMRequestTypeValues.CHAT: | ||
| _set_span_attribute(span, f"{SpanAttributes.LLM_PROMPTS}.0.role", "user") | ||
| # client V1 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t drop tool definitions when events are enabled.
Tools are only recorded inside the “should_send_prompts() and not should_emit_events()” gate. In event mode (non‑legacy), tool definitions are silently lost. Either emit them as events or set the attributes when content tracing is enabled regardless of event mode.
Minimal fix: move the tools block outside the “not should_emit_events()” gate but keep the “should_send_prompts()” guard.
- if should_send_prompts() and not should_emit_events():
+ if should_send_prompts() and not should_emit_events():
...
- if kwargs.get("tools"):
- for index, tool in enumerate(kwargs.get("tools")):
- ...
- _set_span_attribute(
- span,
- f"{SpanAttributes.LLM_REQUEST_FUNCTIONS}.{index}.parameters",
- dump_object(function.get("parameters")),
- )
+ # prompt attributes only (legacy) above
+
+ # record tools when content tracing is enabled (legacy or events)
+ if should_send_prompts() and kwargs.get("tools"):
+ for index, tool in enumerate(kwargs.get("tools")):
+ function = tool.get("function")
+ if not function:
+ continue
+ _set_span_attribute(span, f"{SpanAttributes.LLM_REQUEST_FUNCTIONS}.{index}.name", function.get("name"))
+ _set_span_attribute(span, f"{SpanAttributes.LLM_REQUEST_FUNCTIONS}.{index}.description", function.get("description"))
+ _set_span_attribute(span, f"{SpanAttributes.LLM_REQUEST_FUNCTIONS}.{index}.parameters", dump_object(function.get("parameters")))Also applies to: 65-85
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for span_utils.py file path..."
FILE_PATH=$(rg --hidden --no-ignore --files | rg 'packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py' || true)
if [ -z "$FILE_PATH" ]; then
FILE_PATH=$(find . -type f -name 'span_utils.py' 2>/dev/null | rg 'opentelemetry.*cohere' || true)
fi
if [ -z "$FILE_PATH" ]; then
echo "FILE_NOT_FOUND"
rg -n --hidden --no-ignore 'should_send_prompts|should_emit_events|SpanAttributes.LLM_REQUEST_FUNCTIONS|GEN_AI_PROMPT|\"tools\"' || true
exit 0
fi
echo "FOUND: $FILE_PATH"
echo "---- print file with line numbers (1..240) ----"
nl -ba "$FILE_PATH" | sed -n '1,240p'
echo "---- grep for relevant symbols in file ----"
rg -n -C3 'should_send_prompts|should_emit_events|tools|LLM_REQUEST_FUNCTIONS|GEN_AI_PROMPT|SpanAttributes' "$FILE_PATH" || trueLength of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for span_utils.py..."
FILE=$(rg --hidden --no-ignore --files | rg 'packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py' || true)
if [ -z "$FILE" ]; then
FILE=$(find . -type f -name 'span_utils.py' 2>/dev/null | rg 'opentelemetry.*cohere' || true)
fi
if [ -z "$FILE" ]; then
echo "FILE_NOT_FOUND"
rg -n --hidden --no-ignore 'should_send_prompts|should_emit_events|SpanAttributes.LLM_REQUEST_FUNCTIONS|GEN_AI_PROMPT|\"tools\"' || true
exit 0
fi
echo "FOUND: $FILE"
echo "---- file line count ----"
wc -l "$FILE" || true
echo "---- printing file with line numbers (first 400 lines) ----"
awk '{printf "%4d: %s\n", NR, $0}' "$FILE" | sed -n '1,400p'
echo "---- grep for relevant symbols with context ----"
rg -n -C3 'should_send_prompts|should_emit_events|tools|SpanAttributes|GEN_AI_PROMPT|LLMRequestTypeValues' "$FILE" || trueLength of output: 21946
Don't drop tool definitions when events are enabled.
Tool definitions are only set inside the if should_send_prompts() and not should_emit_events(): branch, so when events are enabled the tools are not recorded — move the tools loop (currently lines 65–84) outside the not should_emit_events() guard while keeping the should_send_prompts() check so tools are recorded for both legacy and event modes. File: packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py.
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py
around lines 39–45 and the tools loop at ~65–84, the current code only sets tool
definitions inside the branch guarded by "not should_emit_events()", which
causes tools to be omitted when events are enabled; move the tools loop out of
the "and not should_emit_events()" guard but keep it inside the
"should_send_prompts()" check so tools are always recorded regardless of events
mode, i.e., ensure should_send_prompts() is evaluated before setting prompts and
tools, extract the tools loop so it runs when should_send_prompts() is true
(independent of should_emit_events()), and adjust any indentation/branching
accordingly.
| final_response["message"] = (item_dict.get("delta") or {}).get("message") or DEFAULT_MESSAGE | ||
| final_response["id"] = item_dict.get("id") | ||
| elif item_dict.get("type") == "content-start": | ||
| new_content_item = ((item_dict.get("delta") or {}).get("message") or {}).get("content") | ||
| current_content_item.clear() | ||
| current_content_item.update(new_content_item or {}) | ||
| elif item_dict.get("type") == "content-delta": |
There was a problem hiding this comment.
Copy DEFAULT_MESSAGE on fallback to avoid shared mutation.
- if item_dict.get("type") == "message-start":
- final_response["message"] = (item_dict.get("delta") or {}).get("message") or DEFAULT_MESSAGE
+ if item_dict.get("type") == "message-start":
+ import copy as _copy
+ final_response["message"] = (item_dict.get("delta") or {}).get("message") or _copy.deepcopy(DEFAULT_MESSAGE)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final_response["message"] = (item_dict.get("delta") or {}).get("message") or DEFAULT_MESSAGE | |
| final_response["id"] = item_dict.get("id") | |
| elif item_dict.get("type") == "content-start": | |
| new_content_item = ((item_dict.get("delta") or {}).get("message") or {}).get("content") | |
| current_content_item.clear() | |
| current_content_item.update(new_content_item or {}) | |
| elif item_dict.get("type") == "content-delta": | |
| if item_dict.get("type") == "message-start": | |
| import copy as _copy | |
| final_response["message"] = (item_dict.get("delta") or {}).get("message") or _copy.deepcopy(DEFAULT_MESSAGE) | |
| final_response["id"] = item_dict.get("id") | |
| elif item_dict.get("type") == "content-start": | |
| new_content_item = ((item_dict.get("delta") or {}).get("message") or {}).get("content") | |
| current_content_item.clear() | |
| current_content_item.update(new_content_item or {}) | |
| elif item_dict.get("type") == "content-delta": |
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py
around lines 140 to 146, final_response["message"] falls back to DEFAULT_MESSAGE
directly which can lead to shared-mutation bugs; change the fallback to return a
new copy of DEFAULT_MESSAGE (e.g., use copy.deepcopy(DEFAULT_MESSAGE) or
DEFAULT_MESSAGE.copy()) so each response gets its own independent dict instance.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (3)
125-144: Async v2 handler doesn't callspan.end()on the error pathAt line 144,
span.end()is only called in the else branch (success case). If an error occurs, the span won't be ended, leading to leaked spans.Apply this fix to ensure the span is always ended:
if final_response.get("error"): span.set_status(Status(StatusCode.ERROR, final_response.get("error"))) span.record_exception(final_response.get("error")) else: span.set_status(Status(StatusCode.OK)) - span.end() + span.end()
12-18:⚠️ Avoid mutation ofDEFAULT_MESSAGEduring streaming operationsThe current approach of assigning
DEFAULT_MESSAGEdirectly tofinal_response["message"]at line 74 and 114 creates a shared reference that could be mutated, leading to cross-contamination between different streaming operations.Apply this fix to create a deep copy of
DEFAULT_MESSAGE:+import copy + DEFAULT_MESSAGE = { "content": [], "role": "assistant", "tool_calls": [], "tool_plan": "", # TODO: Add citations }Then update lines 74 and 114:
- "message": DEFAULT_MESSAGE, + "message": copy.deepcopy(DEFAULT_MESSAGE),
111-124: Missing try/finally in async v2 handlerThe async v2 handler (lines 111-144) doesn't have a try/finally block like the sync version (lines 71-107), meaning spans could leak if an exception occurs during iteration.
Wrap the async for loop in a try/finally block:
async def aprocess_chat_v2_streaming_response(span, event_logger, llm_request_type, response): final_response = { "finish_reason": None, - "message": DEFAULT_MESSAGE, + "message": copy.deepcopy(DEFAULT_MESSAGE), "usage": {}, "id": "", "error": None, } current_content_item = {"type": "text", "thinking": None, "text": ""} current_tool_call_item = { "id": "", "type": "function", "function": {"name": "", "arguments": "", "description": ""}, } + try: async for item in response: span.add_event(name=f"{SpanAttributes.LLM_CONTENT_COMPLETION_CHUNK}") item_to_yield = item try: _accumulate_stream_item(item, current_content_item, current_tool_call_item, final_response) except Exception: pass yield item_to_yield set_span_response_attributes(span, final_response) if should_emit_events(): emit_response_events(event_logger, llm_request_type, final_response) elif should_send_prompts(): _set_span_chat_response(span, final_response) if final_response.get("error"): span.set_status(Status(StatusCode.ERROR, final_response.get("error"))) span.record_exception(final_response.get("error")) else: span.set_status(Status(StatusCode.OK)) - span.end() + finally: + span.end()
🧹 Nitpick comments (4)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (3)
101-103: Record proper exception objects instead of strings
span.record_exception()expects an Exception object, butfinal_response.get("error")might be a string or other non-exception type. This could cause issues with telemetry backends.Wrap the error in an exception if it's not already one:
if final_response.get("error"): span.set_status(Status(StatusCode.ERROR, final_response.get("error"))) - span.record_exception(final_response.get("error")) + error = final_response.get("error") + if isinstance(error, Exception): + span.record_exception(error) + else: + span.record_exception(Exception(str(error)))Apply the same fix to line 141 in the async version.
151-151: Fallback to copy of DEFAULT_MESSAGE instead of empty dictAt line 151, when there's no message in the delta, the code falls back to
{**DEFAULT_MESSAGE}, which creates a shallow copy. The nested lists (contentandtool_calls) are still shared references.Use deep copy for proper isolation:
- final_response["message"] = (item_dict.get("delta") or {}).get("message") or {**DEFAULT_MESSAGE} + final_response["message"] = (item_dict.get("delta") or {}).get("message") or copy.deepcopy(DEFAULT_MESSAGE)
28-29: Remove unnecessary f-string formattingLine 28 uses an f-string but doesn't include any variables to format.
- span.add_event(name=f"{SpanAttributes.LLM_CONTENT_COMPLETION_CHUNK}") + span.add_event(name=SpanAttributes.LLM_CONTENT_COMPLETION_CHUNK)Apply the same fix to lines 53, 87, and 126.
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (1)
287-288: Missing span.end() before re-raising exception in async handlerIn the async exception handler,
span.end()is called at line 287 but the span is managed by the context manager and should not be manually ended here.Remove the manual
span.end()call as the context manager handles it:except Exception as e: if span.is_recording(): span.set_status(Status(StatusCode.ERROR, str(e))) span.record_exception(e) - span.end() raise
📜 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 selected for processing (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py(7 hunks)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py(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-cohere/opentelemetry/instrumentation/cohere/__init__.pypackages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (3)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/utils.py (4)
dont_throw(13-35)should_send_prompts(38-41)to_dict(64-77)should_emit_events(44-49)packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/span_utils.py (2)
set_span_response_attributes(166-244)_set_span_chat_response(247-288)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
SpanAttributes(64-261)
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py
258-258: Unused function argument: instance
(ARG001)
343-343: Unused method argument: kwargs
(ARG002)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py
91-92: try-except-pass detected, consider logging the exception
(S110)
91-91: Do not catch blind exception: Exception
(BLE001)
130-131: try-except-pass detected, consider logging the exception
(S110)
130-130: Do not catch blind exception: Exception
(BLE001)
⏰ 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.10)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (2)
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py (2)
321-328: LGTM! Good defensive error handling for instrumentationThe try/except blocks properly handle missing modules or attributes during instrumentation without crashing the entire process, and log failures at debug level for troubleshooting.
234-241: ```shell
#!/bin/bash
set -euo pipefailROOT="packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere"
echo "PWD: $(pwd)"
echoif [ -f "$ROOT/init.py" ]; then
echo "=== Showing $ROOT/init.py (up to 400 lines) ==="
if command -v sed >/dev/null 2>&1; then
sed -n '1,400p' "$ROOT/init.py"
else
head -n 400 "$ROOT/init.py" || true
fi
else
echo "File not found: $ROOT/init.py"
fiecho
echo "=== Searching for @dont_throw decorator ==="
if command -v rg >/dev/null 2>&1; then
rg -n "@dont_throw" --hidden || true
else
grep -RIn "@dont_throw" packages || true
fiecho
echo "=== Searching for span.end occurrences ==="
if command -v rg >/dev/null 2>&1; then
rg -n "span.end(" --hidden -S -C3 || true
else
grep -RIn "span.end(" packages || true
fiecho
echo "=== Searching for record_exception occurrences ==="
if command -v rg >/dev/null 2>&1; then
rg -n "record_exception" --hidden -S -C3 || true
else
grep -RIn "record_exception" packages || true
fiecho
echo "=== Searching for except Exception blocks that reference span ==="
if command -v rg >/dev/null 2>&1; then
rg -nP "except\s+Exception[\s\S]{0,200}span." --hidden -S || true
rg -n "except Exception" --hidden -S -C3 || true
else
grep -RIn "except Exception" packages || true
fiecho
echo "=== Searching for functions (def / async def) that take 'response' and reference span ==="
if command -v rg >/dev/null 2>&1; then
rg -nP "def\s+\w+([^)]response[^)]):[\s\S]{0,200}span." --hidden -S || true
rg -nP "async def\s+\w+([^)]response[^)]):[\s\S]{0,200}span." --hidden -S || true
else
grep -RIn "def " packages | rg "response" || true
fiecho
echo "=== Listing .py files under packages/opentelemetry-instrumentation-cohere ==="
if [ -d "packages/opentelemetry-instrumentation-cohere" ]; then
find packages/opentelemetry-instrumentation-cohere -type f -name "*.py" -print || true
else
echo "directory packages/opentelemetry-instrumentation-cohere not found"
fi</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| if to_wrap.get("stream_process_func"): | ||
| return to_wrap.get("stream_process_func")(span, event_logger, llm_request_type, response) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify stream processor functions handle all edge cases
The streaming delegation correctly passes control to specialized stream processors. Ensure these processors handle early cancellation, connection drops, and other streaming edge cases.
🏁 Script executed:
#!/bin/bash
# Check if stream processors handle early cancellation and exceptions
ast-grep --pattern 'def $_(span, $_, $_, response):
$$$
try:
$$$
finally:
span.end()
$$$'
# Check test coverage for streaming edge cases
rg -A5 -B5 "test.*stream.*cancel|test.*stream.*error|test.*stream.*interrupt" --type=pyLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Show file around lines 200-320 ==="
nl -ba packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py | sed -n '1,400p' || true
echo "=== Search for 'stream_process_func' occurrences ==="
rg -n "stream_process_func" -S || true
echo "=== Search for 'stream' references in the package ==="
rg -n --hidden --no-ignore -S "stream" packages/opentelemetry-instrumentation-cohere || true
echo "=== Search for function defs containing 'stream' ==="
rg -n --type py "def\s+.*stream" -S || true
echo "=== Search tests for streaming/cancel/error keywords ==="
rg -n --type py "stream.*cancel|stream.*error|stream.*interrupt|cancel.*stream|interrupt.*stream|stream.*exception" -S || trueLength of output: 58215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== streaming.py (cohere) ==="
awk '{printf("%5d: %s\n", NR, $0)}' packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py || true
echo "=== __init__.py (cohere) ==="
awk '{printf("%5d: %s\n", NR, $0)}' packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/__init__.py || true
echo "=== Relevant test snippets (tests/test_chat.py head) ==="
awk '{printf("%5d: %s\n", NR, $0)}' packages/opentelemetry-instrumentation-cohere/tests/test_chat.py | sed -n '1,260p' || true
echo "=== Search for try/finally/exception handling in streaming.py ==="
rg -n --hidden --no-ignore -S "try:|except |finally|span.end\(|stream-end|is_finished" packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py || true
echo "=== Search for tests covering streaming exceptions/cancellation in cohere tests ==="
rg -n --hidden --no-ignore -S "exception|cancel|not_consumed|partial_consumption|interrupt|disconnect|connection|broken|stream.*exception" packages/opentelemetry-instrumentation-cohere/tests || trueLength of output: 39167
Guarantee span.end() and robust cancellation/error handling for async v2 streaming
- aprocess_chat_v2_streaming_response is missing a try/finally and only calls span.end() on the success path — file packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py (async def aprocess_chat_v2_streaming_response, ~lines 111–144). Wrap the async iteration in try/finally to always end the span and record exceptions consistently (match the other processors that use finally).
- Add unit tests that simulate early cancellation/connection drops and exceptions during iteration in packages/opentelemetry-instrumentation-cohere/tests/test_chat.py (no cohere tests currently cover these edge cases).
| except Exception: | ||
| pass |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Log exceptions during stream accumulation instead of silently swallowing them
The except Exception: pass pattern at lines 91-92 and 130-131 silently swallows exceptions that could indicate data parsing issues. This makes debugging difficult and could lead to incomplete telemetry data.
Consider logging these exceptions at debug level:
+import logging
+logger = logging.getLogger(__name__)Then update the exception handlers:
- except Exception:
- pass
+ except Exception as e:
+ logger.debug(f"Failed to accumulate stream item: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception: | |
| pass | |
| import logging | |
| logger = logging.getLogger(__name__) | |
| except Exception as e: | |
| logger.debug(f"Failed to accumulate stream item: {e}") |
🧰 Tools
🪛 Ruff (0.12.2)
91-92: try-except-pass detected, consider logging the exception
(S110)
91-91: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-cohere/opentelemetry/instrumentation/cohere/streaming.py
around lines 91-92 and 130-131, replace the silent "except Exception: pass"
handlers with logged exceptions: catch the exception as a variable (e.g., except
Exception as e) and emit a debug-level log that includes a clear context message
and the exception details/stack (using the module logger and logger.debug(...,
exc_info=True) or equivalent), so parsing/accumulation errors are recorded
without changing control flow.
2 participants
I did not instrument generate_stream (completions) because they literally discontinue it tomorrow
feat(instrumentation): ...orfix(instrumentation): ....Important
Adds support for Cohere API v2 with async and streaming capabilities, updates tests, and introduces new functions for handling input and response content.
__init__.py, including async and streaming capabilities.generate_streamdue to deprecation.process_chat_v1_streaming_responseandprocess_chat_v2_streaming_responseinstreaming.pyfor handling streaming responses.set_input_content_attributesandset_response_content_attributesinspan_utils.pyfor handling input and response content.test_chat.py,test_embed.py, andtest_rerank.pyto cover v2 API, async, and streaming functionalities.coheredependency to^5.18.0inpyproject.toml.pytest-asyncioto test dependencies inpyproject.toml.This description was created by
for 96db843. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores