Improve CI e-mail notifications

As far as I know, we will change how CI emails are handled quite soon.

@grzesiek can you link to an issue?

@ayufan Do we have an issue about how CI notification emails will be changed ?

Thanks @grzesiek and @JobV for following up. Any other ideas or reference? What are the plans for CI emails?

@balameb I think that @ayufan can tell more about plans.

Reassigned to @ayufan

We can use that as an issue. I'll discuss that with designers about the e-mail template.

Added feature proposal and removed ~128370 labels

mentioned in issue #4263 (closed)

mentioned in issue #13559 (closed)

It doesn't make sense to do before this: https://gitlab.com/gitlab-org/gitlab-ce/issues/14149

Assignee removed

mentioned in issue #14717 (closed)

mentioned in issue #15598 (closed)

mentioned in issue #17686 (closed)

Milestone changed to %8.9

mentioned in issue #8114 (closed)

mentioned in issue #13186 (moved)

Milestone changed to %8.10

mentioned in issue #18076 (moved)

mentioned in issue #18920 (closed)

This is really important, but doesn't look like it's going to get in to 8.10.

Milestone changed to %8.11

@markpundsack I agree. We have too much.

Maybe we could start by desiging e-mail template first? Should we get a @hazelyang and @annabeldunstone assistance on this?

If you have any suggestion, please let me know. I am not pretty sure what kind of information should be shown in this email.

Email template

@hazelyang I'm not sure if sending entire build log in build emails is a good idea as this may be very long email in some cases. Then it may happen that relevant lines about failure are in the middle of 1000 lines long build log, and everything after 500th line is irrelevant, then sending only last 50, although better, may also not be a solution good enough. And what is more important is that since we have a pipeline, we may prefer email about the status of entire pipeline instead of the status of a one particular build. What do you think?

@hazelyang But we are really on the good track, because this template looks awesome :)

@grzesiek I think it's a good idea.

@hazelyang Great! What do you think about showing pipeline vertically instead of horizontally? We may extend it easier in the future with additional information about stages that way, and this will maybe look better if someone has 10 stages instead of just 4?

Hmm, looks nice :)

@grzesiek Agree with you.

@hazelyang Great! What do you think about adding some details (we have some free space there). We can add for example - stage duration (if applicable) and number of jobs (also relevant when previous stages were successful). What do you think?

I think your suggestion is good because these information are probably important for users.

@hazelyang Will it be good to also show the list of failed builds?

@hazelyang We are almost there! What do you think about making a compound status a little different in the mockup? First row, that indicates a compound status (Status) can be mistaken with a stage. Can we make it somehow different? Maybe we should extract this outside of the table? What do you think?

@grzesiek @hazelyang Maybe we should improve the Pipeline header? And put the status there. Probably we could include there a Commit Message/Author/Short SHA, Date and Duration, Status.

@grzesiek @ayufan I did some changes. I think it looks better. How do you think?

For reference, here's some prior art:

Some key points:

