mirror of
https://github.com/prometheus/prometheus.git
synced 2024-12-24 21:24:05 -08:00
Fix FloatHistogram.Add/Sub mutating its argument
Signed-off-by: Linas Medziunas <linas.medziunas@gmail.com>
This commit is contained in:
parent
965e603fa7
commit
f99ecc376e
|
@ -268,12 +268,23 @@ func (h *FloatHistogram) Add(other *FloatHistogram) *FloatHistogram {
|
|||
h.Count += other.Count
|
||||
h.Sum += other.Sum
|
||||
|
||||
if other.Schema != h.Schema {
|
||||
other = other.ReduceResolution(h.Schema)
|
||||
var (
|
||||
otherPositiveSpans = other.PositiveSpans
|
||||
otherPositiveBuckets = other.PositiveBuckets
|
||||
otherNegativeSpans = other.NegativeSpans
|
||||
otherNegativeBuckets = other.NegativeBuckets
|
||||
)
|
||||
|
||||
if other.Schema < h.Schema {
|
||||
panic(fmt.Errorf("cannot add histogram with schema %d to %d", other.Schema, h.Schema))
|
||||
} else if other.Schema > h.Schema {
|
||||
otherPositiveSpans, otherPositiveBuckets = reduceResolution(otherPositiveSpans, otherPositiveBuckets, other.Schema, h.Schema, false)
|
||||
otherNegativeSpans, otherNegativeBuckets = reduceResolution(otherNegativeSpans, otherNegativeBuckets, other.Schema, h.Schema, false)
|
||||
}
|
||||
|
||||
h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, h.PositiveSpans, h.PositiveBuckets, other.PositiveSpans, other.PositiveBuckets)
|
||||
h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, h.NegativeSpans, h.NegativeBuckets, other.NegativeSpans, other.NegativeBuckets)
|
||||
h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets)
|
||||
h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets)
|
||||
|
||||
return h
|
||||
}
|
||||
|
||||
|
@ -284,12 +295,23 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) *FloatHistogram {
|
|||
h.Count -= other.Count
|
||||
h.Sum -= other.Sum
|
||||
|
||||
if other.Schema != h.Schema {
|
||||
other = other.ReduceResolution(h.Schema)
|
||||
var (
|
||||
otherPositiveSpans = other.PositiveSpans
|
||||
otherPositiveBuckets = other.PositiveBuckets
|
||||
otherNegativeSpans = other.NegativeSpans
|
||||
otherNegativeBuckets = other.NegativeBuckets
|
||||
)
|
||||
|
||||
if other.Schema < h.Schema {
|
||||
panic(fmt.Errorf("cannot subtract histigram with schema %d to %d", other.Schema, h.Schema))
|
||||
} else if other.Schema > h.Schema {
|
||||
otherPositiveSpans, otherPositiveBuckets = reduceResolution(otherPositiveSpans, otherPositiveBuckets, other.Schema, h.Schema, false)
|
||||
otherNegativeSpans, otherNegativeBuckets = reduceResolution(otherNegativeSpans, otherNegativeBuckets, other.Schema, h.Schema, false)
|
||||
}
|
||||
|
||||
h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, h.PositiveSpans, h.PositiveBuckets, other.PositiveSpans, other.PositiveBuckets)
|
||||
h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, h.NegativeSpans, h.NegativeBuckets, other.NegativeSpans, other.NegativeBuckets)
|
||||
h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, h.PositiveSpans, h.PositiveBuckets, otherPositiveSpans, otherPositiveBuckets)
|
||||
h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, h.NegativeSpans, h.NegativeBuckets, otherNegativeSpans, otherNegativeBuckets)
|
||||
|
||||
return h
|
||||
}
|
||||
|
||||
|
|
|
@ -1573,9 +1573,12 @@ func TestFloatHistogramAdd(t *testing.T) {
|
|||
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
in2Copy := c.in2.Copy()
|
||||
require.Equal(t, c.expected, c.in1.Add(c.in2))
|
||||
// Has it also happened in-place?
|
||||
require.Equal(t, c.expected, c.in1)
|
||||
// Check that the argument was not mutated.
|
||||
require.Equal(t, in2Copy, c.in2)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
@ -1659,9 +1662,12 @@ func TestFloatHistogramSub(t *testing.T) {
|
|||
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
in2Copy := c.in2.Copy()
|
||||
require.Equal(t, c.expected, c.in1.Sub(c.in2))
|
||||
// Has it also happened in-place?
|
||||
require.Equal(t, c.expected, c.in1)
|
||||
// Check that the argument was not mutated.
|
||||
require.Equal(t, in2Copy, c.in2)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue