Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

Add keepLastValue function #995

Merged
merged 6 commits into from
Aug 15, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
added tests, removed string validation
stivenbb committed Aug 15, 2018
commit 940e62db9a2606746f51db140996eb045af971d1
14 changes: 3 additions & 11 deletions expr/func_keeplastvalue.go
Original file line number Diff line number Diff line change
@@ -3,16 +3,14 @@ package expr
import (
"fmt"
"math"
"strconv"

"github.com/grafana/metrictank/api/models"
schema "gopkg.in/raintank/schema.v1"
)

type FuncKeepLastValue struct {
in GraphiteFunc
limit int64
slimit string
in GraphiteFunc
limit int64
}

func NewKeepLastValue() GraphiteFunc {
@@ -26,7 +24,7 @@ func (s *FuncKeepLastValue) Signature() ([]Arg, []Arg) {
opt: true,
args: []Arg{
ArgInt{val: &s.limit},
ArgString{val: &s.slimit, validator: []Validator{IsNumberString}},
ArgString{}, // Allow user to specify 'INF' as value. if so, will fall back to maxInt
},
},
},
@@ -43,12 +41,6 @@ func (s *FuncKeepLastValue) Exec(cache map[Req][]models.Series) ([]models.Series
return nil, err
}
limit := int(s.limit)
if s.slimit != "" {
limitf, _ := strconv.ParseFloat(s.slimit, 64)
if !math.IsInf(limitf, 0) {
limit = int(limitf)
}
}
outSeries := make([]models.Series, len(series))
for i, serie := range series {
serie.Target = fmt.Sprintf("keepLastValue(%s)", serie.Target)
198 changes: 198 additions & 0 deletions expr/func_keeplastvalue_test.go
Original file line number Diff line number Diff line change
@@ -1 +1,199 @@
package expr

import (
"math"
"strconv"
"testing"

"github.com/grafana/metrictank/api/models"
"github.com/grafana/metrictank/test"
"gopkg.in/raintank/schema.v1"
)

func TestKeepLastValueAll(t *testing.T) {
out := []schema.Point{
{Val: 0, Ts: 10},
{Val: 0, Ts: 20},
{Val: 5.5, Ts: 30},
{Val: 5.5, Ts: 40},
{Val: 5.5, Ts: 50},
{Val: 1234567890, Ts: 60},
}

testKeepLastValue(
"keepAll",
math.MaxInt64,
[]models.Series{
{
Interval: 10,
Target: "a",
Datapoints: getCopy(a),
},
},
[]models.Series{
{
Interval: 10,
Target: "keepLastValue(a)",
Datapoints: out,
},
},
t,
)
}

func TestKeepLastValueNone(t *testing.T) {

testKeepLastValue(
"keepNone",
0,
[]models.Series{
{
Interval: 10,
Target: "sum4a2b",
Datapoints: getCopy(sum4a2b),
},
},
[]models.Series{
{
Interval: 10,
Target: "keepLastValue(sum4a2b)",
Datapoints: getCopy(sum4a2b),
},
},
t,
)
}

func TestKeepLastValueOne(t *testing.T) {
out := []schema.Point{
{Val: 0, Ts: 10},
{Val: math.MaxFloat64, Ts: 20},
{Val: math.MaxFloat64 - 20, Ts: 30},
{Val: math.MaxFloat64 - 20, Ts: 40},
{Val: 1234567890, Ts: 50},
{Val: 1234567890, Ts: 60},
}

testKeepLastValue(
Copy link
Contributor

@Dieterbe Dieterbe Aug 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to feedback in previous PR, using utility functions is good, but if they call t.Fatal internally, it's harder to trace back to the calling function so if a test fails it's hard to tell where it failed.
better for the util function to return an error, and here, check for error and if so call t.Fatal.

not a huge deal though, if you have more important stuff to do, then don't worry about this. but something to keep in mind for the future at least. i don't want you to refactor this if you have other functions or whatever to work on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I didn't have the utility functions when I wrote this (I just rebased), so didn't do that. Will use them in the future though.

"keepOne",
1,
[]models.Series{
{
Interval: 10,
Target: "b",
Datapoints: getCopy(b),
},
{
Interval: 10,
Target: "a",
Datapoints: getCopy(a),
},
},
[]models.Series{
{
Interval: 10,
Target: "keepLastValue(b)",
Datapoints: out,
},
{
Interval: 10,
Target: "keepLastValue(a)",
Datapoints: getCopy(a),
},
},
t,
)
}

func testKeepLastValue(name string, limit int64, in []models.Series, out []models.Series, t *testing.T) {
f := NewKeepLastValue()
f.(*FuncKeepLastValue).in = NewMock(in)
f.(*FuncKeepLastValue).limit = limit
gots, err := f.Exec(make(map[Req][]models.Series))
if err != nil {
t.Fatalf("case %q (%d): err should be nil. got %q", name, limit, err)
}
if len(gots) != len(out) {
t.Fatalf("case %q (%d): isNonNull len output expected %d, got %d", name, limit, len(out), len(gots))
}
for i, g := range gots {
exp := out[i]
if g.Target != exp.Target {
t.Fatalf("case %q (%d): expected target %q, got %q", name, limit, exp.Target, g.Target)
}
if len(g.Datapoints) != len(exp.Datapoints) {
t.Fatalf("case %q (%d) len output expected %d, got %d", name, limit, len(exp.Datapoints), len(g.Datapoints))
}
for j, p := range g.Datapoints {
bothNaN := math.IsNaN(p.Val) && math.IsNaN(exp.Datapoints[j].Val)
if (bothNaN || p.Val == exp.Datapoints[j].Val) && p.Ts == exp.Datapoints[j].Ts {
continue
}
t.Fatalf("case %q (%d): output point %d - expected %v got %v", name, limit, j, exp.Datapoints[j], p)
}
}
}

func BenchmarkKeepLastValue10k_1NoNulls(b *testing.B) {
benchmarkKeepLastValue(b, 1, test.RandFloats10k, test.RandFloats10k)
}
func BenchmarkKeepLastValue10k_10NoNulls(b *testing.B) {
benchmarkKeepLastValue(b, 10, test.RandFloats10k, test.RandFloats10k)
}
func BenchmarkKeepLastValue10k_100NoNulls(b *testing.B) {
benchmarkKeepLastValue(b, 100, test.RandFloats10k, test.RandFloats10k)
}
func BenchmarkKeepLastValue10k_1000NoNulls(b *testing.B) {
benchmarkKeepLastValue(b, 1000, test.RandFloats10k, test.RandFloats10k)
}

func BenchmarkKeepLastValue10k_1SomeSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 1, test.RandFloats10k, test.RandFloatsWithNulls10k)
}
func BenchmarkKeepLastValue10k_10SomeSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 10, test.RandFloats10k, test.RandFloatsWithNulls10k)
}
func BenchmarkKeepLastValue10k_100SomeSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 100, test.RandFloats10k, test.RandFloatsWithNulls10k)
}
func BenchmarkKeepLastValue10k_1000SomeSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 1000, test.RandFloats10k, test.RandFloatsWithNulls10k)
}

