Add a header to count retried remote write requests#12729
Add a header to count retried remote write requests#12729bboreham merged 5 commits intoprometheus:mainfrom
Conversation
Signed-off-by: Marc Tuduri <marctc@protonmail.com> Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com>
Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com>
storage/remote/client.go
Outdated
| httpReq.Header.Set("User-Agent", UserAgent) | ||
| httpReq.Header.Set("X-Prometheus-Remote-Write-Version", "0.1.0") | ||
| if attempt > 0 { | ||
| httpReq.Header.Set("X-Retry-Count", strconv.Itoa(attempt)) |
There was a problem hiding this comment.
AFAICT this isn't an official HTTP spec header right? If not, what do you think of calling it X-Retry-Attempt? To me, count here implies the remaining count of retries before we would give up.
Lets make sure we document the new retry options as part of the 1.1 spec. We should probably also define the header names via constants at some point.
There was a problem hiding this comment.
AFAICT this isn't an official HTTP spec header right?
No it is not. I tried looking but couldn't find one in an RFC or any widely-used conventions.
what do you think of calling it X-Retry-Attempt
Yeah, I'm changing it to that.
Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com>
|
CI is failing due to an unrelated test, could we please restart this check? |
cstyan
left a comment
There was a problem hiding this comment.
@tpaschalis tests look fine to me?
will merge soon, likely tomorrow
|
Can we get rid of the X-? I think X- prefix is deprecated in HTTP headers. |
Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com>
@roidelapluie yes, done! |
|
Looks like we hit another test flake here :( |
Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com>
Header name is `Retry-Attempt`, only set when >0. Signed-off-by: Marc Tuduri <marctc@protonmail.com> Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com>
Header name is `Retry-Attempt`, only set when >0. Signed-off-by: Marc Tuduri <marctc@protonmail.com> Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com>
Header name is `Retry-Attempt`, only set when >0. Signed-off-by: Marc Tuduri <marctc@protonmail.com> Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com> (cherry picked from commit c173cd5)
Header name is `Retry-Attempt`, only set when >0. Signed-off-by: Marc Tuduri <marctc@protonmail.com> Signed-off-by: Paschalis Tsilias <paschalis.tsilias@grafana.com> (cherry picked from commit c173cd5)
Similar to how we set the
HTTPResendCountattribute in the tracing span whenever sending a remote write batch, we could add a new header for retried requests, identifying the current attempt number.This would allow remote write backends to more intelligently respond to requests with a proper Retry-After header (eg. #12677).
In case we don't want to alter the Store interface, we can pass the attempt number as a value through the context as well.