JENKINS-55194 Add logLevel to support debugging and also reduce default excessive logging #12
Conversation
…lt excessive logging
|
Test results: Masked IP address.
|
|
|
||
| |logLevel | ||
| |String | ||
| a|Defaults to *SEVERE* |
There was a problem hiding this comment.
Is the a supposed to be at the start of this line?
There was a problem hiding this comment.
That is actually asciidoc syntax to support asciidoc content in column (list in this case)
https://asciidoctor.org/docs/user-manual/#cols-format
Look for the string below to find the table
The column styles are described in the table below.
There was a problem hiding this comment.
nice. I wondered if it might be related to the list.
| if(remote.appendName) { | ||
| interaction = { | ||
| when(line: _, from: standardOutput) { | ||
| logger.println("$remote.name|$it") |
There was a problem hiding this comment.
can we do something like this instead so we don't repeat as much?
| logger.println("$remote.name|$it") | |
| def logPrefix = remote.appendName ? "$remote.name|" : '' | |
| interaction = { | |
| when(line: _, from: standardOutput) { | |
| logger.println("$logPrefix$it") | |
| } | |
| when(line: _, from: standardError) { | |
| logger.println("$logPrefix$it") | |
| } | |
| } |
There was a problem hiding this comment.
Sure, let me update it.
There was a problem hiding this comment.
Updated with ce8bccb and also tested by deploying change on a local Jenkins again.
|
Thank you @ghenkes for a quick review. :) |
| try { | ||
| Level.parse(remote.logLevel) | ||
| } catch (IllegalArgumentException e) { | ||
| throw new IllegalArgumentException(getPrefix() + "Bad log level $remote.logLevel for $remote.name") |
There was a problem hiding this comment.
Can you add the original exception here?
| throw new IllegalArgumentException(getPrefix() + "Bad log level $remote.logLevel for $remote.name") | |
| throw new IllegalArgumentException(getPrefix() + "Bad log level $remote.logLevel for $remote.name", e) |
There was a problem hiding this comment.
As it was IllegalArgumentException itself, I don't think adding e would add much value.
| @@ -38,6 +40,9 @@ class Common { | |||
| validateUserAuthentication(remote) | |||
| validateHostAuthentication(remote) | |||
| validateProxyConnection(remote) | |||
| if(remote.logLevel) { | |||
There was a problem hiding this comment.
Nice catch, let me update it quickly.
| if(remote.logLevel) { | ||
| rootLogger.setLevel(Level.parse(remote.logLevel)) | ||
| } else { | ||
| rootLogger.setLevel(Level.SEVERE) |
There was a problem hiding this comment.
Is there much noise in org.hidetake to pick a default log level this high?
There was a problem hiding this comment.
Intention was to turn off the logging by default and leave it to the consumer to increase it based on the needs.
Description
JENKINS-55194 Add logLevel to support debugging and also reduce default excessive logging
See JENKINS-55194.