Closed Bug 511749 Opened 15 years ago Closed 14 years ago

tryserver: e-mails should include link to build results

Categories

(Release Engineering :: General, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: cjones, Assigned: catlee)

References

Details

(Whiteboard: [tryserver][oldbug])

Attachments

(2 files, 1 obsolete file)

I love the tryserver e-mail notification, but I wish it would send me a link to the build results it's notifying me about.  The tryserver tinderbox page is big enough now that it takes me a while to hunt down my results.
I'd like to second this request.

Chris: In the meantime, I've been trying this view out a bit:

    http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry
(In reply to comment #1)
> Chris: In the meantime, I've been trying this view out a bit:
> 
>     http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry

I like this too, but it leaks memory like nobody's business :S.  I got tired of waking up and finding my machine in swap.
Moving to future till this gets assigned.
Component: Release Engineering → Release Engineering: Future
Just FWIW, this is impossible with our current setup. The builders run buildbot, which sends an email to the Tinderbox server containing the build log, and then Tinderbox generates the log filename from the start time, the time the mail was received, and some other random bit (PID?). Buildbot, which sends you the email, has no way of knowing that logfile name.

We'll have to take Tinderbox out of the loop here to make this possible (which we'd like to do anyway).
Whiteboard: [tryserver]
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Priority: -- → P5
Assignee: nobody → catlee
Priority: P5 → P3
So, we can upload the logs in a few different formats:

* compressed or uncompressed: is it a burden to have to decompress the log?  If so, maybe we can look into teaching the web server how to adjust the Content-Type and Content-Encoding appropriately to serve compressed logs

* each sub-step of the build as a single log file, or the entire build as one log file.  In the first option, you'd have a dozen or so smaller log filed named something like 1-cleanup.txt, 2-update.txt, 3-build.txt, ...

By uploading the logs directly to FTP, we lose any parsing that Tinderbox is doing, so there will be no equivalent to the brief log.
It's hard to say without getting the chance to try it out. Compressed is fine. I guess I would say one log file for now.
Small suggestion: if there is no option for 'Brief log' then the URL should response as attachment content disposition.  It is unthinkable to display full logs in a browser (any).  I always save it by right-clicking the 'Show full log' link and display externally.
Unthinkable?  Painfully slow, yes, but that's a bug we should fix. :)
(In reply to comment #10)
> Unthinkable?  Painfully slow, yes, but that's a bug we should fix. :)

bug 474724.
Another note regarding to the log location.

We can put the log file(s) using 2 different approaches.

1. Put the log file(s) to the directory where built tarballs live. Easy to find even if you don't use email :)
 TODO:
 * modify post_upload.py to print the target directory (and host?)
 * modify build factories to set some additional properties (server, directory, filename?) which will be used in the buildbot status plugin.

2. Use "log central". Put the logs to yyyy/mm/dd/whatever unique directories and use the link in email notifications. The only way to find your log file will be reading the email.

The second method IMO is easier to integrate into the current setup. There is no need to modify post_upload.py and the build factories. Additionally I hope that some day buildbot will support slave side logging. :)
Depends on: 565344
The location doesn't really matter to me; being able to get to it easily from the email is the important part.
You can find some examples of how this might look here:
http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux/1274788709/

We'd be able to link to the appropriate log in the try server e-mails.

Any comments on the log formatting?
Attachment #449936 - Flags: feedback?(nrthomas)
Attachment #449936 - Flags: feedback?(bhearsum)
Comment on attachment 449936 [details] [diff] [review]
Upload logs to ftp

The idea is that after this is working, we can do the email generation via the same script and include the url to the logs in the mail.
Comment on attachment 449936 [details] [diff] [review]
Upload logs to ftp

I'm not super comfortable with the duplication of the post upload command generation in log_uploader.py. It would be nicer if we didn't have to keep this file + factory.py's upload commands in sync. This would be particularly problematic if we want to use this for all the different types of uploads we do (try, dep, nightly, release, etc.).

If we moved the post_upload.py arguments to config.py they could be shared by the factories and log handler. I'm not sure how to deal with dep vs. nightly or what to do for release, though.

+    if latest:
+        cmd.append("--release-to-latest")
+    if nightly:
+        cmd.append("--release-to-dated")

Any particular reason not to upload the logs to the latest directory, too? I can see people wanting to be able to see them without digging through dated directories.


