Skip to content

Commit

Permalink
Add unit tests
Browse files Browse the repository at this point in the history 
Fix bugs detected during unit tests
  • Loading branch information
@garyclee
garyclee committed Aug 21, 2023
1 parent 1622900 commit d068c93
Show file tree
Hide file tree
Showing 3 changed files with 388 additions and 24 deletions.
40 changes: 20 additions & 20 deletions pkg/promclient/multi_api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func TestMultiAPIMerging(t *testing.T) {
a: NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
labelNames: []string{"a"},
labelValues: []model.LabelValue{"1", "2"},
v: model.Vector{
Expand All @@ -226,12 +226,12 @@ func TestMultiAPIMerging(t *testing.T) {
NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 2),
}, model.Time(0), nil, 1, false),
}, model.Time(0), nil, 2, false),
labelNames: []string{"a"},
labelValues: []model.LabelValue{"1", "2"},
v: model.Vector{
Expand All @@ -250,23 +250,23 @@ func TestMultiAPIMerging(t *testing.T) {
NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 2),
}, model.Time(0), nil, 1, false),
}, model.Time(0), nil, 2, false),
NewMustMultiAPI([]API{
NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"b": "1"}},
&AddLabelClient{stub, model.LabelSet{"b": "1"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"b": "2"}},
&AddLabelClient{stub, model.LabelSet{"b": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 2),
}, model.Time(0), nil, 2),
}, model.Time(0), nil, 1, false),
}, model.Time(0), nil, 2, false),
}, model.Time(0), nil, 2, false),
labelNames: []string{"a", "b"},
labelValues: []model.LabelValue{"1", "2"},
v: model.Vector{
Expand Down Expand Up @@ -295,12 +295,12 @@ func TestMultiAPIMerging(t *testing.T) {
NewMustMultiAPI([]API{
&errorAPI{&AddLabelClient{stub, model.LabelSet{"a": "1"}}, fmt.Errorf("")},
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
NewMustMultiAPI([]API{
&errorAPI{&AddLabelClient{stub, model.LabelSet{"a": "2"}}, fmt.Errorf("")},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 2),
}, model.Time(0), nil, 1, false),
}, model.Time(0), nil, 2, false),
labelNames: []string{"a"},
labelValues: []model.LabelValue{"1", "2"},
v: model.Vector{
Expand All @@ -318,20 +318,20 @@ func TestMultiAPIMerging(t *testing.T) {
NewMustMultiAPI([]API{
&errorAPI{&AddLabelClient{stub, model.LabelSet{"a": "1"}}, fmt.Errorf("")},
&errorAPI{&AddLabelClient{stub, model.LabelSet{"a": "1"}}, fmt.Errorf("")},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
NewMustMultiAPI([]API{
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 2),
}, model.Time(0), nil, 1, false),
}, model.Time(0), nil, 2, false),
err: true,
},
// if in a multi, all that "match" error, we should error
{
a: NewMustMultiAPI([]API{
&errorAPI{&AddLabelClient{stub, model.LabelSet{"a": "1"}}, fmt.Errorf("")},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
err: true,
},
// however, in a multi if a single one succeeds for a given "group" then it should pass
Expand All @@ -340,7 +340,7 @@ func TestMultiAPIMerging(t *testing.T) {
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
&errorAPI{&AddLabelClient{stub, model.LabelSet{"a": "1"}}, fmt.Errorf("")},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
labelNames: []string{"a"},
labelValues: []model.LabelValue{"1", "2"},
v: model.Vector{
Expand All @@ -358,7 +358,7 @@ func TestMultiAPIMerging(t *testing.T) {
stub,
&AddLabelClient{stub, model.LabelSet{"a": "1"}},
&AddLabelClient{stub, model.LabelSet{"a": "2"}},
}, model.Time(0), nil, 1),
}, model.Time(0), nil, 1, false),
labelNames: []string{"a"},
labelValues: []model.LabelValue{"1", "2"},
v: model.Vector{
Expand Down
9 changes: 6 additions & 3 deletions pkg/promhttputil/merge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,19 +255,22 @@ func MergeSampleStream(antiAffinityBuffer model.Time, a, b *model.SampleStream,
} else {
done := false

// see if there is a nearby sample from b that is larger than a
// see if there is a sample from b within antiAffinityBuffer, that is larger than a
for i := lastOffset; i < len(b.Values); i++ {
bValue := b.Values[i]
// b is not within antiAffinityBuffer of a
if bValue.Timestamp > aValue.Timestamp+antiAffinityBuffer {
if bValue.Timestamp >= (aValue.Timestamp+antiAffinityBuffer) && bValue.Timestamp != aValue.Timestamp {
break
}
// b is within antiAffinityBuffer of a
if bValue.Timestamp > aValue.Timestamp-antiAffinityBuffer {
if bValue.Timestamp == aValue.Timestamp || bValue.Timestamp > (aValue.Timestamp-antiAffinityBuffer) {
// no need to iterate b before this offset next time
lastOffset = i
if bValue.Value > aValue.Value {
// use the larger value from b
// note: there may be larger values from b after this, we will choose the first one we find
// within the antiAffinityBuffer
bValue.Timestamp = aValue.Timestamp
newValues = append(newValues, bValue)
done = true
}
Expand Down
Loading

0 comments on commit d068c93

Please sign in to comment.