Skip to content

exp/api/remote: validate confirmation before writing stats headers - #2153

Open
AdamMagued wants to merge 2 commits into
prometheus:mainfrom
AdamMagued:fix-issue-2129
Open

AdamMagued wants to merge 2 commits into
prometheus:mainfrom
AdamMagued:fix-issue-2129

Conversation

@AdamMagued

Copy link
Copy Markdown

Summary

In exp/api/remote/remote_headers.go, writeHeaders previously checked only msgType != WriteV1MessageType to decide whether to write the statistics headers (writtenSamplesHeader, writtenHistogramsHeader, writtenExemplarsHeader). Unconfirmed stats (such as default unpopulated stats instances) were emitted as zeros for non-v1 requests, which conflicted with PRW 2.0 confirmation semantics where missing headers indicate unconfirmed metrics.

This change refactors writeHeaders and the stats models:

  • Adds shouldWriteStatsHeaders checking message type validity, exclusion of v1 messages, stats confirmation status, and metric non-negativity.
  • Adds Confirmed and SetConfirmed methods on WriteResponseStats (promoted on WriteResponse).
  • Adds Validate method on WriteResponseStats to verify metric counts are non-negative.
  • Adds NewWriteResponseStats, NewConfirmedWriteResponseStats, and NewWriteResponseWithStats constructors.
  • Adds SetStats method on WriteResponse.
  • Preserves confirmation state across Add operations when either operand is confirmed.
  • Adds automated tests covering confirmed and unconfirmed statistics emission, validation, and parsing.

Fixes #2129

Refactor writeHeaders to check confirmation and validity of WriteResponseStats prior to emitting remote write statistics headers. Add constructors and helper methods on WriteResponse and WriteResponseStats to support setting and inspecting confirmation state.

Fixes prometheus#2129

Signed-off-by: AdamMagued <adamismailmageud@gmail.com>
Signed-off-by: AdamMagued <adamismailmageud@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

exp/api: Refactor writeHeaders to cleanly handle WriteResponseStats confirmation status

1 participant