write_handler: Use client_golang/exp/api/remote for handling remote writes#16086
write_handler: Use client_golang/exp/api/remote for handling remote writes#16086saswatamcode wants to merge 5 commits intoprometheus:mainfrom
Conversation
bwplotka
left a comment
There was a problem hiding this comment.
Thanks. We removed more than add, so our helper helped (:
LGTM 💪🏽
| Default("false").BoolVar(&cfg.web.EnableRemoteWriteReceiver) | ||
|
|
||
| supportedRemoteWriteProtoMsgs := config.RemoteWriteProtoMsgs{config.RemoteWriteProtoMsgV1, config.RemoteWriteProtoMsgV2} | ||
| supportedRemoteWriteProtoMsgs := remoteapi.MessageTypes{remoteapi.WriteV1MessageType, remoteapi.WriteV2MessageType} |
There was a problem hiding this comment.
We could replace config entries as well with remoteapi perhaps? 🤔
There was a problem hiding this comment.
Ok so I did most of the constants, but some still remain. I'll modify that in next PR for replacing client/sending code.
| github.com/ovh/go-ovh v1.6.0 | ||
| github.com/prometheus/alertmanager v0.28.0 | ||
| github.com/prometheus/client_golang v1.21.0-rc.0 | ||
| github.com/prometheus/client_golang/exp v0.0.0-20250227122456-ad23ad6d5468 |
There was a problem hiding this comment.
Wanna go ahead and make v0.1.0 release on client_golang? Likely the tag has to be in form of exp v0.1.0 (https://stackoverflow.com/a/77864755) if you want.
There was a problem hiding this comment.
Yup totally. I think https://github.com/hashicorp/vault/tags does a similar thing
There was a problem hiding this comment.
Let's hold off tagging, until I have PR for replacing some of the client code, that way, we can release v0.1.0 with exp we know works.
…rites Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
|
Why would we rely on something "experimental" to implement something declared stable? |
|
@bboreham do you mean remote write protocol 1.0 stability? That's fair, the ecosystem relies on it. However the stability is for API, not for quality of the code. For this work we want to ensure the wider ecosystem can use the same stable behaviours for both remote write 1.0, 2.0 and beyond. In order to do so we extracted relevant pieces to an external API which is now experimental as we may shape the API a bit (and since we are in the same team, making sure it's working for Prometheus well). This means only more stability for projects like Mimir, Thanos, Cortex and senders like Alloy, Tempo, avalanche, etc To sum up, we don't change any protocol behaviours here AFAIK (we carefully checked that). If you are worried about the risk, we don't risk anything more than when we were (and will be) adjusting both the handler and writer for 2.0 and future work here. WDYT? |
|
Superseded by #17197 🙂 |
This PR uses the new client_golang experimental remote API package, introduced in prometheus/client_golang#1658 to handle remote writes (v1 and v2).
Also, removes a couple of test cases around missing headers/no content-type as those are now handled by the exp write handler (assumes V1 and snappy as defaults).
cc: @bwplotka