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

chore: slack-lite notifications for issue/PR open/close actions #496

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

trentm
Copy link
Member

@trentm trentm commented Dec 19, 2024

the issue

The typical way to get notifications in Slack for various actions in a GitHub repo is via https://github.com/integrations/slack, the GitHub-maintained integration for Slack.

This integration has the following issues with its notifications for GitHub issue and PR actions:

  • It is over-wordy, resulting in notification noise masking any other conversation in the channel. This lowers the signal-to-noise ratio in the channel, which discourages meaningful conversation.
    • It includes some body content from issues/PRs. This may be a nice idea for a repo with commonly simple and short issue/PR descriptions. However, in practice (IMHO) it is rarely enough to avoid the need to visit the full issue in the browser to meaningfully understand and interact with the issue/PR. It can also get silly for PRs that have large <details> blocks, which the Slack rendering doesn't hide. E.g. deps: Bump updatecli version to v0.89.0 opbeans-node#304
    • It includes some issue metadata, sometimes some chat-ops buttons ("Comment", "Close").
    • It includes a variable-height summary of actions/checks on PRs. This sounds nice, but it results in dynamic-height content in the chat area that is distracting for ongoing conversation.
  • In the wordiness the (small, gray) text with the repo name is hard to find. We have these notifications for 4+ repos, so it is pain to have to hunt for that.
  • It @-names people listed as reviewers for PRs and makes a Slack thread for every PR to show when it is merged. This sounds like a nice idea, but in practice means that the "Threads" section of Slack is spammed with useless unread threads for merged PRs that I've reviewed.
  • Basically the Slack integration is trying to be a fancy chat-ops thingy, and I don't want the noise. I want higher conversational signal.
  • It doesn't support ignoring some PR categories: specifically ignoring dependabot/renovate PRs. Currently we have those scheduled for Sunday. Every Monday morning is a multi-page scroll of useless notifications about regular dependabot maintenance work. It is very easy to lose meaningful conversation from Friday or over the weekend in that noise.

If the integration supported ignoring PRs from dependabot, renovate, and possibly other automation, that likely would have sufficed. There are a dozen or so issues requesting something like this (https://github.com/integrations/slack/issues?q=is%3Aissue+dependabot), the main one being this one, and even two PRs implementing it (integrations/slack#974, integrations/slack#1252). However, since that first one, GitHub removed the source code from the repo to use a closed source implementation and does not accept PRs.

first attempt: Slack App watching GitHub repo events API

Initially I started implementing a Slack app, using Slack's Bolt framework for building Slack apps. This app would replace the issue and PR functionality of GitHub's Slack integration. It would listen to GitHub repository events for each subscribed repo and send appropriate messages to the Slack channel.

The main reasons I gave up on this approach are:

  1. The GitHub repository events API says:

    This API is not built to serve real-time use cases. Depending on the time of day, event latency can be anywhere from 30s to 6h.

    While this Slack notifications app need not be real-time, somewhat timely would be nice. Having 30s-6h delays on notifications isn't appealing. I suspect in practice the latency is typically fine. (I have a total guess that using an internal-to-GitHub message queue, rather than the public GitHub REST API, is a reason GitHub moved the integration implementation to be closed source.)

  2. The complexity of running a persistent service for these notifications was not appealing to me. I'd have to get into deploying this (with secrets) somewhere. A goal of this hack was to simplify. I didn't want to add oncall/maintenance work for myself.

second attempt: GH Action in each repo

integrations/slack#951 (comment) proposed a comparatively simple workaround: add a GitHub Action to the repo you want to watch that sends a notification to Slack.

Basically I riffed on that idea, playing with the notification style/layout and using shell: python to better cope with encoding issues.

Screenshot 2025-01-07 at 4 13 15 PM

Screenshot 2025-01-07 at 5 01 42 PM

proposal

  • Discuss this. (David and I discussed today.)
  • Try this out in elastic/elastic-otel-node and elastic/apm-agent-nodejs.
  • Then if we prefer it, stop the issue and PR notifications from the existing GitHub+Slack integration, via /github subscribe owner/repo releases deployments.
    • We can keep its notifications for some other things like unfurling links to GH issues/comments/code-ranges).
    • The command to do this is: /github unsubscribe owner/repo issues pulls commits. This keeps the default releases and deployments features. We require all commits to shipping branches to go through a PR, so showing commits is a somewhat redundant.
  • And then we do this for the other repos we care about.

I am proposing this as an alternative to the issue and PR Slack
notifications we currently get from https://github.com/integrations/slack
because of the following issues with that integration:
- It is over-wordy, resulting in notification noise masking any
  other conversation in the channel. This lowers the signal-to-noise
  ratio in the channel, which discourages meaningful conversation.
    - It includes some body content from issues/PRs. This may be a nice
      idea for a repo with commonly simple and short issue/PR
      descriptions. However, in general it is just spam. It can also get
      silly for PRs that have large `<details>` blocks, which the Slack
      rendering doesn't hide. E.g. elastic/opbeans-node#304
    - It includes some issue metadata, sometimes some chat-ops buttons
      ("Comment", "Close").
    - It includes a variable-height summary of actions/checks on PRs.
      This sounds nice, but it results in dynamic-height content in the
      chat area that discourages ongoing conversation.
- It `@-`names people listed as reviewers for PRs and makes a *threads*
  of every PR to show when it is merged. This sounds like a nice idea,
  but in practice means that the "Threads" section of Slack is spammed
  with useless unread threads for merged PRs that I've reviewed.
- Basically the Slack integration is trying to be a fancy full-on
  chat-ops thingy, and I don't want the noise. I want higher signal.
- It doesn't support ignoring some PR categories: specifically ignoring
  dependabot/renovate PRs. Currently we have those scheduled for Sunday.
  Every Monday morning is a multi-page scroll of useless notifications
  about regular dependabot maintenance work. It is very easy to lose
  *meaningful* conversation from Friday or over the weekend in that
  noise.
@trentm trentm requested a review from david-luna December 19, 2024 00:45
@trentm trentm self-assigned this Dec 19, 2024
@trentm
Copy link
Member Author

trentm commented Dec 19, 2024

  • It can also get
    silly for PRs that have large <details> blocks, which the Slack
    rendering doesn't hide. E.g.

Screenshot 2024-12-18 at 4 32 45 PM

@trentm trentm merged commit bcc2379 into main Jan 8, 2025
2 checks passed
@trentm trentm deleted the trentm/slack-lite branch January 8, 2025 01:07
trentm added a commit to elastic/apm-agent-nodejs that referenced this pull request Jan 8, 2025
An alternative to https://github.com/integrations/slack for
Slack notifications for issue/PR actions.

Refs: elastic/elastic-otel-node#496
@trentm trentm changed the title chore: Slack notifications for issue/PR open/close actions chore: slack-lite notifications for issue/PR open/close actions Jan 8, 2025
@trentm
Copy link
Member Author

trentm commented Jan 8, 2025

Notes to self:

Could add a GH icon in the context (a la notifications from the GitHub Slack integration) via:

				{
					"type": "context",
					"elements": [
						{
							"type": "image",
							"image_url": "https://github.githubassets.com/favicon.ico",
							"alt_text": "images"
						},
						{
							"type": "mrkdwn",
							"text": "trentm/play#10 · *PR opened* by trentm"
						}
					]
				},

However the sizing is a bit off there.

Could also use :github: slack emoji in the text, but that depends on that Emoji having been separately added to the particular slack.

@trentm
Copy link
Member Author

trentm commented Jan 8, 2025

I tweaked the styling a bit (will ahve in an upcoming PR):

Screenshot 2025-01-07 at 8 20 03 PM

trentm added a commit to elastic/apm-agent-nodejs that referenced this pull request Jan 8, 2025
An alternative to https://github.com/integrations/slack for
Slack notifications for issue/PR actions.

Refs: elastic/elastic-otel-node#496

This switches to the older form of styling Slack 'attachments'
eschewing the newer 'blocks', which don't style quite as nicely
IMHO.
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