packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py (1)

24-29: Confirm and document version guard for reasoning support

Verified that OpenAI Python SDK v1.58.0 (released 2024-12-17) introduced the reasoning_effort parameter (github.com) and that the OpenAI API preview 2024-12-01-preview added the usage.completion_tokens_details.reasoning_tokens field (autotaker.github.io). The existing >= "1.58.0" check is accurate:

• SDK threshold: v1.58.0 (2024-12-17) – adds reasoning_effort to all chat methods
• API preview: 2024-12-01-preview – introduces reasoning_effort request param and reasoning_tokens in responses

If possible, prefer runtime feature detection (e.g., testing for response.usage.completion_tokens_details.reasoning_tokens or presence of the ChatCompletionReasoningEffort enum) for production behavior, and reserve the hardcoded version check for unit tests.

Please update the docstring on is_reasoning_supported() to reference:

• GitHub release notes for v1.58.0
• OpenAI Platform changelog entry for 2024-12-01-preview

packages/opentelemetry-instrumentation-openai/tests/traces/cassettes/test_chat/test_chat_reasoning.yaml (1)

83-90: Consider redacting organization/project identifiers.

Headers openai-organization and openai-project can be considered sensitive identifiers. Redact to minimize metadata leakage.

-      openai-organization:
-      - user-mktczbuqo14ok5zq3zvvus0l
+      # openai-organization:
+      # - REDACTED
-      openai-project:
-      - proj_HqO8HnKp7rJsjrDN6n3Y0TPc
+      # openai-project:
+      # - REDACTED

Optionally also mask x-request-id.

-      x-request-id:
-      - req_fb455d524b7f4775956fba99734cc8d9
+      # x-request-id:
+      # - REDACTED

packages/opentelemetry-instrumentation-openai/tests/traces/test_chat.py (1)

1498-1520: Use semconv constants in assertions and guard absent attributes.

Relying on string keys is brittle. Prefer SpanAttributes constants and .get() to avoid KeyError when a field is legitimately absent.

-    assert span.attributes["gen_ai.request.reasoning_effort"] == "low"
-    assert span.attributes["gen_ai.usage.reasoning_tokens"] > 0
+    assert span.attributes.get(SpanAttributes.LLM_REQUEST_REASONING_EFFORT) == "low"
+    assert span.attributes.get(SpanAttributes.LLM_USAGE_REASONING_TOKENS, 0) > 0

packages/opentelemetry-instrumentation-openai/tests/traces/cassettes/test_azure/test_chat_reasoning.yaml (1)

55-77: Optional: redact correlation IDs and internal deployment metadata.

Although not secrets, headers like apim-request-id, x-request-id, azureml-model-session, and x-ms-deployment-name create churn and leak internal topology. Redact to reduce noise and leakage surface.

-      apim-request-id:
-      - aebd8320-f701-4e7d-801f-2955f84e3811
-      azureml-model-session:
-      - d004-20250815200304
-      x-ms-deployment-name:
-      - gpt-5-nano
-      x-request-id:
-      - 7acf5821-70fa-4fab-b202-ba3700578d08
+      # apim-request-id: REDACTED
+      # azureml-model-session: REDACTED
+      # x-ms-deployment-name: REDACTED
+      # x-request-id: REDACTED

Also add cassette filtering (filter_headers=[...]) to your test config.

packages/opentelemetry-instrumentation-openai/tests/traces/test_azure.py (1)

767-789: Solid Azure reasoning coverage; consider aligning model naming for Azure deployments.

The assertions look good and validate the new attributes. Minor nit: all other Azure chat tests use the Azure deployment name ("openllmetry-testing") instead of a model name. For consistency and to reduce the chance of cassette mismatches in future re-recordings, consider using the same deployment alias here.

Apply if you want consistency with the rest of this file:

-        model="gpt-5-nano",
+        model="openllmetry-testing",

packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v1/responses_wrappers.py (1)

227-245: Avoid redundant f-strings for constant attribute keys.

Passing the constant directly is cleaner. Keeping the empty-tuple default since tests assert (). If you’re open to it later, consider None to avoid type ambiguity for string-typed attributes.

-    _set_span_attribute(
-        span,
-        f"{SpanAttributes.LLM_REQUEST_REASONING_SUMMARY}",
-        traced_response.request_reasoning_summary or (),
-    )
+    _set_span_attribute(
+        span,
+        SpanAttributes.LLM_REQUEST_REASONING_SUMMARY,
+        traced_response.request_reasoning_summary or (),
+    )
@@
-    _set_span_attribute(
-        span,
-        f"{SpanAttributes.LLM_REQUEST_REASONING_EFFORT}",
-        traced_response.request_reasoning_effort or (),
-    )
+    _set_span_attribute(
+        span,
+        SpanAttributes.LLM_REQUEST_REASONING_EFFORT,
+        traced_response.request_reasoning_effort or (),
+    )
@@
-    _set_span_attribute(
-        span,
-        f"{SpanAttributes.LLM_RESPONSE_REASONING_EFFORT}",
-        traced_response.response_reasoning_effort or (),
-    )
+    _set_span_attribute(
+        span,
+        SpanAttributes.LLM_RESPONSE_REASONING_EFFORT,
+        traced_response.response_reasoning_effort or (),
+    )

