Add alertmanager provider #258
Conversation
Alan01252
force-pushed
the
feature/alertmanager
branch
from
10713de
to
6d637c9
Compare
stefanprodan
added
area/alerting
Alan01252
force-pushed
the
feature/alertmanager
branch
2 times, most recently
from
169f4a0
to
5ed5de8
Compare
|
Should be good now @stefanprodan apologies for the many rebases again... |
Alan01252
force-pushed
the
feature/alertmanager
branch
from
5ed5de8
to
ff0fa4d
Compare
docs/spec/v1beta1/provider.md
Outdated
|
|
||
| | Label | Description | | ||
| | ----------- | ------------------------------------------------------------------------------ | | ||
| | alertname | The string FluxNotification | |
There was a problem hiding this comment.
I think the alertname should be the controller name set in event.ReportingController.
There was a problem hiding this comment.
I took inspiration from how other alertmanager alerts are configured in things like the KubePrometheus stack where the name of the alert is more akin to a description of the alert rather than the resource that triggered it
For example:
There was a problem hiding this comment.
I think instead the report controller should be a label inside the alert rather than the alertname.
There was a problem hiding this comment.
Hmm ok so maybe the alertname should be the reason? FluxNotification doesn't seem ok to me.
There was a problem hiding this comment.
Perhaps add the reason to the word flux?
See how the thanos people have named theirs for example for the ThanosCompact component/app their rules look like.
So
FluxNotificationProgressing or FluxProgressing? or Flux(Kustomization|HelmRelease)Progressing?
There was a problem hiding this comment.
Flux(Kustomization|HelmRelease)Progressing looks good to me 💯
There was a problem hiding this comment.
Will push new commits probably tomorrow now, but this is what it looks like in alertmanager as an example.
I agree this is better than just FluxNotification :) ty
Alan01252
force-pushed
the
feature/alertmanager
branch
from
ff0fa4d
to
4accc08
Compare
|
Hmm not sure why DCO is hanging now. |
Alan01252
force-pushed
the
feature/alertmanager
branch
from
4accc08
to
fcd7ebc
Compare
tried another push seems to be still be busted... |
|
The Probot DCO bot was down the whole day... it's back up now, please rebase with upstream and force push. |
This commit adds the alertmanager provider. The provider adds some generic labels based on the event which should be enough to configure appropraite routes within alertmanager. The alert is annotated with the message by default and optionally by the summary field given in the event. Signed-off-by: Alan Hollis <me@alanhollis.com>
Alan01252
force-pushed
the
feature/alertmanager
branch
from
fcd7ebc
to
8bf8150
Compare
|
Done ty |
|
@stefanprodan just checking that there's nothing more you'd like me to do here? Happy to continue until we can get this merged :) |
This commit adds the alertmanager provider. The provider adds some
generic labels based on the event which should be enough to configure
appropraite routes within alertmanager.
The alert is annotated with the message by default and optionally by the
summary field given in the event.