Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2962)

Issue 53590044: Have charm output logged under 'unit', not 'juju'

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by thumper
Modified:
10 years, 3 months ago
Reviewers:
mp+202038, wallyworld, axw
Visibility:
Public.

Description

Have charm output logged under 'unit', not 'juju' This branch addresses something that has been bugging me for some time, and that is that we have all logging on all the time. Some time back I changed it so we didn't have debug logging by default, and it was reverted under the guise of users wanting to see the charm output. Most of our users are not interested in seeing the internal guts of juju in their log files. We as developers are interested, but we have facilities at our control that makes it easy for us to manage. This branch also adds a help topic for logging, which will tell people the various options for how to set the logging levels. Charm hook output is now logged with the module: unit.<unit-id>.<hookname> I have updated the default logging writer for jujud. This makes the output from the unit modules different, so we see something like: 2014-01-17 01:41:30 INFO install Reading package lists... 2014-01-17 01:41:30 INFO install + apt-get -y upgrade No point duplicating the exact same file and line numbers, especially when the user doesn't care. Since the rsyslog tag now specifies the agent tag, in all-machines.log it now looks like this: unit-wordpress-0: 2014-01-17 01:41:30 INFO install Reading package lists... unit-wordpress-0: 2014-01-17 01:41:30 INFO install + apt-get -y upgrade So we get the unit id and the hook name without bloat. Other output logging is unchanged. The environment config is updated so logging for "unit" is always set to DEBUG unless the user explicitly sets it to a different value. As a drive by fix, the juju-log jujuc command now supports the --level parameter. https://code.launchpad.net/~thumper/juju-core/fix-charm-logging/+merge/202038 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 13

Patch Set 2 : Have charm output logged under 'unit', not 'juju' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+246 lines, -82 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/help_topics.go View 1 1 chunk +66 lines, -0 lines 0 comments Download
M cmd/juju/main.go View 1 chunk +1 line, -0 lines 0 comments Download
M cmd/juju/main_test.go View 1 2 chunks +2 lines, -1 line 0 comments Download
M cmd/jujud/main.go View 3 chunks +38 lines, -1 line 0 comments Download
M cmd/logging.go View 1 5 chunks +18 lines, -3 lines 0 comments Download
M cmd/logging_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M environs/config/config.go View 2 chunks +26 lines, -18 lines 0 comments Download
M environs/config/config_test.go View 4 chunks +16 lines, -14 lines 0 comments Download
M state/apiserver/logger/logger_test.go View 1 3 chunks +5 lines, -4 lines 0 comments Download
M testing/testbase/log.go View 1 chunk +1 line, -0 lines 0 comments Download
M worker/uniter/context.go View 6 chunks +13 lines, -5 lines 0 comments Download
M worker/uniter/jujuc/juju-log.go View 3 chunks +21 lines, -11 lines 0 comments Download
M worker/uniter/jujuc/juju-log_test.go View 1 chunk +33 lines, -24 lines 0 comments Download
M worker/uniter/jujuc/util_test.go View 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 4
thumper
Please take a look.
10 years, 3 months ago (2014-01-17 04:35:46 UTC) #1
axw
https://codereview.appspot.com/53590044/diff/1/cmd/juju/help_topics.go File cmd/juju/help_topics.go (right): https://codereview.appspot.com/53590044/diff/1/cmd/juju/help_topics.go#newcode427 cmd/juju/help_topics.go:427: users exposure to the logging mechanism is through either ...
10 years, 3 months ago (2014-01-17 04:55:42 UTC) #2
wallyworld
LGTM with some suggetions https://codereview.appspot.com/53590044/diff/1/state/apiserver/logger/logger_test.go File state/apiserver/logger/logger_test.go (right): https://codereview.appspot.com/53590044/diff/1/state/apiserver/logger/logger_test.go#newcode99 state/apiserver/logger/logger_test.go:99: s.setLoggingConfig(c, "<root>=WARN;juju.log.test=DEBUG;unit=INFO") See TestLoggingConfigForAgent comment ...
10 years, 3 months ago (2014-01-21 20:44:16 UTC) #3
thumper
10 years, 3 months ago (2014-01-22 02:34:48 UTC) #4
Please take a look.

https://codereview.appspot.com/53590044/diff/1/cmd/juju/help_topics.go
File cmd/juju/help_topics.go (right):

https://codereview.appspot.com/53590044/diff/1/cmd/juju/help_topics.go#newcod...
cmd/juju/help_topics.go:427: users exposure to the logging mechanism is through
either the 'debug-log'
On 2014/01/17 04:55:42, axw wrote:
> users' exposure?
> user exposure?

Done.

https://codereview.appspot.com/53590044/diff/1/cmd/juju/help_topics.go#newcod...
cmd/juju/help_topics.go:443: Juju has a hierarchical logging system internally,
and as a user you can
On 2014/01/17 04:55:42, axw wrote:
> It would be useful to provide an example log line, to show users how to
> determine the module that a message originated from. Otherwise they won't know
> what to specify.

Done.

https://codereview.appspot.com/53590044/diff/1/cmd/juju/help_topics.go#newcod...
cmd/juju/help_topics.go:458: - juju bootstrap --logging-config='...'
On 2014/01/17 04:55:42, axw wrote:
> Where's the flag for --logging-config? I can't see it on trunk, or in the MP.

cmd/logging.go

Although as pointed out this was --log-config.  Given that the rest
of the places we use logging-config, this has now been updated.

https://codereview.appspot.com/53590044/diff/1/cmd/juju/help_topics.go#newcod...
cmd/juju/help_topics.go:461: The value that is set is based on the hierarchical
logging system.
On 2014/01/17 04:55:42, axw wrote:
> I don't understand what is meant by this sentence. What's the format?

removed that line.

https://codereview.appspot.com/53590044/diff/1/state/apiserver/logger/logger_...
File state/apiserver/logger/logger_test.go (right):

https://codereview.appspot.com/53590044/diff/1/state/apiserver/logger/logger_...
state/apiserver/logger/logger_test.go:99: s.setLoggingConfig(c,
"<root>=WARN;juju.log.test=DEBUG;unit=INFO")
On 2014/01/21 20:44:16, wallyworld wrote:
> See TestLoggingConfigForAgent comment

Done.

https://codereview.appspot.com/53590044/diff/1/state/apiserver/logger/logger_...
state/apiserver/logger/logger_test.go:135: loggingConfig :=
"<root>=WARN;juju.log.test=DEBUG;unit=INFO"
On 2014/01/21 20:44:16, wallyworld wrote:
> s/loggingConfig/newLoggingConfig
> This makes it a little more explicit what is being done

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b