From 7144397cdb3e9323bd75cfc54ac3ecc222b331ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Nogueira?= Date: Tue, 30 Jul 2024 19:51:25 +0100 Subject: [PATCH] add per fail build user mention --- lib/action.ml | 61 +++++++++++++++++++++++++++++++++++---------------- lib/slack.ml | 47 +++++++++++++++++++++++++++++++++++---- 2 files changed, 85 insertions(+), 23 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index bd371bc9..5879cf90 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -128,6 +128,13 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct in if matched_channel_names = [] then default else matched_channel_names + let get_slack_user_for_notification ~ctx ~cfg email = + match%lwt Slack_api.lookup_user ~ctx ~cfg ~email () with + | Ok res -> Lwt.return_some res.user + | Error e -> + log#warn "couldn't match commit email %s to slack profile: %s" email e; + Lwt.return_none + let partition_status (ctx : Context.t) (n : status_notification) = let repo = n.repository in let cfg = Context.find_repo_config_exn ctx repo.url in @@ -136,17 +143,14 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct let rules = cfg.status_rules.rules in let action_on_match (branches : branch list) ~notify_channels ~notify_dm = let%lwt direct_message = - if notify_dm then begin - match%lwt Slack_api.lookup_user ~ctx ~cfg ~email:n.commit.commit.author.email () with - | Ok res -> + match notify_dm with + | false -> Lwt.return [] + | true -> + (match%lwt get_slack_user_for_notification ~ctx ~cfg n.commit.commit.author.email with + | Some user -> State.set_repo_pipeline_commit ctx.state repo.url ~pipeline ~commit:n.sha; - (* To send a DM, channel parameter is set to the user id of the recipient *) - Lwt.return [ res.user.id ] - | Error e -> - log#warn "couldn't match commit email %s to slack profile: %s" n.commit.commit.author.email e; - Lwt.return [] - end - else Lwt.return [] + Lwt.return [ user.id ] + | None -> Lwt.return []) in let%lwt chans = let default = Stdlib.Option.to_list cfg.prefix_rules.default_channel in @@ -198,25 +202,21 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct let branches = match StringMap.find_opt pipeline repo_state.pipeline_statuses with | Some branch_statuses -> - log#info "-------------------------iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii-check if failure has been fixed for pipeline %s" pipeline; let was_failing_before (branch : branch) = match StringMap.find_opt branch.name branch_statuses with (* because when the job is running the state is `pending`, we need to check if we are transitioning from a pending state that has an original_failed_commit (which means it was in failed state before) *) - | Some { status; original_failed_commit; _ } when status = Pending && Option.is_some original_failed_commit -> true + | Some { status; original_failed_commit; _ } + when status = Pending && Option.is_some original_failed_commit -> + true | _ -> false in let branches = - List.filter - (fun branch -> (was_failing_before branch) && current_status = Success ) - n.branches + List.filter (fun branch -> was_failing_before branch && current_status = Success) n.branches in branches | None -> [] in - let _ = List.iter (fun (branch : branch) -> - log#info "%s" branch.name; - ) branches in action_on_match branches ~notify_channels ~notify_dm end else Lwt.return [] @@ -290,7 +290,30 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct Lwt.return notifs | Status n -> let%lwt channels = partition_status ctx n in - let notifs = List.map (generate_status_notification cfg n) channels in + let repo_state = State.find_or_add_repo ctx.state n.repository.url in + let pipeline = n.context in + (* will return one or two users when build is failing and none when the build is green *) + let%lwt slack_users_who_broke_the_build = + match StringMap.find_opt pipeline repo_state.pipeline_statuses with + | Some branch_statuses -> + let to_users_who_broke_the_builds (branch : branch) = + match StringMap.find_opt branch.name branch_statuses with + | Some { original_failed_commit; current_failed_commit; _ } -> + Lwt_list.filter_map_p + (fun (maybe_commit : State_t.ci_commit option) -> + match maybe_commit with + | None -> Lwt.return_none + | Some commit -> + let%lwt maybe_user = get_slack_user_for_notification ~ctx ~cfg commit.author in + Lwt.return @@ Option.map (fun user -> commit.sha, user) maybe_user) + [ original_failed_commit; current_failed_commit ] + | None -> Lwt.return [] + in + let%lwt slack_user_per_commit = Lwt_list.map_p to_users_who_broke_the_builds n.branches in + Lwt.return @@ List.flatten slack_user_per_commit + | None -> Lwt.return [] + in + let notifs = List.map (generate_status_notification slack_users_who_broke_the_build ctx cfg n) channels in Lwt.return notifs let send_notifications (ctx : Context.t) notifications = diff --git a/lib/slack.ml b/lib/slack.ml index 983279c0..0524a9fe 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -240,7 +240,8 @@ let generate_push_notification notification channel = let buildkite_description_re = Re2.create_exn {|^Build #(\d+)(.*)|} -let generate_status_notification (cfg : Config_t.config) (notification : status_notification) channel = +let generate_status_notification (slack_users : (string * Slack_t.user) list) (_ctx : Context.t) (cfg : Config_t.config) + (notification : status_notification) channel = let { commit; state; description; target_url; context; repository; _ } = notification in let ({ commit : inner_commit; sha; author; html_url; _ } : status_commit) = commit in let ({ message; _ } : inner_commit) = commit in @@ -258,9 +259,11 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ match target_url with | None -> s | Some _ when not is_buildkite -> s - (* TODO when the pipeline matches the pipelines in pipelines_that_allow_all_steps, we know this fixes the final build. We can use this info to generate different notifications when the bujilds are fixed *) + (* TODO when the pipeline matches the pipelines in pipelines_that_allow_all_steps, + TODO we know this fixes the final build. We can use this info to generate different notifications + TODO when the bujilds are fixed *) | Some target_url -> - (* Specific to buildkite *) + (* Specific to buildkite, top pipeline job *) match Re2.find_submatches_exn buildkite_description_re s with (* We use a zero-width space \u{200B} to prevent slack from interpreting #XXXXXX as a color *) | [| Some _; Some build_nr; Some rest |] -> sprintf "Build <%s|#\u{200B}%s>%s" target_url build_nr rest @@ -323,7 +326,43 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ | None -> default_summary | Some pipeline_url -> sprintf "<%s|[%s]>: Build %s for \"%s\"" pipeline_url context state_info commit_message) in - let msg = String.concat "\n" @@ List.concat [ commit_info; branches_info ] in + let cc = + match String.split_on_char '/' notification.context with + | [ "buildkite"; _pipeline ] -> + (match slack_users with + | [ (_commit_hash, user) ] -> + let no_link_cc = sprintf "*CC*: <@%s>. Failing build: %s" user.id notification.context in + Option.map_default + (fun link -> [ sprintf "*CC*: <@%s>. Failing build: <%s|%s>" user.id notification.context link ]) + [ no_link_cc ] notification.target_url + | [ (commit_hash_one, user_one); (_, user_two) ] -> + (* we ignore the warning to pattern match without covering all the cases *) + let (_current_sha :: t) = html_url |> String.split_on_char '/' |> List.rev [@@ocaml.warning "-8"] in + let commit_hash_one_html_url = commit_hash_one :: t |> List.rev |> String.concat "/" in + [ + sprintf "*CC*: <@%s>." user_two.id; + sprintf "*Original failing commit*: `<%s|%s>`. CC: <@%s>" commit_hash_one_html_url commit_hash_one user_one.id; + ] + | _ -> []) + | [ "buildkite"; _pipeline; _step ] -> + (match slack_users with + | [ (_commit_hash, user) ] -> + let no_link_cc = sprintf "*CC*: <@%s>. Failing step: %s" user.id notification.context in + Option.map_default + (fun link -> [ sprintf "*CC*: <@%s>. Failing step: <%s|%s>" user.id notification.context link ]) + [ no_link_cc ] notification.target_url + | [ (commit_hash_one, user_one); (_, user_two) ] -> + (* we ignore the warning to pattern match without covering all the cases *) + let (_current_sha :: t) = html_url |> String.split_on_char '/' |> List.rev [@@ocaml.warning "-8"] in + let commit_hash_one_html_url = commit_hash_one :: t |> List.rev |> String.concat "/" in + [ + sprintf "*CC*: <@%s>." user_two.id; + sprintf "*Original failing commit*: `<%s|%s>`. CC: <@%s>" commit_hash_one_html_url commit_hash_one user_one.id; + ] + | _ -> []) + | _ -> [] + in + let msg = String.concat "\n" @@ List.concat [ cc; commit_info; branches_info ] in let attachment = { empty_attachments with