weird wraparound check

[Dieterbe]

@thepaul

query.py
170:        wraparound = target['tags'].get('wraparound')
171:        if wraparound is not None:
172:            cls.apply_graphite_function_to_target(target, 'nonNegativeDerivative', wraparound)

I don't see 'wraparound' being set anywhere? either way, anything that counts up and wraps around warranting a nonNegativeDerivative should by definition have target_type counter. so let's just check for that and remove the 'wraparound' tag stuff.

[thepaul]

So, I intended for the wraparound point to be declared as an intrinsic property of the metric, in its tags, i.e. by the plugin or in the proto2. It matters because some counters have different wraparound points (216, 231, 2**32 being the most common) and if you use the wrong wraparound point with nonNegativeDerivative it will often be totally wrong. And if you leave the wraparound point off entirely, it usually does a good job, but in a few cases it seems to guess wrong.

[thepaul]

Would it be ok if it always used nonNegativeDerivative for counters, but also kept the ability to use an explicit wraparound point?

[Dieterbe]

wow I'm suprised. I thought graphite's nonNegativeDerivative would be pretty trivial to implement in a robust way: as long as y2 >= y1 -> emit (y2-y1)/(x2-x1), if y2 < y1 emit 0. I assume here that we don't mind losing a simple datapoint. Are you saying the results without specifying the wraparound point are worse than "perfect minus 1 lost datapoint"?

Either way, yes it seems useful to optionally specify a wraparound point for increased accuracy. and yes, when deriving counters we should always use nonNegativeDerivative

[thepaul]

Yeah, it probably is just one lost datapoint per wraparound. But in a few cases those have been important datapoints, particularly at higher rollup levels.

[Dieterbe]

merged #66, this is no longer an issue