I didn't step through the log handlers line by line, but they seem good after a brief look.
(In reply to comment #19)
> (From update of attachment 449936 [details] [diff] [review])
> I'm not super comfortable with the duplication of the post upload command
> generation in log_uploader.py. It would be nicer if we didn't have to keep this
> file + factory.py's upload commands in sync. This would be particularly
> problematic if we want to use this for all the different types of uploads we do
> (try, dep, nightly, release, etc.).
> 
> If we moved the post_upload.py arguments to config.py they could be shared by
> the factories and log handler. I'm not sure how to deal with dep vs. nightly or
> what to do for release, though.

Could we have some function in buildbotcustom that generates the command line?

> +    if latest:
> +        cmd.append("--release-to-latest")
> +    if nightly:
> +        cmd.append("--release-to-dated")
> 
> Any particular reason not to upload the logs to the latest directory, too? I
> can see people wanting to be able to see them without digging through dated
> directories.

I just want to make sure that the logs are in the same places the binaries are...I guess this goes back to your first point about making sure the command lines are consistent.

> I didn't step through the log handlers line by line, but they seem good after a
> brief look.
(In reply to comment #20)
> (In reply to comment #19)
> > (From update of attachment 449936 [details] [diff] [review] [details])
> > I'm not super comfortable with the duplication of the post upload command
> > generation in log_uploader.py. It would be nicer if we didn't have to keep this
> > file + factory.py's upload commands in sync. This would be particularly
> > problematic if we want to use this for all the different types of uploads we do
> > (try, dep, nightly, release, etc.).
> > 
> > If we moved the post_upload.py arguments to config.py they could be shared by
> > the factories and log handler. I'm not sure how to deal with dep vs. nightly or
> > what to do for release, though.
> 
> Could we have some function in buildbotcustom that generates the command line?
> 

Yeah, that would WFM.

> > +    if latest:
> > +        cmd.append("--release-to-latest")
> > +    if nightly:
> > +        cmd.append("--release-to-dated")
> > 
> > Any particular reason not to upload the logs to the latest directory, too? I
> > can see people wanting to be able to see them without digging through dated
> > directories.
> 
> I just want to make sure that the logs are in the same places the binaries
> are...I guess this goes back to your first point about making sure the command
> lines are consistent.

Yeah, we actually upload nightlies to latest and dated so this wouldn't be quite the same.
Comment on attachment 449936 [details] [diff] [review]
Upload logs to ftp

I already more or less gave this feedback+ in comments.
Attachment #449936 - Flags: feedback?(bhearsum) → feedback+
Attachment #449936 - Flags: feedback?(nrthomas)
Whiteboard: [tryserver] → [tryserver][oldbug]
Depends on: 530318
Attached patch Try mailer (obsolete) — Splinter Review
So this gets rid of the existing mail notifier for try, and replaces it with a wrapper around log_uploader.  bin/try_mailer.py calls log_uploader.py to upload the build logs, grabs the urls, and then constructs the email to notify the user where the binaries and logs are.
Attachment #471924 - Flags: review?(nrthomas)
Comment on attachment 471924 [details] [diff] [review]
Try mailer

>diff --git a/bin/try_mailer.py b/bin/try_mailer.py
>+def uploadLog(args):

How will we know if this starts failing ? Looks like we'll just start pointing people to tinderbox without notification to us to go fix it.

>+    s = SMTP()
>+    s.connect()
>+    s.sendmail(options.from_, options.to, msg.as_string())

Does everything get logged into twistd.log for debugging ?

>diff --git a/misc.py b/misc.py
>+    extra_args = []
>+    if config.get('enable_mail_notifier'):
....

Looks like this block is getting duplicated in a few places. Can we factor it out in a follow up ? Or now if you prefer.
Attachment #471924 - Flags: review?(nrthomas) → review+
Attached patch Try mailerSplinter Review
Attachment #471924 - Attachment is obsolete: true
Attachment #472736 - Flags: review?(nrthomas)
(In reply to comment #26)
> Comment on attachment 471924 [details] [diff] [review]
> Try mailer
> 
> >diff --git a/bin/try_mailer.py b/bin/try_mailer.py
> >+def uploadLog(args):
> 
> How will we know if this starts failing ? Looks like we'll just start pointing
> people to tinderbox without notification to us to go fix it.

I've changed it to return with the same exit code that log_uploader.py exits with.  The SubprocessLogHandler will pick up the non-zero exit code in cases of failure and put exceptions in the log.

> 
> >+    s = SMTP()
> >+    s.connect()
> >+    s.sendmail(options.from_, options.to, msg.as_string())
> 
> Does everything get logged into twistd.log for debugging ?

The entire message is printed out a few lines above.

> 
> >diff --git a/misc.py b/misc.py
> >+    extra_args = []
> >+    if config.get('enable_mail_notifier'):
> ....
> 
> Looks like this block is getting duplicated in a few places. Can we factor it
> out in a follow up ? Or now if you prefer.

Refactored!
Comment on attachment 472736 [details] [diff] [review]
Try mailer

(In reply to comment #28)
[mail sending code using SMTP module]
> > Does everything get logged into twistd.log for debugging ?
> The entire message is printed out a few lines above.

I really meant errors sending the mail, but if that has an exception and return a non-zero exit status then it'll end up in twistd.log like you said for uploadLog.
Attachment #472736 - Flags: review?(nrthomas) → review+
Comment on attachment 472736 [details] [diff] [review]
Try mailer

changeset:   952:07180ac9c3c1
Attachment #472736 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: