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:
parent
c6fe6459f2
commit
9236aae64b
|
@ -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
|
||||
|
|
|
@ -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"
|
||||
|
|
Loading…
Reference in New Issue