packages/opentelemetry-instrumentation-openai/tests/traces/test_responses.py (1)

155-179: Great end-to-end assertions for reasoning attributes; optional extra checks.

Consider also asserting gen_ai.response.id presence to mirror other tests, but current coverage already validates the essential attributes.

For example:

assert "gen_ai.response.id" in span.attributes

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py (1)

22-22: Good: robust version comparison using packaging.version.

Switching from string comparison to packaging.version.parse avoids subtle ordering bugs (e.g., 1.10 vs 1.9). LGTM.

packages/opentelemetry-instrumentation-openai/tests/traces/test_chat.py (1)

18-18: Import looks good.

Using is_reasoning_supported to gate reasoning tests avoids false failures on older client versions. Nice.

packages/opentelemetry-instrumentation-openai/tests/traces/test_azure.py (1)

4-4: Good guard: version-gated reasoning tests.

Importing and using is_reasoning_supported() to gate tests prevents false failures on older SDKs.

packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v1/responses_wrappers.py (1)

135-139: TracedData: new reasoning fields look correct.

Optional strings with None defaults match how the rest of the model is structured. No concerns here.

packages/opentelemetry-instrumentation-openai/tests/traces/test_responses.py (1)

6-6: Good use of version gating for new reasoning tests.

Keeps the suite green across OpenAI client versions.

packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v1/responses_wrappers.py (2)

206-214: LGTM on the F841 cleanup and explicit type handling for output_tokens_details.
Refactor matches earlier feedback and resolves the unused exception variable while making access explicit.

---

463-475: Nice E131 cleanup; suggest DRY’ing reasoning kwarg extraction.

The switch to implicit parentheses resolves Flake8 E131. To reduce repetition, bind reasoning = kwargs.get("reasoning") or {} once per block and reuse.

Example refactor (apply similarly in all four blocks):

-            traced_data = TracedData(
+            reasoning = kwargs.get("reasoning") or {}
+            traced_data = TracedData(
                 ...
-                request_reasoning_summary=(
-                    kwargs.get("reasoning", {}).get(
-                        "summary", existing_data.get("request_reasoning_summary")
-                    )
-                ),
-                request_reasoning_effort=(
-                    kwargs.get("reasoning", {}).get(
-                        "effort", existing_data.get("request_reasoning_effort")
-                    )
-                ),
-                response_reasoning_effort=kwargs.get("reasoning", {}).get("effort"),
+                request_reasoning_summary=reasoning.get(
+                    "summary", existing_data.get("request_reasoning_summary")
+                ),
+                request_reasoning_effort=reasoning.get(
+                    "effort", existing_data.get("request_reasoning_effort")
+                ),
+                response_reasoning_effort=reasoning.get("effort"),
             )

Also applies to: 526-538, 589-601, 653-665

packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/v1/responses_wrappers.py (3)

135-139: TracedData additions look good; consider stricter typing for “effort” and avoid sentinel defaults downstream.

Fields are appropriate. Optional: use Literal["low","medium","high"] for the two “effort” fields to catch typos at type-check time.

Example change (requires importing Literal):

-from typing import Any, Optional, Union
+from typing import Any, Optional, Union, Literal

-    request_reasoning_effort: Optional[str] = pydantic.Field(default=None)
-    response_reasoning_effort: Optional[str] = pydantic.Field(default=None)
+    request_reasoning_effort: Optional[Literal["low","medium","high"]] = pydantic.Field(default=None)
+    response_reasoning_effort: Optional[Literal["low","medium","high"]] = pydantic.Field(default=None)

---

357-360: Guard against None output_blocks in error paths.

In certain exception flows output_blocks can be None; iterating .values() will raise. Avoid relying on @dont_throw here.

Apply:

-        tool_call_index = 0
-        for block in traced_response.output_blocks.values():
+        tool_call_index = 0
+        if traced_response.output_blocks:
+            for block in traced_response.output_blocks.values():

---

226-244: Optional: align response_reasoning_effort source with the actual response (if available).

Currently mirrored from request kwargs. If OpenAI Responses ever returns a response-side effort, prefer that source for accuracy, falling back to the request setting.

I can add a guarded extraction from parsed_response (e.g., output blocks of type "reasoning" or a top-level field) if you confirm the shape to read from. Want me to open a follow-up PR?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.