Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename gen_ai.openai.request.response_format to gen_ai.request.response_format #1757

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

lmolkova
Copy link
Contributor

@lmolkova lmolkova commented Jan 17, 2025

Structured outputs and response format is used by multiple GenAI systems, so generalizing it.
Also fixing a tiny bug where we have different id and value for completion token types and other minor nits.

See Cohere, Azure AI Inference, Vertex AI docs

Merge requirement checklist

@lmolkova lmolkova changed the title Rename gen-ai.openai.request.response_format to gen-ai.request.response_format generic Rename gen-ai.openai.request.response_format to gen-ai.request.response_format Jan 17, 2025
@lmolkova lmolkova marked this pull request as ready for review January 17, 2025 17:35
@lmolkova lmolkova requested review from a team as code owners January 17, 2025 17:35
Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vertex AI/Gemini also supports response schema. I'll take a look and see if it works with this PR

Copy link
Contributor

@karthikscale3 karthikscale3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

docs/attributes-registry/gen-ai.md Outdated Show resolved Hide resolved
.chloggen/1757.yaml Outdated Show resolved Hide resolved
Comment on lines 117 to 118
| `json_object` | JSON object response format | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `json_schema` | JSON schema response format | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it wasn't changed in this PR, but what is the difference between these? Neither one is an actual "format"

Copy link
Contributor Author

@lmolkova lmolkova Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what OpenAI, Azure AI and Cohere do:

  • OpenAI /Azure AI Inference
    • {"type": "json_object"} - json without schema
    • {"type": "json_schema", "json_schema": {...}} - json with schema
  • Cohere
    • {"type": "json_object" } - json without schema
    • {"type": "json_object", "json_schema": {...}} - json with schema
  • Vertex AI (correct me if I'm wrong, reading this)
    • responseMimeType: application/json | text/plain | text/x.enum
    • responseSchema

I.e. if we apply existing things to vertex, it would be

  • check if responseMimeType is json
    • if responseSchema is set, then gen_ai.request.response_format=json_schema
    • otherwise gen_ai.request.response_format=json_object
  • if responseMimeType is text, gen_ai.request.response_format=text
  • in other cases, probably gen_ai.request.response_format={responseMimeType}

I guess the question is whether we should continue doing this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're reading of Vertex API is correct. Looking ahead, there is a separate API for generating images which also supports more mime types https://cloud.google.com/vertex-ai/generative-ai/docs/model-reference/imagen-api#output-options. I'm guessing gen_ai.request.response_format attribute would be shared for multimodal as well–what do you think of changing this attribute to a mime type?

Regarding json schema, I opened #1760 to support capturing the schema if available.

Copy link
Contributor Author

@lmolkova lmolkova Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing gen_ai.request.response_format attribute would be shared for multimodal as well–what do you think of changing this attribute to a mime type?

great point!

Just did a basic research on types:

I think mime-type won't work in some cases (e.g. image URL), but also it requires to map everything to a mime type. It's not straightforward - e.g. b64_json DALL-E produces seems to be webp but it's not documented (!?) and internet suggests to decode and look at the image header - not something instrumentation should do.

Seeing output_audio_format makes me think that there is a future where you could specify multiple formats in one request (generate video and audio, etc) and we might need different attributes to capture text_format, audio_format, etc.


I think I see two options:

  1. Use response_format as a union of all possible formats across modalities: (text | json | mp3 | url | .. )
    • we can define a few well-known types (e.g. json would be used for all json things - json_object, json_schema, application/json, etc)
    • for everything else it would match constant used by the specific system, Vertex can use audio/x-alaw-basic and openai would do g711_alaw, we don't really try to unify all
  2. Break it down into format-per-modality
    • gen_ai.request.text.response.format = plain | json | bson | xml | python
    • gen_ai.request.text.response.schema = {schema type name} - this is only for text and only for structured output
    • gen_ai.request.image.response_format = url | png | webp | b64_json - we can do some normalization for a few things and record the rest as is
    • gen_ai.request.image.response.aspect_ratio = 4:3 - there are other properties that would be worth recording
    • ...

Either way we could/should add something like
gen_ai.request.response.type = text | image | video | audio | ... - this is the modality

(all attribute names need polishing, but I hope you get the idea)

I hope we can also make it symmetrical with request format and the actual response output type.
Let me do a bit more research, but I'm leaning towards option 2 since it provides typed way to record modality-specific options

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we can also make it symmetrical with request format and the actual response output type. Let me do a bit more research, but I'm leaning towards option 2 since it provides typed way to record modality-specific options

Option 2 sounds good to me on cursory read through. It sounds like a bigger follow up task, so this PR LGTM if you want to address the rest later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced gen_ai.output.json.schema.name and gen_ai.output.type following option 2, let's discuss on the GenAI call tomorrow

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you push the updated code yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apparently no, did it now, thanks!

@lmolkova lmolkova changed the title Rename gen-ai.openai.request.response_format to gen-ai.request.response_format Rename gen_ai.openai.request.response_format to gen_ai.request.response_format Jan 18, 2025
@lmolkova lmolkova force-pushed the gen-ai-response-format-generic branch from 5e574a5 to 07f27d5 Compare January 23, 2025 21:51
@lmolkova lmolkova requested a review from TaoChenOSU January 23, 2025 21:52
# https://github.com/open-telemetry/semantic-conventions/pull/1757
- rename_attributes:
attribute_map:
gen_ai.openai.request.response_format: gen_ai.request.response_format
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inaccurate now.

@@ -0,0 +1,4 @@
change_type: breaking
component: gen-ai
note: "Rename `gen_ai.openai.request.response_format` to `gen_ai.request.response_format`"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inaccurate now.
As well, the PR title should change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants