Skip to content

fix(tracing): don't record an empty streaming model as llm.model_name - #2444

Open
jeremypng wants to merge 2 commits into
envoyproxy:mainfrom
jeremypng:fix-streaming-span-model-name
Open

fix(tracing): don't record an empty streaming model as llm.model_name#2444
jeremypng wants to merge 2 commits into
envoyproxy:mainfrom
jeremypng:fix-streaming-span-model-name

Conversation

@jeremypng

Copy link
Copy Markdown

Description

On streaming chat completions, convertSSEToJSON took the model from the first SSE chunk only. Azure OpenAI's leading content-filter chunk carries model:"", so the span's llm.model_name was overwritten with an empty string (the request-time value is last-write-lost), breaking cost attribution in OpenInference consumers.

This fixes it by capturing the first non-empty model across all SSE chunks in the stream, so a leading empty-model chunk (or any later chunk reporting a different value) can't clobber it. It also adds the same resp.Model != "" guard to buildResponseAttributes that buildResponsesResponseAttributes already has for the Responses API path, so an empty response model can never overwrite the request-time llm.model_name attribute.

This intentionally differs from the translator's streamingResponseModel (used for metrics), which takes the last non-empty value across the stream. For tracing, taking the first non-empty value is a defensive choice: it means a later, possibly-anomalous chunk (e.g. a routed fallback reporting a different model) can't silently override the model that was already correctly captured earlier in the stream.

Two behavioral guarantees after this change:

  • Streamed spans carry the upstream-resolved model as soon as any chunk in the stream reports one, regardless of chunk order.
  • An empty model value never overwrites the request-time llm.model_name value, on either the streaming or non-streaming Chat Completions path.

Related Issues/PRs (if applicable)

Fixes #2443

Special notes for reviewers (if applicable)

  • Test coverage added (internal/tracing/openinference/openai):
    1. TestBuildResponseAttributesEmptyModelOmitsModelName (response_attrs_test.go) — an empty response model must not emit llm.model_name at all.
    2. sse_converter_test.go, case "model from first non-empty chunk (Azure content-filter prologue)" — model comes from the first chunk that actually carries one, skipping the empty-model prologue chunk.
    3. sse_converter_test.go, case "model retains first non-empty value even if a later chunk differs" — a later chunk reporting a different model does not override the first non-empty value.
  • Commit is signed off (DCO) per CONTRIBUTING.md.
  • Generative AI disclosure: this fix and its tests were prepared with AI assistance (Claude); I've reviewed and understand the change and take full ownership of it, per CONTRIBUTING.md's generative AI policy.

On streaming chat completions, convertSSEToJSON took the model from the
first SSE chunk only. Azure OpenAI's leading content-filter chunk carries
model:"", so the span's llm.model_name was overwritten with an empty
string (the request-time value is last-write-lost), breaking cost
attribution in OpenInference consumers. Capture the first non-empty model
across chunks so a leading empty-model chunk (or any later chunk reporting
a different value) can't clobber it — this intentionally differs from the
translator's streamingResponseModel, which takes the last non-empty value
across the stream. Also guard the attribute write like the Responses API
path already does.

Signed-off-by: Jeremy Sanders <23668453+jeremypng@users.noreply.github.com>
@jeremypng
jeremypng requested a review from a team as a code owner July 28, 2026 10:05
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 28, 2026

@nacx nacx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@nacx
nacx enabled auto-merge (squash) July 30, 2026 11:06
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.86%. Comparing base (35bde0c) to head (ecaa537).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2444      +/-   ##
==========================================
- Coverage   84.87%   84.86%   -0.02%     
==========================================
  Files         154      154              
  Lines       22419    22423       +4     
==========================================
+ Hits        19028    19029       +1     
- Misses       2236     2238       +2     
- Partials     1155     1156       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Streaming chat-completion spans record empty llm.model_name when the first SSE chunk carries no model

3 participants