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

Support "code owner" alerts #4075

Open
bretg opened this issue Nov 25, 2024 · 7 comments
Open

Support "code owner" alerts #4075

bretg opened this issue Nov 25, 2024 · 7 comments

Comments

@bretg
Copy link
Contributor

bretg commented Nov 25, 2024

We had a request in the Prebid Slack channel to alert the maintainer when a PR has been opened that affects a bid adapter. This is a pretty reasonable request and not the first time it's come up.

The easiest way to do this would be to turn on the github "code owner" feature so updates to bid adapters and modules at least alert the team that originally wrote it. I played around with the 'code owner' feature and I think it's ok with 3 wrinkles:

  • It requires that the 'code owner' be granted write permission to the repo for specific github handles or teams. I'm not willing to do this except for Prebid.org members.
  • I dislike relying on specific github handles because engineers move around and because it's hard to track why a certain person deserves access to our repo. But we can manage this partly by mandating the use of github teams.
  • Github then requires a review from one of the 'code owners' before that PR can be merged. This could significantly slow merging a PR. But I suppose that's what a team wants if they opt into this feature.

So here's a possible workflow:

  1. Prebid.org members can request code ownership over an adapter or module by emailing prebid-server at prebid dot org. The request must contain the github handles of the people on the review team.
  2. We will create (or update) a github team containing the requested handles.
  3. The CODEOWNERS file will be updated to point to this team as owning the relevant directories.
  4. If a code owner does not respond within 2 weeks, email will be sent to the maintainer email address.
  5. If a company lets its Prebid.org membership lapse, the CODEOWNERS entries will be removed.

An alternate solution (maybe even the preferred solution) would be something similar to CODEOWNERS but that just alerted the maintainer email address with an FYI. An action like the one below could work, but we would need a file for everyone who wanted to do this and github doesn't allow directory structure under workflows. So that directory could get messy with that approach.

name: Notify on BidderA Directory Change

on:
  push:
    paths:
      - 'path/to/your/directory/**' # Specify the directory to monitor

jobs:
  notify:
    runs-on: ubuntu-latest
    steps:
      - name: Send Email Notification
        uses: dawidd6/action-send-mail@v3
        with:
          server_address: smtp.your-email-server.com
          server_port: 587
          username: ${{ secrets.EMAIL_USERNAME }}
          password: ${{ secrets.EMAIL_PASSWORD }}
          subject: Changes Detected in Repository
          body: |
            Changes were made to files in the specified directory:
            ${{ github.event.head_commit.message }}
          to: "[email protected]"

I haven't found any options out there for a CODEOWNERS-like config file.

@oronno
Copy link
Contributor

oronno commented Nov 28, 2024

Do we have a mail server that can be used to send email? If so, I can suggest an easy way to avoid modifying the GitHub workflow structure or writing separate workflows for every notification requirement. Instead, we can achieve this using a combination of simple scripting and a GitHub Action.

Proposed Workflow

Assumptions
A mail server is available and accessible with the SMTP configuration and the sendemail utility is installed on the system. It can be easily installed on Ubuntu/Debian systems using: sudo apt-get install sendemail

Implementation Details

We will introduce a codepath-notification file in the repository. This file contains mappings of file/directory patterns to email addresses, like this:

