most swift daemons don't honor log_name

Bug #680297 reported by clayg
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Low
gholt

Bug Description

the PasteDeploy loadapp/global_conf magic in run_wsgi enables you to set the log_name for each swift server independently. 'object-server' could be 'object-server1' or whatever other name you need to suit your needs.

Most of the daemons give the name of the logger as a string literal when they call get_logger:
self.logger = get_logger(conf, 'object-updater')

In general swift_deamons will use the string literal configured in their __init__ for the NamedLogger tag instead of the value you define in their config section (or DEFAULT) with the log_name key.

The exceptions I can find are the account-reaper, and the db-replicator subclasses. They seem take advantage of the fact that readconf will add a "log_name" key based on the "section_name" if it's not present in the config dict already. If the user sets log_name in the conf - it uses it!

So most calls to utils.get_logger could probably safely drop the literal name kwarg and trust the conf has one in it.

The only exception to that rule would be daemons that call readconf with a section_name of None (AFAIK, only the stats stuff) - they really do need to specify some sort of "default_name" when calling get_logger. In fact the readconf logic in "not section_name" for getting log_name may need it's own bug...

Related branches

Revision history for this message
clayg (clay-gerrard) wrote :

Then again, it may be better to just fix this in get_logger by treating name as a fallback option only.

In addition to having to update less files, it'd probably make more sense. If those daemons actually intended to override a user specified log_name setting it would have been clearer to force them to write:
conf['log_name'] = 'object-updater'
self.logger = get_logger(conf)

More than likely the intent is to provide a default log_name if conf doesn't define one - i.e.
self.logger = get_logger(conf, conf.get('log_name', 'object-updater'))

But in this case, there's no reason not to just do that for them inside get_logger, similarly to how run_wsgi accepts a default port to use only if there isn't already one specified in the conf.

I grepped around for calls to get_logger - I think it's probably safe for get_logger to just be updated so that name defaults to swift and is only used when the conf is missing the 'log_name' key.

clayg (clay-gerrard)
tags: added: low-hanging-fruit
Revision history for this message
gholt (gholt) wrote :

Ah, I'm glad I found this bug; now I don't have to make my own. I've got no idea what's really going on under the covers, but I do know that setting log_name in my [filter:section] didn't do what I expected at all. The local_conf passed to filter_factory doesn't even have log_name in it!

Revision history for this message
gholt (gholt) wrote :

A little more info: paste-deploy conf stuff is broken, in my opinion. If you specify a setting in [DEFAULT] and in a subsection, only the default value comes through. This is without any swift code present.

Revision history for this message
Jay Payne (letterj) wrote :

The only way to get the proxy to use anything other than LOG_LOCAL0 is to put the config setting in the [DEFAULT] section instead of the [app:proxy-server] section as the documentation indicates.

If this is not going to be corrected by the BEXAR release then the documentation should be changed to reflect the current behavior.

Revision history for this message
gholt (gholt) wrote :

Ah, figured a bit more out:

So if you have this conf file:

[DEFAULT]
name1 = globalvalue
name2 = globalvalue

[pipeline:main]
pipeline = myapp

[app:myapp]
use = egg:mypkg#myapp
name1 = localvalue
set name2 = localvalue
name3 = localvalue

The app_factory will get:

global {'__file__': '/etc/mypkg/wsgi.conf', 'here': '/etc/mypkg', 'name2': 'localvalue', 'name1': 'globalvalue'}
local {'name3': 'localvalue'}

So, if an item is specified in the [DEFAULT] section, it can only ever be in that section. If you specify it in the subsection plainly, it'll be ignored. If you specify it in the subsection, but prefix that with 'set', it'll override the value in the [DEFAULT] section, but note that the value is still in the [DEFAULT] section, just overridden -- it won't be in the subsection still. Any items only in the subsection will remain there.

Revision history for this message
gholt (gholt) wrote :

I think this is going to keep screwing with people no matter the documentation because it's screwy. (But, of course, we should try our best in the docs still.)

I would say we should just reparse the damned conf file in our app_factory and filter_factory methods, but there's no way to know which subsection is yours. The user can config several instances of your app/filter (multiple auths, for instance) and paste.deploy doesn't give you enough information in the _factory call.

Looks like we'll just have to field support questions on this for a long while...

Revision history for this message
gholt (gholt) wrote :

I have a working branch linked which documents the paste.deploy configuration file habits; but I still need to work through how we interact with it in the Swift code and make sure that's all okay and understood.

Changed in swift:
assignee: nobody → gholt (gholt)
status: New → In Progress
Revision history for this message
gholt (gholt) wrote :

I think I got it all sorted out. A combination of knowing how the paste.deploy config should work (updated docs) and fixing our get_logger code.

Thierry Carrez (ttx)
Changed in swift:
status: In Progress → Fix Committed
Chuck Thier (cthier)
Changed in swift:
milestone: none → 1.2.0
Chuck Thier (cthier)
Changed in swift:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.