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

Log output from cron jobs. #2826

Merged
merged 2 commits into from Mar 12, 2015
Merged

Conversation

pmclanahan
Copy link
Contributor

No description provided.

@pmclanahan
Copy link
Contributor Author

@jgmize r?

@jgmize
Copy link
Contributor

jgmize commented Mar 10, 2015

This looks like it should work as intended, and I know that's basically what we were doing before, but I wonder if perhaps it would be better to redirect a log file instead /dev/null?

@pmclanahan
Copy link
Contributor Author

I'd be in favor of that. I'll take a look at the logrotate config and see if we can just drop a file in /var/log and have it handled.

@pmclanahan
Copy link
Contributor Author

or, of course, get ops to configure logrotate for us for this.

@pmclanahan
Copy link
Contributor Author

@jgmize okay. now this should log every command's output to a log file in /var/log/bedrock per environment. I did not name the files based on date. I figured we could create a logrotate config to handle rotation and compression. I did add a job that will log the date hourly since many (most) of the commands don't output any date or time information. If you know of a better solution for this then we should do it.

@pmclanahan pmclanahan changed the title Quiet the email from the new cron scripts. Log output from cron jobs. Mar 10, 2015
@@ -7,6 +7,7 @@

HEADER = '!!AUTO-GENERATED!! Edit {template}.tmpl instead.'
TEMPLATE_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'etc', 'cron.d'))
LOG_DIR = '/var/log/bedrock'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this dir already exists on the admin node, but perhaps we should still add a couple of lines to this script to create it if necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad plan. I created the folder manually myself just to make sure I was setting a real, usable, and sane default here. I'll add creation.

@pmclanahan
Copy link
Contributor Author

Updated.

@jgmize
Copy link
Contributor

jgmize commented Mar 10, 2015

Sorry @pmclanahan, but Travis doesn't like your latest update. :(

@pmclanahan
Copy link
Contributor Author

Ah yes, because we check the cron output and gen-crons.py can't create the /var/log/bedrock folder on Travis. Hmm...

@pmclanahan
Copy link
Contributor Author

Changes made @jgmize. Travis running again.

@jgmize
Copy link
Contributor

jgmize commented Mar 10, 2015

This is not a blocker, but one issue is that all the cron jobs in a given environment are now logging to the same file and in at least some cases I don't see a way of identifying which command the output in the file would be-- this is still a huge improvement over /dev/null though :)

@pmclanahan
Copy link
Contributor Author

It's true. The architecture could be improved. Is this worth going with you think, or should we do something better?

@pmclanahan
Copy link
Contributor Author

I think to get much better might be a decent investment of time. I'd be so happy if we had a good logging system for the running apps and all of this other stuff as well.

@jgmize
Copy link
Contributor

jgmize commented Mar 10, 2015

I think this is better than what we have now, and is fine to merge as-is, and we should follow up with a broader discussion about logging in the bedrock devops meeting today.
r+

@superawesome
Copy link

Can you send us a bug on setting up logrotate (or something) for this?

jgmize added a commit that referenced this pull request Mar 12, 2015
@jgmize jgmize merged commit f6e89fd into mozilla:master Mar 12, 2015
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.

None yet

4 participants