Skip to content

export ValidateMatcher for DI#2694

Closed
kylebrandt wants to merge 80 commits intoprometheus:mainfrom
grafana:matcherValidation
Closed

export ValidateMatcher for DI#2694
kylebrandt wants to merge 80 commits intoprometheus:mainfrom
grafana:matcherValidation

Conversation

@kylebrandt
Copy link
Contributor

so third parties, Grafana in particular, can over ride the validation.

Grafana wants to do this because other data sources will have label keys with things like spaces, periods, or other characters - and looking for a better integration with alert manager.

goes with grafana/grafana#38629

kylebrandt added a commit to grafana/grafana that referenced this pull request Sep 7, 2021
@roidelapluie
Copy link
Member

it is really unclear to me what direction grafana is taking. At some point, it might be better to just use a fork of alertmanager rather than adding abstraction layers over all the code...

i think that you could just work with the alertmanager objects, preprare them before passing them here.

it will be very confusing for users if grafana's "alertmanager" behaves differently than alertmanager.

@roidelapluie
Copy link
Member

Especially since Alertmanager has not reached 1.x yet. If we miss some concepts or if there is room for improvements, we could discuss and possibly integrate those features directly in alertmanager :)

@kylebrandt
Copy link
Contributor Author

kylebrandt commented Sep 14, 2021

@roidelapluie Sorry I didn't mean to make a draft PR in this repo yet (meant to use Grafana's but had to many GH tabs), I can close if it is a distraction - as this is still a bit exploratory (just let me know).

The objective with this branch is to allow Label Keys to be able to contain characters beyond the Open API / Prometheus label key restrictions. This is to make so alert manager can be used with other TSDBs (or things acting like TSDBs). Or more on point, other data sources that Grafana supports. (e.g. elastic has things like periods in the name, or other languages).

Renaming label keys to compatible was considered, but then then creating things like links in templates back to other systems doesn't work out.

So far in my exploration this seems to work if I:

  1. Bypass label key validation
  2. Take the matchers in as objects in the config so they do not need to be parsed (like the silence pb already seems to be).

@gotjosh is far more familiar with the alert manage side as well so can maybe add some context here.

@roidelapluie
Copy link
Member

I have boostrapped a discussion on the grafana-developers mailing list.

kylebrandt and others added 21 commits September 20, 2021 11:43
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
…when group_wait is 0s

Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Marcel Hoegel <github@mclhgl.de>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
…ementing limits on alerts.

Update provider/mem/mem.go

Co-authored-by: Julien Pivotto <roidelapluie@gmail.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Limits are not used in standalone alertmanager.

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
…er of alive goroutines.

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Fix JSON marshalling of empty Matchers.

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* Implements a Grafana dashboard to the mixin.

The dashboard aims to show an overview of the overall health of Alertmanager.

Signed-off-by: ArthurSens <arthursens2005@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Sebastian Neuner <neuner@belwue.de>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
The expression alertmanager_cluster_members{job="alertmanager"}[5m]) is assumed to return
one series for each alertmanager instance in the cluster. When running inside Kubernetes,
alertmanager pods can get evicted and rescheduled. This can change the instance label and
produce a new series for that alertmanager instance.

When the same pod gets evicted several times in a row, there will be a short interval in which
Prometheus will return values from both the new series and the old series.
As a result, counting the number of series for the alertmanager_cluster_members metric
will overestimate the number of instances in the given cluster.

This commit modifies the the AlertmanagerMembersInconsistent alert to increase the for clause to 15m
in order to reduce the probability of a false positive.

Signed-off-by: fpetkovski <filip.petkovsky@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Tyler Reid and others added 24 commits September 20, 2021 11:43
This reverts commit 4c2a5f1.

Signed-off-by: Tyler Reid <tyler.reid@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
…blish input to send. Check we populate a region for all requests.

This reverts commit 4c2a5f1.

Signed-off-by: Tyler Reid <tyler.reid@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Ben Ridley <benridley29@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Ben Ridley <benridley29@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Ben Ridley <benridley29@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Ben Ridley <benridley29@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* target_match and source_match are DEPRECATED

Signed-off-by: Dmitry Tolstoy <A-styler@ya.ru>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* OpsGenie provides management of both alerts and incidents.
* package opsgenie uses OpsGenie alert api, so let's refer to
  alert instead of incident in logs and doc to avoid confusion.

Signed-off-by: Laurent CREPET <l.crepet@criteo.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* cli: add new template render command

Add a new template rendering command that allows users to test out their
templates. This is especially needed because small bugs in templates do
not surface until alertmanager actually tries to render them.

* cli: permit passing alert data via a file

Add a new parameter `--templatefile` for `amtool` so that it would be
possible to pass custom alert data. Use an example `template.Data` if
none has been passed to permit simple use-cases.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
This is the best we can do to make amtool support old releases.

Supersedes #2634
Fix #2666

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* Add ability to skip TLS verification for amtool

Signed-off-by: Nikita Nedvetskii <72229464+nedvna@users.noreply.github.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* amtool: Detect version drift and warn users

This change detects the alertmanager version when initiating the client.
It ignores most errors since I expect amtool to fail later.

If amtool is not compiled with proper version, we do not do anything
either.

We use MajorMinor for now as we have not reach 1.0, but we still allow
the bugfix version number (Z in x.y.Z) to differ.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>

* Add version check

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* Add TLS option to gossip cluster

Co-authored-by: Sharad Gaur <sharadgaur@gmail.com>
Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* generate new certs that expire in 100 years

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* Fix tls_connection attributes

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* Improve error message

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* Fix tls client config docs

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* Add capacity arg to message buffer

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* fix formatting

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* Update version; add version validation

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* use lru cache for connection pool

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* lock reading from the connection

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* when extracting net.Conn from tlsConn, lock and throw away wrapper

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* Add mutex to connection pool to protect cache

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

* fix linting

Signed-off-by: Dustin Hooten <dustinhooten@gmail.com>

Co-authored-by: Sharad Gaur <sharadgaur@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: machinly <machinly@acm.org>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: machinly <machinly@acm.org>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: machinly <machinly@acm.org>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
* Enable support for custom callbacks as part of maintenance

This enables support for custom Maintenance callbacks as part of the periodic maintenance of silences and notification logs.
Effectively a no-op for the Alertmanager but allows downstream implementation to inject custom logic as part of it.

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Add tests

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix tests and remove whitespace

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Address review comments

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* run go fmt

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix import ordering

Signed-off-by: gotjosh <josue.abreu@gmail.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Kyle Brandt <kyle@grafana.com>
@kylebrandt
Copy link
Contributor Author

replaced by #2716

@kylebrandt kylebrandt closed this Sep 23, 2021
roidelapluie pushed a commit that referenced this pull request Oct 21, 2021
so third parties, Grafana in particular, can over ride the validation.

Grafana wants to do this because other data sources will have label keys with things like spaces, periods, or other characters - and looking for a better integration with alert manager.

goes with grafana/grafana#38629
replaces #2694

Signed-off-by: Kyle Brandt <kyle@grafana.com>
markoposavec pushed a commit to markoposavec/alertmanager that referenced this pull request Nov 4, 2021
so third parties, Grafana in particular, can over ride the validation.

Grafana wants to do this because other data sources will have label keys with things like spaces, periods, or other characters - and looking for a better integration with alert manager.

goes with grafana/grafana#38629
replaces prometheus#2694

Signed-off-by: Kyle Brandt <kyle@grafana.com>
markoposavec pushed a commit to markoposavec/alertmanager that referenced this pull request Nov 4, 2021
so third parties, Grafana in particular, can over ride the validation.

Grafana wants to do this because other data sources will have label keys with things like spaces, periods, or other characters - and looking for a better integration with alert manager.

goes with grafana/grafana#38629
replaces prometheus#2694

Signed-off-by: Kyle Brandt <kyle@grafana.com>
Signed-off-by: Marko Posavec <Marko.Posavec@infobip.com>
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.