Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions .github/workflows/check-artifacts-ac.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
name: 🔄 Check artifacts to match AC expectations

on:
push:
branches: [main]
pull_request:
branches: [main]

jobs:
check-artifacts-ac:
name: Check artifacts to match AC expectations
runs-on: ubuntu-latest
outputs:
checks: ${{ steps.aggregated_msg.outputs.failure_message }}
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 2

- name: Ensure config.yaml file exists
id: check_file_existence
run: |
if [ ! -f distributions/nrdot-collector-host/config.yaml ]; then
echo "file_exists=false" >> $GITHUB_ENV
echo "file_message=The file distributions/nrdot-collector-host/config.yaml does not exist." >> $GITHUB_ENV
exit 0
fi
echo "file_exists=true" >> $GITHUB_ENV

- name: Check for config.yaml modification
id: check_config
run: |
if git diff --name-only HEAD~1 HEAD | grep -q 'distributions/nrdot-collector-host/config.yaml'; then
echo "config_modified=true" >> $GITHUB_ENV
echo "config_message=The config.yaml file has been modified." >> $GITHUB_ENV
exit 0
fi
echo "config_modified=false" >> $GITHUB_ENV

- name: Check binary name in goreleaser file
id: check_binary
run: |
binary_name=$(grep 'binary:' distributions/nrdot-collector-host/.goreleaser.yaml | awk '{print $2}')
if [ "$binary_name" != "nrdot-collector-host" ]; then
echo "binary_correct=false" >> $GITHUB_ENV
echo "binary_message=The binary name is not nrdot-collector-host." >> $GITHUB_ENV
exit 0
fi
echo "binary_correct=true" >> $GITHUB_ENV

- name: Check package name in goreleaser file
id: check_package
run: |
package_name=$(grep 'package_name:' distributions/nrdot-collector-host/.goreleaser.yaml | awk '{print $2}')
if [ "$package_name" != "nrdot-collector-host" ]; then
echo "package_correct=false" >> $GITHUB_ENV
echo "package_message=The package name is not nrdot-collector-host." >> $GITHUB_ENV
exit 0
fi
echo "package_correct=true" >> $GITHUB_ENV

- name: Aggregate failure messages
id: aggregated_msg
if: env.config_modified == 'true' || env.binary_correct == 'false' || env.package_correct == 'false' || env.file_exists == 'false'
run: |
failure_message=""
if [ "$file_exists" == "false" ]; then
failure_message="$file_message"
fi
if [ "$config_modified" == "true" ]; then
failure_message="$failure_message\n$config_message"
fi
if [ "$binary_correct" == "false" ]; then
failure_message="$failure_message\n$binary_message"
fi
if [ "$package_correct" == "false" ]; then
failure_message="$failure_message\n$package_message"
fi
echo "failure_message=$failure_message" >> $GITHUB_OUTPUT

notify-conflicts:
Copy link
Contributor

Choose a reason for hiding this comment

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

why not making the workflow fail instead of notifying ?
Is it desirable that we brake this contract?

Copy link
Contributor Author

@alvarocabanas alvarocabanas Apr 10, 2025

Choose a reason for hiding this comment

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

@kb-newrelic is that something you could contemplate?
Of course then we shouldn't fail on config.yaml modification and only notify of a change so we can check it, but for the other cases, could we fail the pipeline?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree failing a PR for violating the basic requirements (path, binary/package name) is reasonable. As we already have a sync/check for the k8s distro, I created a PR to follow the same pattern but it was super helpful to have yours as a template, so I appreciate the work you put in!

As you have already pointed out, we can't have the same strictness for the config as it would create a pretty limiting dependency.

I was hoping we could codify the dependency on the config a bit more clearly to avoid having to start the analysis from scratch anytime this config changes. Like if we were to change the memorylimiterprocessor defaults, I assume it would be unnecessary to alert either one of our teams but I need to understand the exact dependency better to codify it properly.
Things that come to mind would be:

  • any change in env vars names (removal, newly added, renames)
  • host entity synthesis (out of scope for this - it's in our backlog to validate that in our E2E tests though)
  • ...?

If your team wants to be notified of any change, we can for sure do that but for our team it would be a non-actionable alert, so I would like to avoid that.

Copy link
Contributor

@kb-newrelic kb-newrelic Apr 14, 2025

Choose a reason for hiding this comment

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

We can decide in this thread what to do and I will replace this TODO with whatever we end up with.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added env var assertions: 4e94d7f

needs: check-artifacts-ac
if: needs.check-artifacts-ac.outputs.checks != ''
runs-on: ubuntu-latest
steps:
- name: Notify AC artifact conflicts via Otel Slack
uses: slackapi/slack-github-action@v2.0.0
with:
webhook: ${{ secrets.OTELCOMM_BOTS_SLACK_HOOK }}
webhook-type: incoming-webhook
payload: |
{
"text": ":warning: [Nightly workflow to check nrdot artifacts integrity found conflicts] @hero check ${{needs.check-artifacts-ac.outputs.checks }}"
}

- name: Notify AC artifact conflicts via AC Slack
uses: slackapi/slack-github-action@v2.0.0
with:
webhook: ${{ secrets.AC_SLACK_WEBHOOK }}
webhook-type: incoming-webhook
payload: |
{
"text": ":warning: [Nightly workflow to check nrdot artifacts integrity found conflicts] @hero check ${{needs.check-artifacts-ac.outputs.checks }}"
}
Loading