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

ci(ftr): notify owners in the slack message when failures #205260

Merged
merged 7 commits into from
Jan 7, 2025

Conversation

v1v
Copy link
Member

@v1v v1v commented Dec 30, 2024

Summary

Customise Slack messages with the Slack Team owners when the FTRs fail.

As long as the PING_SLACK_TEAM is set in the Buildkite pipeline, then the slack notification will poke the Slack team set in the environment variable when the step failed only.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
  • Flaky Test Runner was used on any tests changed
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

@v1v v1v requested a review from a team December 30, 2024 13:23
@v1v v1v self-assigned this Dec 30, 2024
@v1v v1v requested a review from a team as a code owner December 30, 2024 13:23
@v1v v1v added release_note:skip Skip the PR/issue when compiling release notes backport:all-open Backport to all branches that could still receive a release labels Dec 30, 2024
# Report the error to the team using the subfolder matching the Slack Team
popd
mkdir "$REPORT_SLACK_TEAM"
touch "$REPORT_SLACK_TEAM"/obs-ux-infra_services-team.slack
Copy link
Member

Choose a reason for hiding this comment

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

Can you help me understand the changes here vs passing a string PING_SLACK_TEAM='obs-ux-infra_services-team?

I'm not opposed to files but it seems to add some additional complexity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing.

I wanted to make pingTeam agnostic to the team name. But read the files under target/report-slack-team, and those files match the name of the slack team to be notified if any errors.

Regarding your question about PING_SLACK_TEAM='obs-ux-infra_services-team', I'm not sure how the post_command can read that value and whether I could use something like ts-node .buildkite/scripts/lifecycle/ping_team_ftrs.ts "$PING_SLACK_TEAM" in here

Maybe I could use the buildkite metadata set command to notify the Slack team to reduce the additional complexity with files under target/report-slack-team.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I've found a better approach:

Pretty much, if the env variable PING_SLACK_TEAM exists and the step fails, then it will notify the slack team.

Apply suggestions from code review

rename
@v1v v1v force-pushed the feature/ping-errors-eams branch from eeb53eb to d30ec5f Compare December 30, 2024 15:42
@v1v
Copy link
Member Author

v1v commented Dec 30, 2024

However, I've just refactored a bit more:

So I created a specialised library for the Slack notifications, so I don't mix concerns with the TestFailures, in addition, I also changed the post-command hook to run outside of the metadata called "${BUILDKITE_JOB_ID}_is_test_execution_step" - as far as I see, it's set in other places but it's not set when running .buildkite/scripts/steps/functional/profiling_cypress.sh or .buildkite/scripts/steps/functional/apm_cypress.sh

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #41 / InfraOps App Metrics UI Node Details #Asset Type: host Osquery Tab should show a date picker

Metrics [docs]

✅ unchanged

History

cc @v1v

if [[ $BUILDKITE_COMMAND_EXIT_STATUS -ne 0 ]]; then
# If the slack team environment variable is set, ping the team in slack
if [ -n "${PING_SLACK_TEAM:-}" ]; then
ts-node .buildkite/scripts/lifecycle/ping_slack.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a consideration: the PR is now mostly just wrapping around the typescript file, but that can be done with a simple one-liner

buildkite-agent meta-data set 'slack:ping_team:body' \
 "${PING_SLACK_TEAM}, can you please take a look at the test failures?"

(also add ELASTIC_SLACK_NOTIFICATIONS_ENABLED == 'true' above)

Copy link
Member Author

Choose a reason for hiding this comment

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

@delanni, sure thing, I can simplify it. let me do it now

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! I prefer it like this, without the extra bloat for now (while this is not doing more than adding the metadata)

@v1v v1v enabled auto-merge (squash) January 7, 2025 10:27
@v1v v1v merged commit acc5e03 into elastic:main Jan 7, 2025
9 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 7.17, 8.16, 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12651030780

@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
7.17 Backport failed because of merge conflicts
8.16
8.17
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 205260

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 7, 2025
…5260) (#205716)

# Backport

This will backport the following commits from `main` to `8.17`:
- [ci(ftr): notify owners in the slack message when failures
(#205260)](#205260)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Victor
Martinez","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T11:53:04Z","message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:all-open"],"title":"ci(ftr):
notify owners in the slack message when
failures","number":205260,"url":"https://github.com/elastic/kibana/pull/205260","mergeCommit":{"message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205260","number":205260,"mergeCommit":{"message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2"}}]}]
BACKPORT-->

Co-authored-by: Victor Martinez <[email protected]>
kibanamachine added a commit that referenced this pull request Jan 7, 2025
…5260) (#205715)

# Backport

This will backport the following commits from `main` to `8.16`:
- [ci(ftr): notify owners in the slack message when failures
(#205260)](#205260)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Victor
Martinez","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T11:53:04Z","message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:all-open"],"title":"ci(ftr):
notify owners in the slack message when
failures","number":205260,"url":"https://github.com/elastic/kibana/pull/205260","mergeCommit":{"message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205260","number":205260,"mergeCommit":{"message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2"}}]}]
BACKPORT-->

Co-authored-by: Victor Martinez <[email protected]>
kibanamachine added a commit that referenced this pull request Jan 7, 2025
) (#205717)

# Backport

This will backport the following commits from `main` to `8.x`:
- [ci(ftr): notify owners in the slack message when failures
(#205260)](#205260)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Victor
Martinez","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T11:53:04Z","message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:all-open"],"title":"ci(ftr):
notify owners in the slack message when
failures","number":205260,"url":"https://github.com/elastic/kibana/pull/205260","mergeCommit":{"message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205260","number":205260,"mergeCommit":{"message":"ci(ftr):
notify owners in the slack message when failures
(#205260)","sha":"acc5e039baf1f36a46d6444ae79539c8ce05edc2"}}]}]
BACKPORT-->

Co-authored-by: Victor Martinez <[email protected]>
kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:all-open Backport to all branches that could still receive a release release_note:skip Skip the PR/issue when compiling release notes v8.16.3 v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants