Most logging is reduced to info level by default
#39
Conversation
erichaagdev
changed the title
info levelinfo level
There was a problem hiding this comment.
LGTM in general.
I wonder if there is a benefit to emitting a single log message at LIFECYCLE level when DV injection is applied and enabled? This message could even suggest running with --info for more details.
(I've not had the "pleasure" of configuring a DV injection on a real-world project, so I'm not sure how necessary/helpful the different log messages are)
| @@ -93,7 +93,7 @@ if (!isTopLevelBuild) { | |||
| def requestedInitScriptName = getInputParam(gradle, 'develocity.injection.init-script-name') | |||
| def initScriptName = buildscript.sourceFile.name | |||
| if (requestedInitScriptName != initScriptName) { | |||
| logger.quiet("Ignoring init script '${initScriptName}' as requested name '${requestedInitScriptName}' does not match") | |||
| logger.info("Ignoring init script '${initScriptName}' as requested name '${requestedInitScriptName}' does not match") | |||
There was a problem hiding this comment.
I wonder if this should be a warning? Does it indicate some form of misconfiguration, or is there a legitimate use case where the init-script names wouldn't match?
There was a problem hiding this comment.
Good point, but it makes me wonder if it should only be a warning if develocity.injection-enabled=true, since we can assume the user's intention is to do auto-injection?
There was a problem hiding this comment.
For sure, I had assumed that the check for develocity.injection-enabled would precede this check.
There was a problem hiding this comment.
This was addressed in b8bcfae
|
@erichaagdev I personally prefer this approach, but I have an uneasy feeling about how it's going to help in the real world. When enabling DV injection for a large number of projects, how useful are these log messages in troubleshooting, and how difficult will it be to enable them globally across a bunch of projects? As an improvement we could leave these messages at 'info', but provide a mechanism to selectively bump them to 'lifecycle' via an environment variable. This would be a hybrid of this PR with your previous approach. WDYT? It might be worth discussing within the solutions team to get real-world feedback. There's no mechanism to do cross-cutting DV injection with GitHub Actions, so my experience is biased. |
@bigdaz if a user is wanting to set the log level for the injection script, I think it's safe to assume they either want injection logging on or off. At that point it makes more sense to me to just have a debug argument that either enables or disables injection logging, essentially what I had before.
I've had to debug CI injection before and we only used a single project to debug with. I don't see a good use case for needing to enable injection logging globally, but a debug argument could be applied to many projects if needed. The CI plugins could also add a checkbox to enable debug logging for the injection script. The BVS would use it too when |
|
I think it could be beneficial to always emit the logging a This is a different use case than someone configuring DV injection via CI for a set of projects. They may want to temporarily increase DV injection logging for debug purposes, but don't have the ability or desire to update the log level for each individual project. By changing your original PR to default logging to |
erichaagdev
force-pushed
the
erichaagdev/reduce-logging
branch
2 times, most recently
from
ae303aa to
cf13f8a
Compare
The logging done by the init script is mainly used for testing purposes and not needed by consumers. In cases where this additional logging is needed, e.g., for debugging injection in an environment, the property `develocity.injection.debug=true` can be used to enable it.
erichaagdev
force-pushed
the
erichaagdev/reduce-logging
branch
from
cf13f8a to
b8bcfae
Compare
erichaagdev
changed the title
info levelinfo level by default
The logging done by the init script is mainly used for testing purposes and not needed by consumers.
In cases where this additional logging is needed, e.g., for debugging injection in an environment, the property
develocity.injection.debug=truecan be used to enable it.