1390736 - Make it possible to set RUST_LOG on content process only

Status: RESOLVED FIXED

(Core :: General, enhancement)

Description

[Xidorn Quan [:xidorn] UTC+11]

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

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1390736 - Add RUST_LOG_CHILD for child-only logging.

For certain use cases, it's nice to be able to target Rust debug logging to only
child processes (so you focus on web content vs. chrome running in the parent).

This adds `RUST_LOG_CHILD` which is copied to `RUST_LOG` when launching child
processes.

Review commit: https://reviewboard.mozilla.org/r/172384/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/172384/

Comment 2

[Nathan Froyd [:froydnj]]

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...

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=55bfb2425650a070ee4f8512c1c480597a7d4001

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e8c41b9bcc8
Add RUST_LOG_CHILD for child-only logging. r=froydnj

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1e8c41b9bcc8