Skip to content

Log rotate size#236

Merged
und1sk0 merged 26 commits intomasterfrom
log-rotate-size
Dec 18, 2015
Merged

Log rotate size#236
und1sk0 merged 26 commits intomasterfrom
log-rotate-size

Conversation

@und1sk0
Copy link
Copy Markdown
Contributor

@und1sk0 und1sk0 commented Dec 10, 2015

Create an output channel for app logs that rotate based on file-size for sources tagged with syslog facility local7.

Remove input channel tailing raw JSON output from Bunyan.

Manage old logs with a rotate script that compresses logs as they are rotated and removes anything > 72hrs.

ETA - looks like the Ansible is... totally wrong ;)

Update - the rotation script has to truncate the logfile, and rsyslog should not drop privs in /var/log.

test

  • see log rotation work

code review

if no checkboxes with your name are present, and you are reviewing this PR please add your name
if no checkboxes are checked, then just check your checkbox
if there is one checkbox already checked, and you are ok with change, check your box and merge

@bkendall
Copy link
Copy Markdown
Contributor

can we move /home/ubuntu/bin/rotate-logs.sh to /opt/runnable/bin/rotate-logs.sh? we've kinda been consolidating all the runnable scripts into that directory

@bkendall
Copy link
Copy Markdown
Contributor

there's also a significant flaw w/ this. basically, you're using the same script that has a wildcard in it to rotate the logs. so if any log that rsyslog is being used for reaches the max size, it'll rotate all of them

Comment thread ansible/docker-listener.yml Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

probably move this down below git_node_service (since this runs in order), or maybe just closer. if you add tags here, then you don't need the tags field all over your loggly/tasks/main.yml

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

moved it below git_node_service but i'm not sure what i want to do with the tags per se so will leave as is (IINBDFI).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

saw your other comment, and this change. starting a new discussion thread. why does the [0-9] come into play? who is creating those files?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

according to rsyslog the default rotate pattern should be "." just like traditional syslog file rotation. but it occurred to me that if i'm always compressing .0, then i'll keep overwriting the prior rotation (which is not terrible but not necessarily what we want).

so...... i'm going to make a change to only run the bzip2 for files > 24 hours? 48 hours? let's do 48 hours for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh, btw..

bash-3.2$ find . -name ''a.[0-9]+;' bash-3.2$ find . -name ''a.{0-999}'' bash-3.2$ ls a.0 a.1 a.12 a.2 bash-3.2$ shopt -s extglob bash-3.2$ find . -name ''a.+([0-9])'' bash-3.2$ ls a.+([0-9]) a.0 a.1 a.12 a.2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't really care about calling bzip9 again on an already compressed file because bzip2 will just ignore it and move on to the next file:

bash-3.2$ bzip2 -9 /tmp/pewp/a.[0-9]* bzip2: Input file /tmp/pewp/a.0.bz2 already has .bz2 suffix.

Christopher M. Neill added 2 commits December 14, 2015 16:38
…mpress them > 24hrs and remove them > 72 hours.
…their main playbook (or down the road we just automatically include it via some ansible sorcery).

shipped.
@und1sk0
Copy link
Copy Markdown
Contributor Author

und1sk0 commented Dec 16, 2015

alright boys, let's ship this sucker.

@cflynn07
Copy link
Copy Markdown
Contributor

@und1sk0 walked me through these changes. They LGTM. Important note we must send logs to loggly over SSL before we launch.

Comment thread ansible/roles/loggly/tasks/main.yml Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

move to one off scripts

@und1sk0 und1sk0 added the review label Dec 18, 2015
@und1sk0 und1sk0 assigned rsandor and unassigned bkendall Dec 18, 2015
Christopher M. Neill added 2 commits December 17, 2015 19:23
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you confirm that 1mb is acceptable?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this was a copy from our existing loggly config, I set it to 1MB for out log sizes

@anandkumarpatel
Copy link
Copy Markdown

I want to see this working before merge, just for sanity

@und1sk0
Copy link
Copy Markdown
Contributor Author

und1sk0 commented Dec 18, 2015

At long last...

und1sk0 added a commit that referenced this pull request Dec 18, 2015
@und1sk0 und1sk0 merged commit b87a41b into master Dec 18, 2015
@und1sk0 und1sk0 deleted the log-rotate-size branch December 18, 2015 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants