If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Make it possible to set RUST_LOG on content process only

RESOLVED FIXED in Firefox 57

Status

()

Core
General
RESOLVED FIXED
a month ago
19 days ago

People

(Reporter: xidorn, Assigned: jryans)

Tracking

(Blocks: 1 bug)

Trunk
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a month ago
Stylo has lots of stuff printing in log, which sometimes can be very useful.

However, setting RUST_LOG directly would have it print log for both the chrome process and content process, which is not always desirable, especially given that all SVG images in chrome are now styled by stylo.

jryans has a patch [1] to take a "RUST_LOG_CHILD" environment for having it separately, which seems to be pretty useful. We probably should land something like that.


[1] https://github.com/jryans/gecko-dev/commit/4cfcac5b7512d1cadab700708e48e142f234e395
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 2

21 days ago
mozreview-review
Comment on attachment 8900934 [details]
Bug 1390736 - Add RUST_LOG_CHILD for child-only logging.

https://reviewboard.mozilla.org/r/172384/#review179084

Do you know if there's some obvious place we can document this?  Maybe a centralized logging wiki page?

::: ipc/glue/GeckoChildProcessHost.cpp:618
(Diff revision 1)
> +    rustLog.AssignLiteral("RUST_LOG=");
> +    rustLog.Append(childRustLog);
> +    PR_SetEnv(rustLog.get());
> +  }
> +
>    bool retval = PerformAsyncLaunchInternal(aExtraOpts, arch);

It occurs to me that we should really have a way of launching processes with extra environment variables, rather than trying to shoehorn in environment variables the way it's done above.  (This is not intended as a criticism of your approach.)

Maybe we tried that already and it doesn't work well, for some reason...
Attachment #8900934 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 3

20 days ago
mozreview-review-reply
Comment on attachment 8900934 [details]
Bug 1390736 - Add RUST_LOG_CHILD for child-only logging.

https://reviewboard.mozilla.org/r/172384/#review179084

It took a while to find, but the Gecko Logging[1] page seemed reasonable enough.  I added general Rust logging notes and mentioned this new thing too.

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Gecko_Logging

> It occurs to me that we should really have a way of launching processes with extra environment variables, rather than trying to shoehorn in environment variables the way it's done above.  (This is not intended as a criticism of your approach.)
> 
> Maybe we tried that already and it doesn't work well, for some reason...

Yeah, something more general could be useful!  Not sure if there would be any security / sandboxing implications with a more general env var munging system that applies only to certain processes...

I'll mention this new thing on dev-platform, and we can consider something more general if people think there's a need.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=55bfb2425650a070ee4f8512c1c480597a7d4001

Comment 5

19 days ago
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1e8c41b9bcc8
Add RUST_LOG_CHILD for child-only logging. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/1e8c41b9bcc8
Status: ASSIGNED → RESOLVED
Last Resolved: 19 days ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.