Skip to content

Conversation

@roidelapluie
Copy link
Member

No description provided.

@roidelapluie roidelapluie changed the title Add remote write 2 receivers complicance tests Add remote write 2 receivers compliance tests Sep 19, 2025
@roidelapluie
Copy link
Member Author

cc @bwplotka FYI, WIP

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Looks great! I think you gave us some ideas how to improve sender compliance (it tests too much IMO - it's literally scrape+export+Prometheus job/instance/staleness semantics, WDYT?)

What's your experience with the 2.0 protocol? Any feedback before we make it stable?

cc @pipiland2612 do you mind reviewing as well? (:

Copy link
Member

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

Very nice indeed.

I understand it's not fully finished, my list of items needed:

  • exemplars
  • created timestamp
  • different order of protobuf fields - this might be tricky, but for example I think Mimir right now would fail if the symbols table is after the timeseries

@roidelapluie
Copy link
Member Author

Looks great! I think you gave us some ideas how to improve sender compliance (it tests too much IMO - it's literally scrape+export+Prometheus job/instance/staleness semantics, WDYT?)

I think testing RW1 is really difficult because of the lack of headers, out of necessity it is required to also test that the remote is an actual remote.

What's your experience with the 2.0 protocol? Any feedback before we make it stable?

cc @pipiland2612 do you mind reviewing as well? (:

My main experience is that there is quite a few blind spots in the doc.

e.g.:
Senders MAY add ;proto= parameter to the header's value to indicate the fully qualified name of the Protobuf Message that was used,, but it seems like for RW2 message this should be a MUST. But these are minor things.

@roidelapluie
Copy link
Member Author

Pushed an update.

Results on cortex, mimir, prometheus:

https://roidelapluie.github.io/rwreport

@bwplotka
Copy link
Member

bwplotka commented Oct 6, 2025

Looks amazing! I'm encouraging @pipiland2612 to integrate and perhaps reuse index html table

@bwplotka
Copy link
Member

bwplotka commented Oct 6, 2025

e.g.:
Senders MAY add ;proto= parameter to the header's value to indicate the fully qualified name of the Protobuf Message that was used,, but it seems like for RW2 message this should be a MUST. But these are minor things.

Ups, will check, looks like a bug, thanks!

@roidelapluie roidelapluie force-pushed the rw2receiver branch 3 times, most recently from 03344a0 to 201d5db Compare October 13, 2025 14:17
@roidelapluie roidelapluie marked this pull request as ready for review October 13, 2025 14:18
@roidelapluie roidelapluie force-pushed the rw2receiver branch 8 times, most recently from 0e23f12 to 877727b Compare October 14, 2025 09:48
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
@roidelapluie
Copy link
Member Author

Ready for review :-)

@roidelapluie
Copy link
Member Author

Last test run: https://roidelapluie.github.io/rwreport/

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks!

@bwplotka bwplotka merged commit 7667b56 into prometheus:main Oct 24, 2025
7 checks passed
@bwplotka
Copy link
Member

BTW @roidelapluie I checked your feedback around:

e.g.:
Senders MAY add ;proto= parameter to the header's value to indicate the fully qualified name of the Protobuf Message that was used,, but it seems like for RW2 message this should be a MUST. But these are minor things.

I think the current form makes sense. It's MAY and no proto param means v1 for compatibility. That's why it's MAY.

Does it make sense?

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.

3 participants