feat(eap): Implement gen_ai attribute transformations#6201
feat(eap): Implement gen_ai attribute transformations#6201constantinius wants to merge 1 commit into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
constantinius
changed the title
| attributes.remove(GEN_AI__RESPONSE__TOOL_CALLS); | ||
|
|
||
| if parts.is_empty() { | ||
| return; |
There was a problem hiding this comment.
Response attrs dropped when empty
High Severity
When gen_ai.response.text and/or gen_ai.response.tool_calls are present but parsing yields no output parts (e.g. non-JSON tool calls like some_tool_calls, or JSON text with no usable content), the code still removes both deprecated keys and writes nothing to gen_ai.output.messages, so response data is lost instead of being moved or preserved like invalid gen_ai.request.messages.
Reviewed by Cursor Bugbot for commit aa94a95. Configure here.
|
@constantinius talked with the team, we'll look into dealing with the transaction normalization case in #6216 - Since you already removed the code for it, is this normalization required to run on transaction spans? So to get you unblocked I would propose we merge this change for the span streaming pipeline and we as a team take over the transaction change, if it is necessary. Also please remove the redundant information from the PR description, changed files and tests I can tell by the diff, even the functional changes I can see in code, the important information in the description and later commit is the motivation and the effect not the functional changes, which are already present in code. |
loewenheim
left a comment
loewenheim
left a comment
There was a problem hiding this comment.
This is a good start. Please feel free to ask if you need help with the requested changes.
There was a problem hiding this comment.
This shouldn't be necessary anymore, master has a more up to date version.
There was a problem hiding this comment.
This sentry-conventions PR contains the transformations. We should point to that version, once its merged.
4b56bc5 to
5b1564d
Compare
Implement the two attribute transformations from sentry-conventions PR #465: `gen_ai_request_messages_to_input_messages` and `gen_ai_response_to_output_messages`. These reshape attribute values beyond simple key renaming. `gen_ai.request.messages` with `content` fields is converted to `gen_ai.input.messages` with `parts` arrays. `gen_ai.response.text` (plain string, JSON string array, or objects with content) and `gen_ai.response.tool_calls` are combined into a single `gen_ai.output.messages` assistant message with typed parts. The transformation is generic over `AttributesLike` so it works on both `Attributes` (SpanV2) and `SpanData` (SpanV1) without duplication. It runs inside both `normalize_ai` (EAP pipeline) and `enrich_ai_span_data` (transaction/legacy pipeline). Attributes with the new `"transform"` deprecation status produce `WriteBehavior::CurrentName` so `normalize_attribute_names` leaves them alone — the dedicated transformation code handles the full move-and-reshape. Deprecated keys are always cleaned up, even when the canonical key already exists or the value cannot be parsed. Contributes to TET-2587
5b1564d to
8f8a074
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8f8a074. Configure here.
loewenheim
left a comment
loewenheim
left a comment
There was a problem hiding this comment.
This is a lot better. I have two comments, otherwise I'm good with merging this. If you like I can also take care of these.
| /// A message in the old `gen_ai.request.messages` format. | ||
| #[derive(Deserialize)] | ||
| struct OldMessage { | ||
| #[serde(default)] |
There was a problem hiding this comment.
This isn't necessary for Option.
| #[serde(default)] |
| // and let the rest carry the original fields. | ||
| return NewMessage { | ||
| role: None, | ||
| parts: vec![], |
There was a problem hiding this comment.
parts doesn't have skip_serializing_if. That means that in this case it will be serialized as parts: []. Not sure if that is intentional.
3 participants
Summary
Implements the two gen_ai attribute transformations introduced in sentry-conventions PR #465.
Old SDKs send AI messages and responses using deprecated attributes (
gen_ai.request.messages,gen_ai.response.text,gen_ai.response.tool_calls) whose value shapes differ from the canonical replacements (gen_ai.input.messages,gen_ai.output.messages). Simple key renaming is not enough — the values need reshaping to match the newparts-based message schema.These transformations run inside AI span normalization on both the SpanV1 (
enrich_ai_span_data) and SpanV2 (normalize_ai) paths, using theAttributesLikeabstraction. Attributes with the new"transform"deprecation status produceWriteBehavior::CurrentNamesonormalize_attribute_namesleaves them alone.Contributes to TET-2587