Disallowing configure AM with the v1 api#13883
Conversation
|
Thanks for the PR! |
|
Yeah.. that's fair. One can update only prometheus and keep using a old version os AM I guess. But this API is being deprecated for long time and the default is the V2 since 2019. Im ok either way. |
|
I think it's safer to wait until v3, we can continue living with it. |
|
The API stability page says: Some features, which are cosmetic, still under development, or depend on 3rd party services, are not covered by this. I therefore think we can get rid of the V1 alertmanager alerts format. |
Thanks, I forgot about that page, but it's so vague that I bet we can identify a "3rd party service" for every user for every Prometheus feature :) |
machine424
left a comment
There was a problem hiding this comment.
I think we can have this for 3.0
cc @jan--f
|
Yeah, would be cool to get this into v3 to avoid any discussion if this needs a major version bump or not. |
jan--f
left a comment
There was a problem hiding this comment.
This looks good once @machine424 's comments are addressed.
I think this would be fine to go into v3. However I do want to note that iiuc this is the first time we introduce a dependency to a minimal alertmanager version.
Imo its fine to document this in various places (config comments, release notes, migration document) but others might disagree.
|
[this is a copy-paste message on all unfinished breaking change items] |
|
@alanprot any update? |
|
since this is mostly good to go other than the last comments, if @alanprot isn't able to get to this I can just adopt it and apply the notes as a second commit. |
|
Oh.. i missed that one.. i will try to address the comments later today, but if u wanna wrap up faster and create a new PR with the change im fine as well! :D |
|
No worries, just didn't want it to fall through the cracks. Thanks for wrapping it up! |
Signed-off-by: alanprot <alanprot@gmail.com>
Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com> Signed-off-by: Alan Protasio <alanprot@gmail.com>
Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com> Signed-off-by: Alan Protasio <alanprot@gmail.com>
b36d908 to
abfc8b5
Compare
Signed-off-by: alanprot <alanprot@gmail.com>
abfc8b5 to
b0fda91
Compare
machine424
left a comment
There was a problem hiding this comment.
lgtm, thanks.
just a final doc change.
Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com> Signed-off-by: Alan Protasio <alanprot@gmail.com>
Co-authored-by: Jan Fajerski <jan--f@users.noreply.github.com> Signed-off-by: Alan Protasio <alanprot@gmail.com>
|
🤞 haha :D |
* master: (667 commits) NHCB scrape: refactor state handling and speed up scrape test (prometheus#15193) feat(tools): add debug printouts to rules unit testing (prometheus#15196) docs: add keep_firing_for in alerting rules api: Add rule group pagination to list rules api (prometheus#14017) Update scrape/scrape.go benchmark, rename parser omtext_with_nhcb goimports run Better docstring on test Remove omcounterdata.txt as redundant Fix failing benchmarks Add unit test to show that current wrapper is sub-optimal Rename convert_classic_histograms to convert_classic_histograms_to_nhcb More followup to prometheus#15164 Follow up prometheus#15178 Followup to prometheus#15164 test(cmd/prometheus): speed up test execution by t.Parallel() when possible feat: normalize "le" and "quantile" labels values upon ingestion scrape: provide a fallback format (prometheus#15136) Disallowing configure AM with the v1 api (prometheus#13883) feat: ProtobufParse.formatOpenMetricsFloat: improve float formatting by using strconv.AppendFloat instead of fmt.Sprint ... # Conflicts: # go.mod # go.sum
AM V1 API is deprecated and removed.
This PR is just not allowing to configure AM with the V1 api anymore.