Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify Slack when the main branch build fails #1390

Merged
merged 3 commits into from Feb 4, 2022

Conversation

dhwthompson
Copy link
Contributor

Example build failure notification

This is largely based on similar work in other Weaveworks projects, but with a bit of extra information that will hopefully be useful to folks who want to investigate the failure.

There are a few bits of this workflow that I don't love: the jq hackery to extract the first line of the commit message isn't particularly pleasant, but I couldn't see anything exposed by GitHub that gives us just the commit's subject line. Telling jq to JSON-encode the commit subject also feels like a bit of a hack, but these are the perils of telling GitHub how to write YAML that writes JSON that writes Slack's custom mrkdwn syntax.

If we want to add any more context to these messages (for instance, which bits of the run failed and how long they took), I would strongly recommend taking the functionality here and re-implementing it in a scripting language of choice (Python seems like a good fit, but anything else that has JSON support in its standard library and comes bundled with Ubuntu would probably work just as well).

I've tentatively left the job open to the prospect of adding other build failure notifications (Slack or otherwise). Normally, I'd think of this as a premature generalization, but it plays nicely into GitHub's way of evaluating whether to run a job at all.

Reviewer notes

This is one instance where a reviewer is going to get a better impression by looking at the individual commits:

  • The first commit is where the actual work is. Look at this one.
  • The second commit changes a few settings to allow for testing without messing up the main branch or spamming the team.
  • The third commit artificially breaks a bunch of steps in the build, so that I can get feedback in seconds, rather than minutes.

I’m also open to feedback on the information we’re presenting in the notifications. Is there something missing that would be really useful? Is there something there that isn’t helpful? We’re a little limited in what GitHub makes available without checking out the source code or running more advanced scripting, but suggestions are always welcome.

This is largely based on similar work in other Weaveworks projects[1],
but with a bit of extra information that will hopefully be useful to
folks who want to investigate the failure.

There are a few bits of this workflow that I don't love: the `jq`
hackery to extract the first line of the commit message isn't
particularly pleasant, but I couldn't see anything exposed by GitHub
that gives us just the commit's subject line. Telling `jq` to
JSON-encode the commit subject also feels like a bit of a hack, but
these are the perils of telling GitHub how to write YAML that writes
JSON that writes Slack's custom `mrkdwn` syntax.

If we want to add any more context to these messages (for instance,
which bits of the run failed and how long they took), I would strongly
recommend taking the functionality here and re-implementing it in a
scripting language of choice (Python seems like a good fit, but anything
else that has JSON support in its standard library and comes bundled
with Ubuntu would probably work just as well).

I've tentatively left the job open to the prospect of adding other build
failure notifications (Slack or otherwise). Normally, I'd think of this
as a premature generalization, but it plays nicely into GitHub's way of
evaluating whether to run a job at all.

[1]: https://github.com/search?q=org%3Aweaveworks+action-slack-notifier&type=code
Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

Looks great!

- acceptance-tests-2
- acceptance-tests-3
- ui-tests
if: ${{ failure() }}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Didn't know we could do this. Branching in yaml, what a time to be alive.

Copy link
Contributor

@rokshana-b rokshana-b left a comment

Choose a reason for hiding this comment

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

Nice!! 🥇
I'm assuming all the tests will pass eventually 😄

Copy link
Contributor

@J-Thompson12 J-Thompson12 left a comment

Choose a reason for hiding this comment

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

LGTM as well.

`github.sha` is available for more events than just a `push` event.
Granted, we only want to send these notifications on `push` events, but
it's better to have the job successfully do nothing in other cases,
rather than fail.
For other event types (like `pull_request`), the head commit information
isn't available in quite the same way, so rather than have the
intermediate commit summary step fail, we can set a condition on its
execution so that it doesn't even try.

The `event_name` condition on the final step is probably redundant under
the current setup (unless someone tries to set up a pull request *from*
the `main` branch), but it's probably worth having as an extra guard in
case we change the configuration for when this workflow runs.
@dhwthompson dhwthompson marked this pull request as ready for review February 3, 2022 23:15
Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

New commits LGTM

@dhwthompson dhwthompson merged commit 2e6444c into main Feb 4, 2022
@dhwthompson dhwthompson deleted the team-build-notifications branch February 4, 2022 18:14
jpellizzari pushed a commit that referenced this pull request Mar 3, 2022
This is largely based on similar work in other Weaveworks projects[1],
but with a bit of extra information that will hopefully be useful to
folks who want to investigate the failure.

There are a few bits of this workflow that I don't love: the `jq`
hackery to extract the first line of the commit message isn't
particularly pleasant, but I couldn't see anything exposed by GitHub
that gives us just the commit's subject line. Telling `jq` to
JSON-encode the commit subject also feels like a bit of a hack, but
these are the perils of telling GitHub how to write YAML that writes
JSON that writes Slack's custom `mrkdwn` syntax.

If we want to add any more context to these messages (for instance,
which bits of the run failed and how long they took), I would strongly
recommend taking the functionality here and re-implementing it in a
scripting language of choice (Python seems like a good fit, but anything
else that has JSON support in its standard library and comes bundled
with Ubuntu would probably work just as well).

I've tentatively left the job open to the prospect of adding other build
failure notifications (Slack or otherwise). Normally, I'd think of this
as a premature generalization, but it plays nicely into GitHub's way of
evaluating whether to run a job at all.

[1]: https://github.com/search?q=org%3Aweaveworks+action-slack-notifier&type=code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants