Add a statsd check for clashing keys

This added unit-test check ensures that when we assert a statsd key is
set, we haven't tried to set any other subkeys under existing counter
or gauge keys.  Although this is not 100% reliable as different tests
could still create conflicting keys, this would have at least found
the problems in I127e8b6d08ab86e0f24018fd4b33c626682c76c7 and thus
rules out one sort of problem.

Fix the existing statsd test-case which is trying to set subkeys that
wouldn't work; rename them as we're we're testing the hostname
expansion.

Also add a negative test to ensure the failure is found when
asserting.

Change-Id: Ie1ca39659d32e45e0ad1d80ca55e28ae6568a8b4
This commit is contained in:
Ian Wienand 2018-11-28 13:19:19 +11:00
parent c6fe6459f2
commit 9236aae64b
2 changed files with 45 additions and 10 deletions

View File

@ -3128,17 +3128,42 @@ class ZuulTestCase(BaseTestCase):
# large single packets are sent with stats separated by
# newlines; thus we first flatten the stats out into
# single entries.
stats = itertools.chain.from_iterable(
[s.decode('utf-8').split('\n') for s in self.statsd.stats])
stats = list(itertools.chain.from_iterable(
[s.decode('utf-8').split('\n') for s in self.statsd.stats]))
# Check that we don't have already have a counter value
# that we then try to extend a sub-key under; this doesn't
# work on the server. e.g.
# zuul.new.stat is already a counter
# zuul.new.stat.sub.value will silently not work
#
# note only valid for gauges and counters; timers are
# slightly different because statsd flushes them out but
# actually writes a bunch of different keys like "mean,
# std, count", so the "key" isn't so much a key, but a
# path to the folder where the actual values will be kept.
# Thus you can extend timer keys OK.
already_set_keys = set()
for stat in stats:
k, v = stat.split(':')
s_value, s_kind = v.split('|')
if s_kind == 'c' or s_kind == 'g':
already_set_keys.update([k])
for k in already_set_keys:
if key != k and key.startswith(k):
raise Exception(
"Key %s is a gauge/counter and "
"we are trying to set subkey %s" % (k, key))
for stat in stats:
k, v = stat.split(':')
s_value, s_kind = v.split('|')
if key == k:
if kind is None:
# key with no qualifiers is found
return True
s_value, s_kind = v.split('|')
# if no kind match, look for other keys
if kind != s_kind:
continue

View File

@ -2519,12 +2519,22 @@ class TestScheduler(ZuulTestCase):
# test key normalization
statsd.extra_keys = {'hostname': '1_2_3_4'}
statsd.incr('test-incr.{hostname}.{fake}', fake='1:2')
statsd.timing('test-timing.{hostname}.{fake}', 3, fake='1:2')
statsd.gauge('test-gauge.{hostname}.{fake}', 12, fake='1:2')
self.assertReportedStat('test-incr.1_2_3_4.1_2', '1', 'c')
self.assertReportedStat('test-timing.1_2_3_4.1_2', '3', 'ms')
self.assertReportedStat('test-gauge.1_2_3_4.1_2', '12', 'g')
statsd.incr('hostname-incr.{hostname}.{fake}', fake='1:2')
statsd.timing('hostname-timing.{hostname}.{fake}', 3, fake='1:2')
statsd.gauge('hostname-gauge.{hostname}.{fake}', 12, fake='1:2')
self.assertReportedStat('hostname-incr.1_2_3_4.1_2', '1', 'c')
self.assertReportedStat('hostname-timing.1_2_3_4.1_2', '3', 'ms')
self.assertReportedStat('hostname-gauge.1_2_3_4.1_2', '12', 'g')
def test_statsd_conflict(self):
statsd = self.sched.statsd
statsd.gauge('test-gauge', 12)
# since test-gauge is already a value, we can't make
# subvalues. Test the assert works.
statsd.gauge('test-gauge.1_2_3_4', 12)
self.assertReportedStat('test-gauge', '12', 'g')
self.assertRaises(Exception, self.assertReportedStat,
'test-gauge.1_2_3_4', '12', 'g')
def test_stuck_job_cleanup(self):
"Test that pending jobs are cleaned up if removed from layout"