Closed Bug 961147 Opened 6 years ago Closed 6 years ago

tryserver feature request: allow producing and exposing an NSPR log

Categories

(Testing :: Mochitest, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: mayhemer, Assigned: mayhemer)

Details

Attachments

(1 file, 1 obsolete file)

I'm often able to reproduce a bug (crash/assertion) on the try server, but it's very hard to reproduce locally with intention of no more then to get an NSPR log using my selection of log modules.

There is no way to instruct the tryserver to do this for me, even that is actually quite simple, just set NSPR_LOG_MODULES env var to my desired string and NSPR_LOG_FILE to a file that is then uploaded (somewhere, probably when the try logs are...) and exposed publicly.

This would save a lot of work.  Only concern is that the nspr log file can get quit large when modules are improperly chosen.
Severity: normal → enhancement
OS: Windows 7 → All
Hardware: x86_64 → All
Your feature already exists, it's just totally undocumented so you'll have to figure out how to make it work. The env will include MOZ_UPLOAD_DIR set to a path like /builds/slave/talos-slave/test/build/blobber_upload_dir, and any files with an extension in the whitelist in https://github.com/catlee/blobber/blob/master/blobber/config.py#L12 that your test run drops in there will be uploaded to a server, and the URL will be TinderboxPrinted so you can see it on tbpl. So "all" you have to do is hack at the script that starts the browser for whatever test suite you're interested in, like http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#630 where mochitests set up their browser's env, and (handwaving, without having tried it) set env["NSPR_LOG_FILE"] = env["MOZ_UPLOAD_DIR"] + "/nspr.log" and set your NSPR_LOG_MODULES. And keep it under 400MB.
Phil, this works and helped me to catch the problem and have a log!  Thanks.

I may have an (almost) production quality patch for this, interested?
Attached patch v1 (obsolete) — Splinter Review
Phil, what do you think?  Is it worth taking in to the tree?
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #8363135 - Flags: review?(philringnalda)
Depends on: 961430
Seems like a fine idea to me, but mochitest is one of many many things I don't own.
Component: Tools → Mochitest
Product: Release Engineering → Testing
QA Contact: hwine
Version: other → Trunk
Comment on attachment 8363135 [details] [diff] [review]
v1

(suggested reviewers) says...
Attachment #8363135 - Flags: review?(philringnalda) → review?(ted)
Comment on attachment 8363135 [details] [diff] [review]
v1

Review of attachment 8363135 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with a few nits, and a few questions.

::: testing/mochitest/runtests.py
@@ +661,5 @@
>        browserEnv["XPCOM_DEBUG_BREAK"] = "stack-and-abort"
>  
> +    # Produce an NSPR log, is setup (see NSPR_LOG_MODULES global at the top of
> +    # this script).
> +    if NSPR_LOG_MODULES:

You should probably make this:
if NSPR_LOG_MODULES and "MOZ_UPLOAD_DIR" in os.environ:

@@ +664,5 @@
> +    # this script).
> +    if NSPR_LOG_MODULES:
> +      browserEnv["NSPR_LOG_MODULES"] = NSPR_LOG_MODULES
> +
> +      browserEnv["NSPR_LOG_FILE"] = "%s/nspr.log" % os.environ["MOZ_UPLOAD_DIR"]

Seems like you could just stick these in a temp dir instead of the upload dir, since you're just deleting them at the end of the run anyway.

@@ +1061,5 @@
>      self.stopWebSocketServer(options)
>      processLeakLog(self.leak_report_file, options.leakThreshold)
>  
> +    if NSPR_LOG_MODULES:
> +      with zipfile.ZipFile("%s/nsprlog.zip" % browserEnv["MOZ_UPLOAD_DIR"], "w", zipfile.ZIP_DEFLATED) as logzip:

Would it be more useful to simply have all the logs uploaded individually and be able to load them directly rather than having to load a zip? Are you worried about log size? We just had a similar discussion in bug 961207.