path/to/adapterA/** [email protected]
path/to/adapterB/** [email protected]

We create a simple shell script (send_notification_on_change.sh) to parse the codepath-notification file and send email notifications using the mail server.

#!/usr/bin/env bash

# Path to the codepath-notification file
NOTIFICATION_FILE="codepath-notification"

# SMTP Configuration
SMTP_SERVER="smtp.your-email-server.com"
SMTP_PORT="587"
USERNAME="${{ secrets.EMAIL_USERNAME }}"
PASSWORD="${{ secrets.EMAIL_PASSWORD }}"
FROM_EMAIL="[email protected]"

# Git command to get the list of changed files in the merge request
CHANGED_FILES=$(git diff --name-only origin/main...HEAD)

# Email sending function using sendemail
send_email() {
  local recipient=$1
  local file=$2
  echo "Sending notification to $recipient for changes in $file"
  
  SUBJECT="Code Path Change Notification"
  BODY="The following file was modified: $file"

  # Send email using sendemail
  sendemail -f "$FROM_EMAIL" \
            -t "$recipient" \
            -u "$SUBJECT" \
            -m "$BODY" \
            -s "$SMTP_SERVER:$SMTP_PORT" \
            -xu "$USERNAME" \
            -xp "$PASSWORD" \
            -o tls=yes
}


while IFS= read -r line; do
  path_pattern=$(echo "$line" | awk '{print $1}')
  email=$(echo "$line" | awk '{print $2}')

  for file in $CHANGED_FILES; do
    if [[ $file == $path_pattern ]]; then
      send_email "$email" "$file"
    fi
  done
done < "$NOTIFICATION_FILE"

Triggering the Script with GitHub Actions
We use a GitHub Action to run the send_notification_on_change.sh script. The action will be triggered whenever a pull request is created or updated.

name: Notify Code Path Changes

on:
  pull_request:
    paths:
      - '**'

jobs:
  notify:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout Code
        uses: actions/checkout@v3

      - name: Run Notification Script
        run: |
          chmod +x send_notification_on_change.sh
          ./send_notification_on_change.sh

@bretg
Copy link
Contributor Author

bretg commented Dec 2, 2024

Really good stuff @oronno . Thanks.

We can use SMTP_SERVER=smtp.gmail.com

I think we can use [email protected] as the sending account. I have the login info. Will put it on my list to test these scripts over the next week or two.

@bretg bretg moved this from Triage to In Progress in Prebid Server Prioritization Dec 6, 2024
@bretg
Copy link
Contributor Author

bretg commented Dec 11, 2024

@oronno - I've been playing around with this proposal, but ran into a few things.

  1. There are quite a few separate directories that hold bidder-related files: code, schema, yaml, test. We should avoid sending multiple emails for PRs that cover multiple paths.
  2. More importantly, we should handle the scenario where a large PR changes files across many paths belonging to multiple owners. This is pretty common when the core team fixes something across a release.

So I think the script needs to loop through all the changes first, placing them into email buckets, building up the message body for each party affected by the PR. Then at the end, loop through those messages and send.

As an aside, did you actually get the above code to work? I tried it verbatim and it didn't work for me. The secrets had to be in the workflow yaml. Still haven't gotten the git diff to work properly:

fatal: ambiguous argument 'origin/main...HEAD': unknown revision or path not in the working tree.

All the examples out there seem to indicate the git diff should be in the workflow, not the script.

@oronno
Copy link
Contributor

oronno commented Dec 12, 2024

Hey @bretg Sorry, there was a mismatch - main branch vs master branch.
So, instead of
CHANGED_FILES=$(git diff --name-only origin/main...HEAD)
it should be:
CHANGED_FILES=$(git diff --name-only origin/master...HEAD)

If you change to this, it should work correctly.

I did a test run. Check the branch check-notification-for-changes-certain-file in my fork repo: https://github.com/oronno/prebid-server/tree/check-notification-for-changes-certain-file
where both script file send_notification_on_change.sh and codepath-notification file implemented.

Run the script as following:
"""
chmod +x send_notification_on_change.sh
./send_notification_on_change.sh
"""

Instead of sending email, I just printed the email body. It suppose to work in the same way.

@oronno
Copy link
Contributor

oronno commented Dec 12, 2024

And regarding your concept of using "email bucket" to send single email for multiple code path changes as defined in multiple entry in codepath-notification file, I have successfully modified the script to achieve this!
I uploaded this on another branch: https://github.com/oronno/prebid-server/tree/notify-on-certain-file-change-improved

The send_notification_on_change.sh script should now handle scenarios where there are multiple entries in the codepath-notification file correctly.

To test it, I made a commit that touches multiple files. As expected, the script sends only one consolidated email for the changes.

@bretg
Copy link
Contributor Author

bretg commented Dec 17, 2024

I struggled with this for a while and got it all working in Node except for the email part. That's totally not working and my laptop refuses to build the necessary npm packages. Will try it again someday.

@bretg
Copy link
Contributor Author

bretg commented Dec 19, 2024

Got it working. PR for PBS-Java is prebid/prebid-server-java#3645

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

No branches or pull requests

2 participants