Shows meta info in table (similar to Hazel's last image)
Big use of color to indicate problem with build (Circle also sends out an email when the build is fixed)
Includes project name
Shows all commit SHAs and messages, not just last commit
Shows output of failed command (not entire build log)
Leaves out duration and date (since it should have just happened and the email itself will have a timestamp)

I'm not sure we need to include the number of builds. At least as shown in Hazel's last mockup, it looks like 39 builds failed. Also, if we're going to count them, I'd really rather not see the word "builds" there. Can we call them jobs? "Jobs" is better defined in our docs than "builds".

I should include the bottom of that email:

Note that it does list the rest of the successful commands, similar to how Hazel mocked it up. In Circle's case, they break up everything into tons of tiny steps (run on the same container). Most of these are "inferred" magical steps that don't have user configuration. I always thought it was dumb to even mention them since the user was generally unaware of them.

Currently we are not able to show output of only the failed command, due to implementation details.

Right, we don't know which specific step in a job failed, but can we show the output of the entire failed job? We might need to truncate really large builds, but we could even show the last 200 lines and still be meaningful most of the time.

cc @jschatz1

@grzesiek @ayufan Any thoughts for showing the last 200 lines failed command?

If we decide to show the command, the email should show the command for every failed build, right?

@hazelyang I'm not sure about showing build log/last lines, until we will have links to particular line in build log, and ability to know which command generated output. Leaving this up to @ayufan and @markpundsack.

mentioned in issue #19589 (closed)

I think a first iteration where we show the output from the entire build log (truncated to X lines) would be fine and would still be beneficial for a lot of cases. Yes, we should then refine it by only showing the actual step that failed and be able to link to those lines in the web UI. But that shouldn't hold up making some improvement today.

On the other hand, if getting any kind of log information turns out to be really hard, then we should at least improve the rest of it.

While we're looking at emails, we should only send one email per pipeline (not per build) (#19405 (closed)).

@hazelyang Not sure if this is visible enough for which job/stage this build log belongs to. /cc @markpundsack

I wonder how other vendors handle multiple jobs failing. Perhaps we should just show the first one and a link to the web UI to see other failures?

Are the durations and job counts really important in this email? What do developers really want to see?

Can we get the full commit history of the build rather than just the last commit message?

@markpundsack What do you mean by full commit history?

Currently only information stored in Pipelines is a first and last SHA from the git push.

Commit messages for all commits between first and last SHA, rather than just last commit message.

Need help. Where can I get full commit history?

Also need some developers to answer the question below:

What do developers really want to see in the email?

(I just would like to check what kind of information are necessary in this email. )

I put the last 200 lines command ( Since it's a bit long, I crop the mockup).

@hazelyang I think that the most important thing is to know, which builds on which stage failed. Do we really need commit history? (@hazelyang take a look at repository -> commits page)

Good question @grzesiek. If the email goes to the person that broke the build, they probably know the commit history and at most need to be reminded which branch/MR this is about if they're doing multiple in the same project.

Seeing the 200 lines, where the most important stuff is at the bottom, I wonder if we should only show the last 10 or so. For some errors, this of course will fail. But good test frameworks usually put a summary at the bottom, so 10 lines might be enough.

Did some small changes:

200 lines -> 10 lines (If it works for people in most of time, we can keep it. If not, maybe we could drop it. )
Put back the failed builds on each stage. As @grzesiek mentioned , it's the most important thing developer want to know.
Added commit history.

That shows multiple jobs failing, but each job would have its own build log.

What about removing log? I just think the email is mostly to inform people which build failed on which stage and people can access detail through the links, so maybe we could remove log.

Unfortunately the build log is actually important. It can quickly tell the difference between a simple syntax error, a flaky test, or a real problem needing debugging. Done well, the developer will have no reason to click through the link to visit the website. They'll know exactly what to fix.

How about this, drop the stage view and JUST put the build logs (last 10 lines only) for all failed jobs. Also, include the job name, not just the stage name.

So something like:

Something went wrong with your tests.

| | | | --- | --- | | Project: | group/project | | Branch: | gh-pull-requests | | Commit: | Update CHANGELOG 1584912a | | Author: | Douglas Barbosa Alexandre |

Pipeline #2156852 had 3 failed jobs:

Stage 1: rspec 5 20:
[build log]

Stage 1: rspec 12 20:
[build log]

Stage 1: rspec 13 20:
[build log]

Based on @markpundsack 's suggestions.

What do you think of that @hazelyang? It looks pretty good to me!

I like the lighter treatment of the build log. The dark theme would look too strong in email, imho.

Minor thing, but all the errors would most likely be from the same stage. If there's a failure in stage 1, stage 2 never runs (except for some exceptions).

Note that if the build was a tag, the word Branch should be changed to Tag since we won't know the branch. Now that we have tag and branch icons, do you want to include them? Related, should we include the gravatar of the author?

To make it slightly more realistic, you could rename Stage 1 to "Test". That's the most common name.

@markpundsack I think it's good.

You ended up with two branches in there. :)

@markpundsack My fault. Thanks for reminder.

@hazelyang So do you want to drop the branch icon from that row? Or was that oversight?

It was my oversight...

@ayufan @annabeldunstone Are there any backend changes needed for this, or can it be handled by frontend?

@markpundsack There is some backend implementation involved (not so difficult I think), so team work may be a good idea.

@markpundsack

We need to do this first: https://gitlab.com/gitlab-org/gitlab-ce/issues/20115

mentioned in issue #19982 (closed)

Added ~122770 label

Changed title: Attach more information to CI emails → Improve CI e-mail notifications

Build log may contain some secret data, like passwords. If log is to be sent via unencrypted email, secret data may leak. And if someone forwards such e-mail, for example to ask someone outside a team, it may be even worse.

@jk Good point, but it's good practice to filter your log output at the generator rather than relying on the viewer. e.g. Rails can mask passwords and other sensitive variables before they even hit the logs.

Also, we already mask secret variables we detect.

If need be, we could consider letting people opt out of including build logs, but I'd really rather avoid an unnecessary option.

@markpundsack I guess a warning on top of the log would be a good compromise. Just to remind people, that there are sensitive data and you should not forward this mail without thinking.

@hazelyang What do you think about putting a warning in there?

@markpundsack @jk How about put a warning in the log box like this?

Visually, that works. For text, how about "Logs may contain sensitive data. Please consider before forwarding this email."

Yes, that is what I had on my mind.

@markpundsack haha, thanks!

for this issue and most of the mockup!
But do we really have to use this Microsoft-ish "Something went wrong" phrase? I already know that "something went wrong" when I see that email hitting my inbox. No reason to have it in a big red box that doesn't convey any further information. And maybe we can compress the header a bit so you get a glance of the key information right away without having to scroll past irrelevant stuff. BTW: Any idea how this email looks like on mobile? I often get these emails while on the road and it'd be great to (more or less) instantly be able to figure out whether it's something to worry about!

mentioned in issue #13414 (closed)

Hi, The mock looks pretty good and will definitly improve CI email a lot.

As a developer and integrator, to me it's important to quickly identify the project corresponding to the build. In the group/project name, is it possible to prefix each of them with its avatar (if any)?

For the "branch", what if the build was triggered by something else than a push on a branch, like the creation of a tags (which technically require a push, yes, but the important information in this case is the tag itself, which may correspond to a release)? And how about manual triggered builds? Maybe called this line "Ref" and add if possible the tag name (if any) after the branch name?

The first text in the red box is disturbing me : "Something went wrong with your tests". I don't think tests is the right word, as CI jobs are not only used to launch tests (compilation, build, packaging, deployment...).

@markpundsack you assigned the %8.11 milestone some time ago. I assume this won't make it in time, but could you propose how to proceed?

@pbr

Yes, you are right. We didn't manage to make it on time. We finished all Backend work required to have that implemented. You will definitely see that in 8.12.

Milestone changed to %8.12

mentioned in issue #21277 (closed)

Reassigned to @godfat

I'll start working on this in the next few days.

I assume what we currently have is BuildsEmailService, and we would want PipelinesEmailService here instead. However we might want to convert existing BuildsEmailService into the new PipelinesEmailService, but I am not sure how we should do this. We could either just use the existing data, or add a migration to convert them.

I'll begin with adding new data.

On the other hand, I always want this notification go to the author of who ever triggered this pipeline instead of a pre-set recipients. Could we also add this? Given a large project, I don't think a fixed recipients would work well.

Oh, I guess the option: add_pusher is exactly for adding the person who triggered the build. I'll copy this behaviour.

So the other thing I want this be improved is that, for now only the admin/master could add this service and enable add_pusher. However personally I would like to enable this for myself. I am quite tired of constantly refreshing pages nor do I want to enable this to annoy everyone involved in the same project.

This is probably out of scope of adding a new service for pipeline emails though, since this is how service is working right now.

mentioned in merge request !6019 (merged)

Personally, I feel that pipeline notifications should be first-class, not just a "service". For example, they should be enabled by default with good default settings rather than needing to be added and configured. But perhaps there are other ways to solve that.

We should email whoever broke the build, which would generally be the pusher, but technically there could be different committers in the build.

CircleCI also emails when builds start working again (only once), and people seem to like that. Might be out of scope for this, but something to keep in mind.

Personally, I feel that pipeline notifications should be first-class, not just a "service". For example, they should be enabled by default with good default settings rather than needing to be added and configured. But perhaps there are other ways to solve that.

I do also think that this should be integrated as part of GitLab notifications, rather than a service. For example, a failed build should probably be added as a TODO as well, and it could be hard to do so with service approach.

On the other hand, in which I mentioned in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5142#note_14486306, it would be nice if we have something like:

/after ci-success /assign ...

It makes little sense to assign to the reviewer only that seeing the builds didn't pass and assign back.

/after ci-failed /notify @me

We won't need this if it's already doing so, but as reviewers they might want to assign to someone whenever the builds didn't pass, for example.

This way I could do some simple automation from commands, and from inbox, so that I don't even have to navigate to GitLab to make my actions. However I am not sure if this is generalized enough to make this happen.

We should email whoever broke the build, which would generally be the pusher, but technically there could be different committers in the build.

Yes the builds could contain committers other than the pusher, but I think it's fine to exclude them. For example, I could be merging from master having 1000 commits and I don't really want to bother all of the committers :P

It's funny that I did contribute to JRuby a few times, and at the time they're using JIRA, and I received a few failing builds notifications, which were completely irrelevant to my changes. I was not annoyed because I didn't receive those too many times, but it could be annoying if it's not handled properly.

CircleCI also emails when builds start working again (only once), and people seem to like that. Might be out of scope for this, but something to keep in mind.

I believe TravisCI would also notify whenever the builds are recovered from failing. I do also like that otherwise I could think that the builds are still running and I have no idea if they succeeded or not.

Yes @markpundsack and @godfat. At first I thought about making it as a service, but I think that it still makes more sense to have that as a first-class thing, working similar to issues.

We could easily hook that with GitLab notification system, and dispatch the messages using the same mechanism where people have their own notification rules.

I generally don't like an e-mails to be send to committers, because when you push someone else code you don't really want them to be notified in case of test failure. It did happen for me a few times :) So if we were to refactor BuildsEmailService we should send a notification on pipeline to a list of predefined e-mails and/or committer, but since the committer will be already covered by former (hooks in GitLab notifiaction system) it seems like a not relevant.

