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

Syslog ng debun improvements #1840

Merged
merged 4 commits into from
Jan 19, 2018

Conversation

jszigetvari
Copy link
Contributor

Details:

  • cosmetic changes to remove backtick-style code invokations in favor of dollarsign+parentheses
  • gather the persist file even when vardir is too big
  • gather two sets of syslog-ng stats, with Unix timestamps to allow EPS counting based on the diffs

    * removed backtick shell code invokations in favor of the dollarsign+parentheses style

Signed-off-by: Janos SZIGETVARI <jszigetvari@gmail.com>
    * harvest the persist file and some others even if the vardir is too big

Signed-off-by: Janos SZIGETVARI <jszigetvari@gmail.com>
@kira-syslogng
Copy link
Contributor

This user does not have permission to start the build. Can one of the admins verify this patch and start the build? (admin please type: ok to test)

@Kokan
Copy link
Collaborator

Kokan commented Jan 14, 2018

@kira-syslogng ok to test

@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

echo ${syslogbin}-ctl query get "*"
if [ -x ${syslogbin}-query ]; then
#if we reach this brach, then syslog.query.all - as generated above - will not contain any meaningful data
${syslogbin}-query sum "*" > ${tmpdir}/syslog.query.all 2>/dev/null
rm ${tmpdir}/syslog.query.all.${tsdata}
ts=$( timestamp )
Copy link
Collaborator

Choose a reason for hiding this comment

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

This variable is not used anywhere ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a typo all right.
Corrected.

echo ${syslogbin}-ctl stats
${syslogbin}-ctl query get "*" > ${tmpdir}/syslog.query.all 2>/dev/null
tsdata=$( timestamp )
${syslogbin}-ctl query get "*" > ${tmpdir}/syslog.query.all.${tsdata} 2>/dev/null
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could save the stderr also for the query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Saving the stderr output is not really helpful in this case. I don't want the user to see it, and we won't use it do determine why it did not work. The other parts of the script will give us enough information about syslog's state, so that we won't need stderr here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok.

@@ -1243,6 +1264,7 @@ setup_env_solaris () {
netstatlunp () { netstat -P udp -na ; }
myplimit () { plimit $1 ; }
free () { prtconf | grep Mem ; printf Pagesize:\ ; pagesize -a ; }
timestamp () { /usr/bin/perl -e 'print time, "\n";' ; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the timestamp re-defined here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The script has to run on multiple different platforms. This means that we have to handle somehow when a given OS behaves differently than the others. That is why we introduced OS-specific re-definitions of shell functions or variables.

In this specific case, Solaris does not support the date +%s way of getting the unix timestamp, so we had to override the default way of doing it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, one minor thing the path of perl I think should not be absolute (at least not hard coded absolute path), see also the sed_equivalent_cmd, where perl is relative.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

    * added timestamps to the generated stats files
    * added code to generate stats a second time at the end
    * this way it will be possible to calculate EPS values based on the stat diffs

Signed-off-by: Janos SZIGETVARI <jszigetvari@gmail.com>
@jszigetvari jszigetvari force-pushed the syslog-ng-debun-improvements branch from 6e94213 to 7596040 Compare January 16, 2018 08:47
@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

#handy-dandy delay magic, triggered when we'd step on our own foot
[ -f ${tmpdir}/syslog.stats.${tsdata} ] && sleep 5 && tsdata=$( timestamp )

${syslogbin}-ctl stats > ${tmpdir}/syslog.stats.${tsdata} 2>&1
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better (for reading) to create a separate variable for ${syslogbin}-ctl it is used in a few places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

    * fixed a typo in the example about syslog-ng residing in a custom place
      (Thank you Endre for pointing it out.)

contrib/syslog-ng-debun: miscellaneous modifications

    * version bump
    * put the paths for syslog-ng-ctl and syslog-ng-query into separate variables

Signed-off-by: Janos SZIGETVARI <jszigetvari@gmail.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@Kokan Kokan merged commit 61f7500 into syslog-ng:master Jan 19, 2018
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