Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Failure logging is incorrect #460

Closed
bbangert opened this issue May 5, 2016 · 4 comments
Closed

Failure logging is incorrect #460

bbangert opened this issue May 5, 2016 · 4 comments
Assignees
Labels

Comments

@bbangert
Copy link
Member

bbangert commented May 5, 2016

Logging errors is broken as instances are still sneaking in, better checks need to be done, and the format attempt itself should be in a try/except, with errors sentry reported.

@jrconlin
Copy link
Member

jrconlin commented May 5, 2016

@bbangert could you add an example of a bad log entry? It'd be useful in the future to check for regression or if we see something similar.

@bbangert
Copy link
Member Author

bbangert commented May 5, 2016

Sure, this is a sample:

May 05 16:53:21 ip-172-31-39-23 python[11560]: Unable to format event {'uaid_hash': 'asdfasdfasdfasdfasdfsd', 'log_namespace': 'autopush.endpoint.EndpointHandler', 'log_level': <LogLevel=critical>, 'message_ttl': '200', 'log_logger': <Logger 'autopush.endpoint.EndpointHandler'>, 'channelID': 'asdfasdfasdfasdfasdf', 'log_source': <autopush.endpoint.EndpointHandler object at 0x38f99c50>, 'log_format': <twisted.python.failure.Failure exceptions.KeyError: u'prod.autopush.message_2016_2'>, 'message_size': 489, 'authorization': '', 'router_key': u'webpush', 'request_id': 'fa877aa4-1702-4d13-ba4d-5d2ab313b165', 'log_time': 1462467201.143596, 'remote_ip': '184.173.141.4', 'message_id':'asdfasdfasdfasdfasdfasdfasdfsdaf-asdfasdfasdfasdfsdafasdfasdf=', 'log_failure': <twisted.python.failure.Failure exceptions.KeyError: u'prod.autopush.message_2016_2'>, 'user_agent': ''}: Log format must be unicode or bytes, not <twisted.python.failure.Failure exceptions.KeyError: u'prod.autopush.message_2016_2'> 

I replaced some ID's there of course. In this case log_format seems to have gotten a failure instance which can't be formatted of course. This definitely should've resulted in a Sentry call as well, but no Sentry call, I'm not sure its being called properly actually.

Per https://docs.getsentry.com/hosted/clients/python/api/, it appears captureException takes a 'message' argument, and then the exc_info. We are not passing a message, and only passing the exc_info tuple, so that might explain the problem.

We should have a method somewhere we can hit when in stage, that trigger an exception on purpose for Sentry reporting so we can verify it works.

@bbangert
Copy link
Member Author

bbangert commented May 5, 2016

I should note, since the problematic fields here are not included in our custom JSON, my guess is that the formatEvent call on line 127 is failing, thus this error. It should be wrapped in a try/except and the message can be set to, 'unable to format event', and include a repr of the event instead.

jrconlin added a commit that referenced this issue May 6, 2016
Declare the arguments for raven to hopefully prevent key corruption

Closes #460
@jrconlin jrconlin self-assigned this May 6, 2016
@jrconlin jrconlin added in progress and removed ready labels May 6, 2016
@bbangert bbangert changed the title Log formatting is broken Failure logging is incorrect May 6, 2016
@bbangert
Copy link
Member Author

bbangert commented May 6, 2016

We are apparently not logging failures correctly. log.failure takes a message as the first arg, not a failure instance. The failure instance should be passed as failure=, per the docs: https://twistedmatrix.com/documents/16.1.1/api/twisted.logger.Logger.html#failure

jrconlin added a commit that referenced this issue May 6, 2016
Declare the arguments for raven to hopefully prevent key corruption.
NOTE: twisted logging appears to be fairly touchy about parameters. We
fully specify the parameter names to prevent issues with arguments being
used in unexpected ways and generating odd exceptions in handling.
Because errors in error handling just mean higher bar bills.

Closes #460
jrconlin added a commit that referenced this issue May 6, 2016
Declare the arguments for raven to hopefully prevent key corruption.
NOTE: twisted logging appears to be fairly touchy about parameters. We
fully specify the parameter names to prevent issues with arguments being
used in unexpected ways and generating odd exceptions in handling.
Because errors in error handling just mean higher bar bills.

Closes #460
jrconlin added a commit that referenced this issue May 6, 2016
Declare the arguments for raven to hopefully prevent key corruption.
NOTE: twisted logging appears to be fairly touchy about parameters. We
fully specify the parameter names to prevent issues with arguments being
used in unexpected ways and generating odd exceptions in handling.
Because errors in error handling just mean higher bar bills.

Closes #460
jrconlin added a commit that referenced this issue May 6, 2016
Declare the arguments for raven to hopefully prevent key corruption.
NOTE: twisted logging appears to be fairly touchy about parameters. We
fully specify the parameter names to prevent issues with arguments being
used in unexpected ways and generating odd exceptions in handling.
Because errors in error handling just mean higher bar bills.

Closes #460
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants