Conversation
Added upstream 4 months ago: prometheus/prometheus#13060
rfratto
left a comment
There was a problem hiding this comment.
LGTM, left a few comments about how to set it to true by default if that's what you think it should be.
| Scheme: "http", | ||
| HonorLabels: false, | ||
| HonorTimestamps: true, | ||
| TrackTimestampsStaleness: false, |
There was a problem hiding this comment.
@bboreham If you want to set the default to true, this is the place to do it.
| `enable_protobuf_negotiation` | `bool` | Whether to enable protobuf negotiation with the client. | `false` | no | ||
| `honor_labels` | `bool` | Indicator whether the scraped metrics should remain unmodified. | `false` | no | ||
| `honor_timestamps` | `bool` | Indicator whether the scraped timestamps should be respected. | `true` | no | ||
| `track_timestamps_staleness` | `bool` | Indicator whether to track the staleness of the scraped timestamps. | `false` | no |
There was a problem hiding this comment.
@bboreham If you want to change the default to true, this also needs to be updated.
Yes I absolutely do, this is the correct setting for most people. There was a lot of debate at #5972 (review) |
|
However it may be appropriate to merge this PR, which is just allowing the config, then do a separate PR where we start arguing again. |
Sure, if you think that's easier 👍 merging |
clayton-cornell
left a comment
There was a problem hiding this comment.
Docs are fine as-is
Added upstream 4 months ago: prometheus/prometheus#13060 Co-authored-by: Paulin Todev <paulin.todev@gmail.com>
PR Description
Added upstream 4 months ago: prometheus/prometheus#13060
Which issue(s) this PR fixes
#5921
Notes to the Reviewer
I have cherry-picked the commit originally in #5972; I did not yet change the default to true, because I'm unsure where that would go.
PR Checklist