@@ +1065,5 @@
> +      with zipfile.ZipFile("%s/nsprlog.zip" % browserEnv["MOZ_UPLOAD_DIR"], "w", zipfile.ZIP_DEFLATED) as logzip:
> +        for logfile in glob.glob("%s/nspr*.log" % browserEnv["MOZ_UPLOAD_DIR"]):
> +          logzip.write(logfile)
> +          os.remove(logfile)
> +        logzip.close()

You don't need the close here, the with statement will close it for you.
Attachment #8363135 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> Comment on attachment 8363135 [details] [diff] [review]
> v1
> 
> Review of attachment 8363135 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with a few nits, and a few questions.
> 
> ::: testing/mochitest/runtests.py
> @@ +661,5 @@
> >        browserEnv["XPCOM_DEBUG_BREAK"] = "stack-and-abort"
> >  
> > +    # Produce an NSPR log, is setup (see NSPR_LOG_MODULES global at the top of
> > +    # this script).
> > +    if NSPR_LOG_MODULES:
> 
> You should probably make this:
> if NSPR_LOG_MODULES and "MOZ_UPLOAD_DIR" in os.environ:

Yes, this is the reason why B2G was failing on this.  Thanks.

> 
> @@ +664,5 @@
> > +    # this script).
> > +    if NSPR_LOG_MODULES:
> > +      browserEnv["NSPR_LOG_MODULES"] = NSPR_LOG_MODULES
> > +
> > +      browserEnv["NSPR_LOG_FILE"] = "%s/nspr.log" % os.environ["MOZ_UPLOAD_DIR"]
> 
> Seems like you could just stick these in a temp dir instead of the upload
> dir, since you're just deleting them at the end of the run anyway.

os.environ["TEMP"]?

Is it multiplaform?

> 
> @@ +1061,5 @@
> >      self.stopWebSocketServer(options)
> >      processLeakLog(self.leak_report_file, options.leakThreshold)
> >  
> > +    if NSPR_LOG_MODULES:
> > +      with zipfile.ZipFile("%s/nsprlog.zip" % browserEnv["MOZ_UPLOAD_DIR"], "w", zipfile.ZIP_DEFLATED) as logzip:
> 
> Would it be more useful to simply have all the logs uploaded individually
> and be able to load them directly rather than having to load a zip? Are you
> worried about log size? We just had a similar discussion in bug 961207.

No, the first version was just uploading all logs and it was quite luck to get under 400MB before the failure (fortunately a fatal assert) happen for me.

Zip saves time/bandwidth/space/money.  And everybody knows how to unzip it ;)
(In reply to Honza Bambas (:mayhemer) from comment #7)
> > Seems like you could just stick these in a temp dir instead of the upload
> > dir, since you're just deleting them at the end of the run anyway.
> 
> os.environ["TEMP"]?
> 
> Is it multiplaform?

You might be able to use mozfile.TemporaryDirectory:
http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozfile/mozfile/mozfile.py?force=1#309

If not, just tempfile.mkdtemp() is fine.

> > Would it be more useful to simply have all the logs uploaded individually
> > and be able to load them directly rather than having to load a zip? Are you
> > worried about log size? We just had a similar discussion in bug 961207.
> 
> No, the first version was just uploading all logs and it was quite luck to
> get under 400MB before the failure (fortunately a fatal assert) happen for
> me.
> 
> Zip saves time/bandwidth/space/money.  And everybody knows how to unzip it ;)

Right, same conversation we had in the Android logcat bug. Seems fine to zip them, we can always change it later if we build gzip support into the uploader.
No longer depends on: 961430
Attached patch v2Splinter Review
- using tempfile.gettempdir() to get the temp dir and store logs into it
- using GECKO_SEPARATE_NSPR_LOGS=1 to avoid write conflicts that founds to be enough to achieve it correctly for the purpose
- zip file left (as agreed)
Attachment #8363135 - Attachment is obsolete: true
Attachment #8363379 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c207d835c631
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.