So my idea for doing that now. Is to extend NotificationService with templates for sending pipeline_success_email and pipeline_failed_email and hook these notifications to Pipeline.state_machine.

Oh, so basically we're not going to use the existing approach? Then I was doing it wrong in !6019 (merged). I'll correct that.

Apparently the backend is being worked on already and scheduled for %8.12 . What's the status of the UX/design part? @hazelyang

mentioned in issue #21662 (moved)

mentioned in issue #21930 (closed)

Were @orobardet's comments above ever addressed?

Particularily:

The first text in the red box is disturbing me : "Something went wrong with your tests". I don't think tests is the right word, as CI jobs are not only used to launch tests (compilation, build, packaging, deployment...).

Should we use "CI builds" or "pipelines" similar phrasing instead of "tests"?

@mikegreiling We should do. Maybe "pipelines" would be better since we use it more often on gitlab.com, right?

To be consistent I think we should use "pipeline". This would be used for a particular "pipeline", which has many "jobs". So either "pipeline" or "jobs".

@mikegreiling @hazelyang @godfat

I suggest rephrasing/replacing/dropping the entire sentence (also see my comment above, https://gitlab.com/gitlab-org/gitlab-ce/issues/3976#note_13607597). I find this "something went wrong" just awful and I guess that quite a few other developers will feel insulted by this wording. People receiving this email already know that "something went wrong" the moment they see the email subject. The point of the email is to explain what exactly went wrong and there is absolutely no reason to have that big box at the beginning of the email.

PS: What about moving the "Pipeline #... had 3 failed jobs" to the top? That is what the email is about and it is much more informative than a plain "something went wrong".
The main reason I don't like the latter (and feel so strongly about it) is that it insinuates that we don't know what went wrong. But we do! And that's why this email is sent out.

@pbr I agree that "something went wrong" is a bad phrase, and I thought that this phrase was from Rails. I propose to phrase it like Pipeline #1234 failed, or also have project and namespace in it like Pipeline #1234 for gitlab-org/gitlab-ce failed. I don't want to completely drop it because we could also email "succeeded" pipelines, or "recovered" pipelines. So I think it's good to have a very clear message on top. (with a very clear status of that pipeline)

Thanks @godfat. You're also completely right about the succeeded ones. Couldn't agree more

I think "insulting" is a bit of a stretch, but I agree the wording should be changed.

Agree with @godfat too.

OK, great we all agree

So what's the status of implementing this, both on the backend and the UX/frontend side?

I am currently reading up on HTML email best practices to make sure I'm up to date. I've done this before but it was a few years ago and technology changes fast :)

Hoping I don't still have to regress to inline-styles and table-based layouts like I used to just to support arcane email clients like Outlook 2003.

@pbr, @hazelyang The WIP is here: !6019 (merged)

Awesome, thanks @mikegreiling. I wasn't aware that this is already WIP

The backend for pipeline email service is considered done, and in the mean time, I am also trying to integrate this into the notification system so that it would be working out of the box. With pipeline email service we have right now (in the WIP merge request !6019 (merged)), it would need to be enabled for each projects so it could be cumbersome to setup.

You could also track that progress in #21930 (closed). It would use the same WIP merge request as well, since it's highly depending on it. @mikegreiling So I might still push some changes to the branch. Please don't rebase while we're both working on it :)

On the other hand, since there are still a few left to work on, I am not sure if we could make this into %8.12. Only 10 days left. If the notification can't be done but the front-end template is already done, then I would cut it off and still ship with pipeline email service. But if we can't make the template done in %8.12 then let's ship them all in %8.13.

Wow, we're already only 10 days out from 8.12? still gotta get myself used to the breakneck release schedule here.

@godfat Barring any unforeseen issues I think I can manage to get this template done by then. I'll keep you updated, and I will wait to commit until we're both on the same page regarding rebases... Though if you rebase, I'm fine with cherry-picking my commits and fixing any mess that might come up :)

BTW, how do you link milestones within comments like you did above? I know ! for merge requests, # for issues, and ~ for tags... is there a glossary someplace?

@mikegreiling %, see https://docs.gitlab.com/ce/user/markdown.html#special-gitlab-references

@mikegreiling Feel free to commit and push! :) Save your works, and we could discuss on the code early if you're committing. I am very used to collaborating on the same branch, no worries.

@mikegreiling Thanks!!

Added ~307491 label

@hazelyang could I get you to put together a "CI build succeeded" email design?

Also I've updated the message at the top to say

Oops, there was a failure in your CI pipeline!

Is this acceptable? Looking at the CircleCI implementation as a reference. I think the "Oops/Uh Oh, ..." adds a bit of levity and personality. I also think adding the pipeline # and repo in the message (as suggested above) are redundant as they are listed immediately below.

I've also made the design responsive on displays smaller than 640px (seen below on an iPhone 6s plus screen):

Lastly, would it be beneficial to link back to any open merge request associated with this branch (if available)? Would this go well after the "branch" listing in the design?

Branch .............. gh-pull-requests (merge request 1234)

Also did we decide if the "Commits" part of the message will include all of the commit messages being tested or just the last one? What do we do about really long commit messages? Do we just include the first line truncated to a certain number of characters?

@mikegreiling A failure read a bit weird to me because there could have a lot more failures.

Having a link to go to the corresponding merge request would be great. Honestly going to the branch is not too useful most of the time. If I go there it's because that branch page has a link to go to the open merge request... However that link was not always shown, for example it seems that if you don't merge against default branch, that won't show. So I guess there's some technical difficulty, and also the reason why we didn't include it in a lot of places.

For commit messages, for me just the first line is good enough.

@mikegreiling

Also did we decide if the "Commits" part of the message will include all of the commit messages being tested or just the last one? What do we do about really long commit messages? Do we just include the first line truncated to a certain number of characters?

It is a best practice to write long commit messages with short lines, where the first line is a heading or very short summary. See the Git book. Therefore, I'd suggest showing the first line, truncated to 50 characters.

Also I've updated the message at the top to say

Oops, there was a failure in your CI pipeline!

An improvement, IMO. To address @godfat's comment, I'd suggest "Your CI pipeline was not successful." and "Your CI pipeline was successful.".

Added ~15078 label

Milestone changed to %8.13

Added emails pipeline labels

Mentioned in issue #22445 (closed)

Mentioned in issue #17254 (moved)

Mentioned in merge request !6019 (merged)

Status changed to closed by merge request !6019 (merged)

Mentioned in commit d550e437

Has this feature landed on the last stable release? I still receive old-styled emails, and I did not find doc about it. Have I missed something?

Yes. You have to configure Pipeline emails service.

@ayufan Are we planning to enable this for the GitLab CE project so we start using it internally?

@ayufan Emails are activated since I receive them, they just do not have the fancy new look we can see on the screeshots above.

@azmeuk There is a difference between Builds emails service and Pipeline emails service. The latter will give you the fancy looking emails. (But don't ask me why the "builds" service still exists.)

@pbr We are taking iterative approach not removing everything on day one :)

Sure @ayufan But I also assumed that pipeline emails were a replacement for build emails (probably just as @azmeuk). Personally, I think this lacks a lot of documentation and activating the services simply has the worst UX ever and still contains old bugs. (Sorry for OT.)

When the builds emails service gets removed, will there be some sort of migration to the pipeline service? Is that planned and/or implemented yet?

@pbr thanks. It works now.

Yes. We will migrate configuration. cc @godfat

@pbr @azmeuk Yes we'll remove Builds emails service in favour of Pipeline emails service in !6348 (closed), after that we probably don't really need to explain the difference, I assume. On the other hand, I totally agreed that having to enable this service has really bad UX and feels quite foreign to the system, therefore we're also working on !6342 (merged) to integrate this directly to the notification system, therefore we don't need to enable this service unless you want to setup additional email addresses to receive those emails. We'll also remove pusher option while integrating this to the system, because it would just be replaced by it.

Mentioned in issue #18076 (moved)

Improve CI e-mail notifications

Description

Proposal

Links / references

Designs

Child items ...

Activity

Email template

Admin message

Admin message

Improve CI e-mail notifications

Description

Proposal

Links / references

Activity

Email template