From 77fc20834f67fa026c4da91f174b730ebddbd1cb Mon Sep 17 00:00:00 2001 From: Arthur Wang Date: Mon, 23 Mar 2015 18:12:26 -0400 Subject: [PATCH 1/2] Omit MAIN type from varnish metric names --- checks.d/varnish.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks.d/varnish.py b/checks.d/varnish.py index 597abc2997..b335faeb1f 100644 --- a/checks.d/varnish.py +++ b/checks.d/varnish.py @@ -49,7 +49,7 @@ def _end_element(self, name): # reset for next stat element self._reset() - elif name in ("type", "ident", "name"): + elif name in ("ident", "name") or (name == "type" and self._current_str != "MAIN"): self._current_metric += "." + self._current_str def _char_data(self, data): From dbf3a81f785fe2422738fcd17c99d132eb2f028f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Cavaill=C3=A9?= Date: Mon, 13 Apr 2015 10:08:47 -0400 Subject: [PATCH 2/2] [varnish/ci] Add real integration testing Test on 3.x and 4.x. Allows to test the different metrics and ensure non-regression. --- .travis.yml | 2 + Rakefile | 1 + checks.d/varnish.py | 18 +++-- ci/varnish.rb | 69 +++++++++++++++++ tests/test_varnish.py | 169 +++++++++++++++++++++++++++++------------- utils/shell.py | 24 ++++++ 6 files changed, 225 insertions(+), 58 deletions(-) create mode 100644 ci/varnish.rb create mode 100644 utils/shell.py diff --git a/.travis.yml b/.travis.yml index 4bd05eeca5..3ba5fbf620 100644 --- a/.travis.yml +++ b/.travis.yml @@ -74,6 +74,8 @@ env: # hugepages are enabled # - TRAVIS_FLAVOR=tokumx - TRAVIS_FLAVOR=tomcat # JMX testing machine / need the other ones before + - TRAVIS_FLAVOR=varnish FLAVOR_VERSION=3.0.7 + - TRAVIS_FLAVOR=varnish FLAVOR_VERSION=4.0.3 - TRAVIS_FLAVOR=zookeeper # Override travis defaults with empty jobs diff --git a/Rakefile b/Rakefile index 62eee0903c..b2fa7c5517 100755 --- a/Rakefile +++ b/Rakefile @@ -31,6 +31,7 @@ require './ci/supervisord' require './ci/sysstat' require './ci/tokumx' require './ci/tomcat' +require './ci/varnish' require './ci/zookeeper' CLOBBER.include '**/*.pyc' diff --git a/checks.d/varnish.py b/checks.d/varnish.py index b335faeb1f..944926af9a 100644 --- a/checks.d/varnish.py +++ b/checks.d/varnish.py @@ -87,11 +87,9 @@ def check(self, instance): tags += [u'varnish_name:%s' % name] else: tags += [u'varnish_name:default'] - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - output, error = proc.communicate() - if error and len(error) > 0: - self.log.error(error) + + output = self._get_varnishstat_output(cmd) + self._parse_varnishstat(output, use_xml, tags) # Parse service checks from varnishadm. @@ -104,6 +102,14 @@ def check(self, instance): if output: self._parse_varnishadm(output) + def _get_varnishstat_output(self, cmd): + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + output, error = proc.communicate() + if error and len(error) > 0: + self.log.error(error) + return output + def _get_version_info(self, varnishstat_path): # Get the varnish version from varnishstat output, error = subprocess.Popen([varnishstat_path, "-V"], @@ -171,6 +177,8 @@ def _parse_varnishstat(self, output, use_xml, tags=None): """ tags = tags or [] + # FIXME: this check is processing an unbounded amount of data + # we should explicitly list the metrics we want to get from the check if use_xml: p = xml.parsers.expat.ParserCreate() p.StartElementHandler = self._start_element diff --git a/ci/varnish.rb b/ci/varnish.rb new file mode 100644 index 0000000000..73a6e83ab4 --- /dev/null +++ b/ci/varnish.rb @@ -0,0 +1,69 @@ +require './ci/common' + +def varnish_version + ENV['FLAVOR_VERSION'] || '4.0.3' +end + +def varnish_rootdir + "#{ENV['INTEGRATIONS_DIR']}/varnish_#{varnish_version}" +end + +namespace :ci do + namespace :varnish do |flavor| + task :before_install => ['ci:common:before_install'] + + task :install => ['ci:common:install'] do + unless Dir.exist? File.expand_path(varnish_rootdir) + sh %(curl -s -L\ + -o $VOLATILE_DIR/varnish-#{varnish_version}.tar.gz\ + https://s3.amazonaws.com/dd-agent-tarball-mirror/varnish-#{varnish_version}.tar.gz) + sh %(mkdir -p #{varnish_rootdir}) + sh %(mkdir -p $VOLATILE_DIR/varnish) + sh %(tar zxf $VOLATILE_DIR/varnish-#{varnish_version}.tar.gz\ + -C $VOLATILE_DIR/varnish --strip-components=1) + # Hack to not install the docs that require python-docs + ENV['RST2MAN'] = 'echo' + sh %(cd $VOLATILE_DIR/varnish\ + && ./configure --prefix=#{varnish_rootdir}\ + && make -j $CONCURRENCY\ + && make install) + end + end + + task :before_script => ['ci:common:before_script'] do + sh %(#{varnish_rootdir}/sbin/varnishd -b 127.0.0.1:4242 -a 127.0.0.1:4000 -P $VOLATILE_DIR/varnish.pid) + # We need this for our varnishadm/varnishstat bins + ENV['PATH'] = "#{varnish_rootdir}/bin:#{ENV['PATH']}" + end + + task :script => ['ci:common:script'] do + this_provides = [ + 'varnish' + ] + Rake::Task['ci:common:run_tests'].invoke(this_provides) + end + + task :cleanup => ['ci:common:cleanup'] do + sh %(kill `cat $VOLATILE_DIR/varnish.pid`) + end + + task :execute do + exception = nil + begin + %w(before_install install before_script script).each do |t| + Rake::Task["#{flavor.scope.path}:#{t}"].invoke + end + rescue => e + exception = e + puts "Failed task: #{e.class} #{e.message}".red + end + if ENV['SKIP_CLEANUP'] + puts 'Skipping cleanup, disposable environments are great'.yellow + else + puts 'Cleaning up' + Rake::Task["#{flavor.scope.path}:cleanup"].invoke + end + fail exception if exception + end + end +end diff --git a/tests/test_varnish.py b/tests/test_varnish.py index d35f4e3c0d..b9ca81cf95 100644 --- a/tests/test_varnish.py +++ b/tests/test_varnish.py @@ -1,64 +1,127 @@ +# stdlib import os -import time -import unittest -from tests.common import get_check +# 3p +from nose.plugins.attrib import attr +# project +from tests.common import AgentCheckTest +from utils.shell import which -class VarnishTestCase(unittest.TestCase): - def setUp(self): - varnish_dump_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'varnish') - self.v_dump = open(os.path.join(varnish_dump_dir, 'v_dump'), 'r').read() - self.xml_dump = open(os.path.join(varnish_dump_dir, 'dump.xml'), 'r').read() +VARNISH_DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'varnish') - self.varnishadm_dump = open(os.path.join(varnish_dump_dir, 'varnishadm_dump'), 'r').read() - self.config = """ -instances: - - varnishstat: /usr/bin/varnishstat -""" +def mocked_varnishstatoutput(cmd): + return open(os.path.join(VARNISH_DATA_DIR, 'dump.xml'), 'r').read() - def test_parsing(self): - v, instances = get_check('varnish', self.config) - v._parse_varnishstat(self.v_dump, False) - metrics = v.get_metrics() - self.assertEquals([m[2] for m in metrics - if m[0] == "varnish.n_waitinglist"][0], 980) - assert "varnish.fetch_length" not in [m[0] for m in metrics] - # XML parsing - v._parse_varnishstat(self.xml_dump, True) - metrics = v.get_metrics() - self.assertEquals([m[2] for m in metrics - if m[0] == "varnish.SMA.s0.g_space"][0], 120606) - assert "varnish.SMA.transient.c_bytes" not in [m[0] for m in metrics] +COMMON_METRICS = [ + 'varnish.SMA.Transient.g_alloc', + 'varnish.SMA.Transient.g_bytes', + 'varnish.SMA.Transient.g_space', + 'varnish.VBE.default_127.0.0.1_4242.vcls', + 'varnish.n_backend', + 'varnish.n_expired', + 'varnish.n_lru_moved', + 'varnish.n_lru_nuked', + 'varnish.n_object', + 'varnish.n_objectcore', + 'varnish.n_objecthead', + 'varnish.n_vampireobject', + 'varnish.n_waitinglist', + 'varnish.sms_balloc', + 'varnish.sms_bfree', + 'varnish.sms_nbytes', + 'varnish.sms_nobj', + 'varnish.vmods', +] +METRICS_4_X = [ + 'varnish.MEMPOOL.busyobj.live', + 'varnish.MEMPOOL.busyobj.pool', + 'varnish.MEMPOOL.busyobj.sz_needed', + 'varnish.MEMPOOL.busyobj.sz_wanted', + 'varnish.MEMPOOL.req0.live', + 'varnish.MEMPOOL.req0.pool', + 'varnish.MEMPOOL.req0.sz_needed', + 'varnish.MEMPOOL.req0.sz_wanted', + 'varnish.MEMPOOL.req1.live', + 'varnish.MEMPOOL.req1.pool', + 'varnish.MEMPOOL.req1.sz_needed', + 'varnish.MEMPOOL.req1.sz_wanted', + 'varnish.MEMPOOL.sess0.live', + 'varnish.MEMPOOL.sess0.pool', + 'varnish.MEMPOOL.sess0.sz_needed', + 'varnish.MEMPOOL.sess0.sz_wanted', + 'varnish.MEMPOOL.sess1.live', + 'varnish.MEMPOOL.sess1.pool', + 'varnish.MEMPOOL.sess1.sz_needed', + 'varnish.MEMPOOL.sess1.sz_wanted', + 'varnish.MEMPOOL.vbc.live', + 'varnish.MEMPOOL.vbc.pool', + 'varnish.MEMPOOL.vbc.sz_needed', + 'varnish.MEMPOOL.vbc.sz_wanted', + 'varnish.SMA.s0.g_alloc', + 'varnish.SMA.s0.g_bytes', + 'varnish.SMA.s0.g_space', + 'varnish.bans', + 'varnish.bans_completed', + 'varnish.bans_obj', + 'varnish.bans_persisted_bytes', + 'varnish.bans_persisted_fragmentation', + 'varnish.bans_req', + 'varnish.n_obj_purged', + 'varnish.n_purges', + 'varnish.pools', + 'varnish.thread_queue_len', + 'varnish.threads', + 'varnish.vsm_cooling', + 'varnish.vsm_free', + 'varnish.vsm_overflow', + 'varnish.vsm_used' +] + +METRICS_3_X = [ + 'varnish.n_ban', + 'varnish.n_ban_gone', + 'varnish.n_sess', + 'varnish.n_sess_mem', + 'varnish.n_vbc', + 'varnish.n_wrk', + 'varnish.SMF.s0.g_smf', + 'varnish.SMF.s0.g_bytes', + 'varnish.SMF.s0.g_space', + 'varnish.SMF.s0.g_smf_large', + 'varnish.SMF.s0.g_alloc', + 'varnish.SMF.s0.g_smf_frag', +] + +class VarnishCheckTest(AgentCheckTest): + CHECK_NAME = 'varnish' + + @attr(requires='varnish') def test_check(self): - v, instances = get_check('varnish', self.config) - import pprint - try: - for i in range(3): - v.check({"varnishstat": os.popen("which varnishstat").read()[:-1]}) - pprint.pprint(v.get_metrics()) - time.sleep(1) - except Exception: - pass - - def test_service_check(self): - - v, instances = get_check('varnish', self.config) - v._parse_varnishadm(self.varnishadm_dump) - service_checks = v.get_service_checks() - self.assertEquals(len(service_checks), 2) - - b0_check = service_checks[0] - self.assertEquals(b0_check['check'], v.SERVICE_CHECK_NAME) - self.assertEquals(b0_check['tags'], ['backend:b0']) - - b1_check = service_checks[1] - self.assertEquals(b1_check['check'], v.SERVICE_CHECK_NAME) - self.assertEquals(b1_check['tags'], ['backend:b1']) - -if __name__ == '__main__': - unittest.main() + varnishstat_path = which('varnishstat') + self.assertTrue(varnishstat_path is not None, "Flavored testing should be run with a real varnish") + + config = { + 'instances': [{ + 'varnishstat': varnishstat_path, + 'tags': 'cluster:webs' + }] + } + + self.run_check(config) + version, _ = self.check._get_version_info(varnishstat_path) + + to_check = COMMON_METRICS + if version == 3: + to_check.extend(METRICS_3_X) + elif version == 4: + to_check.extend(METRICS_4_X) + + for mname in to_check: + self.assertMetric(mname, count=1) + + self.coverage_report() diff --git a/utils/shell.py b/utils/shell.py new file mode 100644 index 0000000000..3b8ae71dfa --- /dev/null +++ b/utils/shell.py @@ -0,0 +1,24 @@ +# stdlib +import os + + +def which(program): + """ shutil.which() goodness in python2.7 + Taken from + http://stackoverflow.com/questions/377017/test-if-executable-exists-in-python/377028#377028 + """ + def is_exe(fpath): + return os.path.isfile(fpath) and os.access(fpath, os.X_OK) + + fpath, fname = os.path.split(program) + if fpath: + if is_exe(program): + return program + else: + for path in os.environ["PATH"].split(os.pathsep): + path = path.strip('"') + exe_file = os.path.join(path, program) + if is_exe(exe_file): + return exe_file + + return None