Skip to content

Add monitoring and observability for AlertManager rollbacks - #4376

Draft
jall0h wants to merge 23 commits into
masterfrom
u/abubakarrj/PAASTA-18923-monitoring
Draft

Add monitoring and observability for AlertManager rollbacks#4376
jall0h wants to merge 23 commits into
masterfrom
u/abubakarrj/PAASTA-18923-monitoring

Conversation

@jall0h

@jall0h jall0h commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

PAASTA-18923

Changes:

  • Add alertmanager_rollback_triggered counter metric (dimensions: paasta_service, paasta_cluster)
  • Add alertmanager_rollback_cancelled counter metric (increments when alerts stop firing before rollback executes)
  • Pass service/deploy_group labels to the alertmanager watcher threads
  • Add dry_run field to rollback audit logs and update tests to include this

Testing:

Depends on sticht #68 (updated alert manager interface). make test passes locally with that sticht branch installed.

nemacysts and others added 22 commits July 31, 2026 11:40
There'll very likely be more than these in the near future, but just
adding a couple simple ones to get started
...not sure why i had a non-major-version placeholder...
...not sure why i had a non-major-version placeholder...
We can't just filter by service name since an instance in a
cluster/deploy group with no changes might be encountering issues, and
those are (likely) unrelated to our current deployment
…m:Yelp/paasta into PAASTA-18919/alertmanager-mfd-integration
…m:Yelp/paasta into luisp/PAASTA-18914-deploy-group-shenanigans
As mentioned in the CEP, these (pre-existing) alerts have a different
set of labels, so we can't filter in quite the same way as we do for the
"custom" alerts.

However, as (also) mentioned in the CEP, we can still construct a filter
that will match these alerts given the data we have available.

This hopefully isn't too complex (and hopefully my comments are useful),
but it is slightly funky :p - although, imo, mostly due to needing to
figure out the "real" region for a cluster + needing to handle
multi-registration instances :p
This is the PaaSTA side of Yelp/sticht#67 -
i.e,. how we'll allow folks to experiment with alertmanager rollbacks
without needing to fully commit (and/or for testing new rollback alerts)

As usual, config mostly lives in deploy.yaml per-deploy-group :)

That said, this 1000% requires the sticht PR linked above, since that's
essentially where all the logic actually lives
…er and alertmanager rollbacks that get cancelled
"deploy_group": deploy_group,
"commit": commit,
"image_version": image_version,
"dry_run": "False",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably leave this out - the audit log will have non-rollback events in it (and this is really just a normal deploy event)

Comment on lines +823 to +828
labels = {"deploy_group": self.deploy_group, "service": self.service}
self.start_alertmanager_watcher_threads(
alertmanager_url=alertmanager_url,
filters=filters,
check_interval_s=self.alertmanager_poll_interval_s,
labels=labels,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: i'd maybe do something like this since we're not really gonna reuse this dictionary later on and inline the label dict - e.g.,

self.start_alertmanager_watcher_threads(
    alertmanager_url=alertmanager_url,
    filters=filters,
    check_interval_s=self.alertmanager_poll_interval_s,
    # while we can technically grab these from the filters, we'll pass
    # these through separately so that we're not relying on a specific filter format :p
    extra_monitoring_labels={
        "deploy_group": self.deploy_group, 
        "service": self.service,
    },

Comment on lines -827 to -828

# Initialize Slack threads and send the first message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably leave this alone :p


def __build_rollback_audit_details(
self, rollback_type: RollbackTypes
self, rollback_type: RollbackTypes, dry_run: bool = False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: s/dry_run/is_dry_run so that it doesn't look like this function will mutate things if dry_run is false :p

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.

2 participants