func BenchmarkKeepLastValue10k_1AllSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 1, test.RandFloatsWithNulls10k, test.RandFloatsWithNulls10k)
}
func BenchmarkKeepLastValue10k_10AllSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 10, test.RandFloatsWithNulls10k, test.RandFloatsWithNulls10k)
}
func BenchmarkKeepLastValue10k_100AllSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 100, test.RandFloatsWithNulls10k, test.RandFloatsWithNulls10k)
}
func BenchmarkKeepLastValue10k_1000AllSeriesHalfNulls(b *testing.B) {
benchmarkKeepLastValue(b, 1000, test.RandFloatsWithNulls10k, test.RandFloatsWithNulls10k)
}

func benchmarkKeepLastValue(b *testing.B, numSeries int, fn0, fn1 func() []schema.Point) {
var input []models.Series
for i := 0; i < numSeries; i++ {
series := models.Series{
QueryPatt: strconv.Itoa(i),
}
if i%2 == 0 {
series.Datapoints = fn0()
} else {
series.Datapoints = fn1()
}
input = append(input, series)
}
b.ResetTimer()
for i := 0; i < b.N; i++ {
f := NewKeepLastValue()
f.(*FuncKeepLastValue).in = NewMock(input)
got, err := f.Exec(make(map[Req][]models.Series))
if err != nil {
b.Fatalf("%s", err)
}
results = got
}
}
6 changes: 0 additions & 6 deletions expr/validator.go
Original file line number Diff line number Diff line change
@@ -2,7 +2,6 @@ package expr

import (
"errors"
"strconv"

"github.com/grafana/metrictank/consolidation"
"github.com/raintank/dur"
@@ -45,8 +44,3 @@ func IsOperator(e *expr) error {
}
return errors.New("Unsupported operator: " + e.str)
}

func IsNumberString(e *expr) error {
_, err := strconv.ParseFloat("INF", 64)
return err
}