Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use logging system for failure logging #50

Merged
merged 1 commit into from
Jan 13, 2016
Merged

Conversation

defect
Copy link
Contributor

@defect defect commented Jan 12, 2016

When using log from within tasks we have a pretty decent logging system set up that allows for pluggable loggers (syslog + sending logs to collins for instance), but this is not being used by the task runner. This means that task failures only shows up on stdout since we just puts them.

This changes the framework to use the logging system instead for failure logging to ensure that that the reason for the failures, as well as retries, gets logged the same way log messages from the tasks does.

@tumblr/systems

@roymarantz
Copy link
Contributor

👍
do you want to also log task is being skipped messages or would that be too noisy? (I think it is too noisy)
Does the task name get included (correctly) in the log message? (not that this PR should fix that)

@defect
Copy link
Contributor Author

defect commented Jan 12, 2016

Yeah, i agree on that being too noisy. Perhaps if we had the notion of a severity level or something, but at that point it's starting to get pretty complex.

The log method prefixes the message with the task name. It looks something like this when failing in the run block:

FailTask :: task is attempting run #1...
FailTask :: Run #1 caused error: <error message here>
FailTask :: task is sleeping for 0 seconds...
FailTask :: task is attempting run #2...

and similarly for the precondition, init and condition blocks. I removed the task name from their error messages to avoid duplication.

@byxorna
Copy link
Contributor

byxorna commented Jan 12, 2016

I like it! Great job @defect! I would say dont worry about squashing noisy logs.... Leave it up to the log ingester to hide/drop noise like debug, etc. That already is the case with collins (default views on overview page only shows warning and higher severity).

@william-richard
Copy link
Contributor

LGTM

@defect
Copy link
Contributor Author

defect commented Jan 12, 2016

@byxorna The problem is that the logging system in genesis does't have different "levels" of logs. When using the collins logger for instance everything is logged as INFORMATIONAL.

It might make more sense to send everything through the logging system if/when we do have different levels, but right now I mostly want to get actual failures logged to something else than stdout. I'll open a ticket to investigate implementing a more proper logging system with different levels/severities though.

defect added a commit that referenced this pull request Jan 13, 2016
Use logging system for failure logging
@defect defect merged commit e9c3477 into master Jan 13, 2016
@defect defect mentioned this pull request Jan 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants