Instrument OpenAI Responses.retrieve and AsyncResponses.retrieve#184
Instrument OpenAI Responses.retrieve and AsyncResponses.retrieve#184JacksonWeber wants to merge 4 commits into
Conversation
These fetch a stored response by id without going through create(), so the existing create wrappers don't cover them. Wrap both sync and async, reusing the create wrappers since the result is the same Response shape. Add sync/async tests and cassettes. Refs open-telemetry#141 Assisted-by: Claude Opus 4.8 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover the 404 NotFoundError path for sync and async retrieve, matching the happy-path-plus-error coverage of the other Responses operations. Assisted-by: Claude Opus 4.8 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds OpenAI Responses retrieve() coverage to the existing opentelemetry-instrumentation-genai-openai package so stored responses fetched by ID emit the same GenAI telemetry as create(), including content-capture behavior and error recording.
Changes:
- Wrap
openai.resources.responses.responses.Responses.retrieveandAsyncResponses.retrieveusing the existingresponses_create/async_responses_createwrappers. - Add sync + async unit tests for
retrieve()covering basic attributes, content capture, and 404 errors (with new VCR cassettes). - Add a towncrier changelog fragment documenting the new API coverage.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| instrumentation/opentelemetry-instrumentation-genai-openai/src/opentelemetry/instrumentation/genai/openai/init.py | Instruments/uninstruments Responses.retrieve and AsyncResponses.retrieve using existing response wrappers. |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/test_responses.py | Adds sync retrieve() tests for basic attributes, content capture, and API error behavior. |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/test_async_responses.py | Adds async retrieve() tests for basic attributes, content capture, and API error behavior (explicit cassette usage). |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/cassettes/test_responses_retrieve_basic[content_mode0].yaml | VCR cassette for sync retrieve() happy path (no content capture). |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/cassettes/test_responses_retrieve_captures_content[content_mode0].yaml | VCR cassette for sync retrieve() happy path with output content capture. |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/cassettes/test_responses_retrieve_api_error[content_mode0].yaml | VCR cassette for sync retrieve() 404 / NotFoundError path. |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/cassettes/test_async_responses_retrieve_basic[content_mode0].yaml | VCR cassette for async retrieve() happy path (no content capture). |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/cassettes/test_async_responses_retrieve_captures_content[content_mode0].yaml | VCR cassette for async retrieve() happy path with output content capture. |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/cassettes/test_async_responses_retrieve_api_error[content_mode0].yaml | VCR cassette for async retrieve() 404 / NotFoundError path. |
| instrumentation/opentelemetry-instrumentation-genai-openai/.changelog/184.added | Changelog fragment for the new retrieve() instrumentation coverage. |
| span.attributes[GenAIAttributes.GEN_AI_OUTPUT_MESSAGES], | ||
| format_simple_expected_output_message(response.output_text), | ||
| ) | ||
| # retrieve has no request-side input messages to capture. |
There was a problem hiding this comment.
which operation name it's reported as? This should not be an inference span - no inference has happened, no tokens burned. It seems it looks like chat, but it does not seem right.
We need to add conventions for this first.
There was a problem hiding this comment.
Created a follow-up PR that adds the necessary work to genai semantic conventions: open-telemetry/semantic-conventions-genai#353
There was a problem hiding this comment.
Should we close this until that pr is merged?
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
Adds instrumentation for the OpenAI Responses
retrieveAPI (sync and async), addressing the OpenAI item in #141.Responses.retrieve/AsyncResponses.retrievefetch a stored response by id without going throughcreate(), so the existing create wrappers don't cover them. Both are now wrapped (reusing the create wrappers, since the result is the sameResponseshape) and unwrapped on uninstrument.Note:
retrieveonly takes aresponse_id, so the span has nogen_ai.request.model— all attributes come from the fetched response.Tests: sync/async basic + content-capture cases with cassettes.
Refs #141