Skip to content

Commit

Permalink
Fix TypeError for JSONFormatter and udp2log_host
Browse files Browse the repository at this point in the history
Summary:
Currently, in the case where `getMessage()` is called on a LogRecord
created by `JSONFormatter.make_record` it will fail. This is because
`JSONFormatter` treats args as a list instead of a tuple.

If there are actual args for use in the msg format string,
`getMessage()` will call:

record.msg % record.args

This will fail with something like:

'%s %s' % ['hello', 'world']
TypeError: not enough arguments for format string

Which is what happens inside the LogstashFormatter when a command fails
on a remote and the log record is propogated to Logstash. This shadows
the actual error message with:

File "/mnt/srv/deployment/scap/scap/scap/log.py", line 180, in format
    fields['message'] = fields['msg'] % fields['args']
    TypeError: not enough arguments for format string

Change-Id: I1c7360009e89aac9ab60ec5e84a4ded6af20a950

Test Plan:
On scap-vagrant:

- inside `scap/scap.cfg` set `udp2log_host: localhost`
- chown some files on the targets to create errors
- the final error should NOT be
  `TypeError: not enough arguments for format string`

Reviewers: demon, mmodell, #release-engineering-team, dduvall

Reviewed By: mmodell, #release-engineering-team, dduvall

Subscribers: jenkins

Differential Revision: https://phabricator.wikimedia.org/D83
  • Loading branch information
thcipriani committed Jan 6, 2016
1 parent e613d82 commit 4f94e28
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
12 changes: 10 additions & 2 deletions scap/log.py
Expand Up @@ -103,7 +103,7 @@ class JSONFormatter(logging.Formatter):
('filename', None),
('lineno', None),
('msg', ''),
('args', []),
('args', ()),
('exc_info', None),
('funcName', None)]

Expand All @@ -120,7 +120,15 @@ class JSONFormatter(logging.Formatter):
@staticmethod
def make_record(data):
fields = json.loads(data)
args = [fields.get(k, v) for k, v in JSONFormatter.DEFAULTS]
args = []
for k, v in JSONFormatter.DEFAULTS:
val = fields.get(k, v)

if k == 'args':
val = tuple(val)

args.append(val)

record = logging.LogRecord(*args)

for k in fields:
Expand Down
2 changes: 1 addition & 1 deletion tests/scap/test_log.py
Expand Up @@ -144,7 +144,7 @@ def test_make_record(self):
self.assertIsInstance(record, logging.LogRecord)

self.assertEqual(record.name, 'foo')
self.assertEqual(record.args, [])
self.assertEqual(record.args, ())
self.assertEqual(record.filename, 'foo_file')
self.assertEqual(record.levelno, 10)
self.assertEqual(record.lineno, 123)
Expand Down

0 comments on commit 4f94e28

Please sign in to comment.