From 4e901d7cc780e1d7dfc414585c51fad5f01be11b Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Wed, 26 Aug 2020 21:13:48 -0700 Subject: [PATCH 01/23] Added the moving movingMin function --- .../graphite/native/builtin_functions_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 6b76608f90..ace67a79a8 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2474,6 +2474,24 @@ func testMovingMedian(t *testing.T) { []common.TestSeries{expected}, res.Values) } +func TestMovingMin(t *testing.T) { + // create test context + now := time.Now() + engine := NewEngine( + testStorage, + ) + startTime := now.Add(-3 * time.Minute) + endTime := now.Add(-time.Minute) + ctx := common.NewContext(common.ContextOptions{Start: startTime, End: endTime, Engine: engine}) + defer ctx.Close() + + + vals := []float64{1.0, 2.0, 3.0, 4.0, math.NaN()} + expected := common.TestSeries{Name: "foo (avg: 2.500)", Data: vals} + + common.CompareOutputsAndExpected(t, 10000, testMovingAverageStart, expected, res.Values) +} + func TestLegendValue(t *testing.T) { ctx := common.NewTestContext() defer ctx.Close() From f0df4ce2d1ab81420b814b7d8c285ca35dcd7dd9 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Thu, 27 Aug 2020 09:36:32 -0700 Subject: [PATCH 02/23] worked on moving min --- .../graphite/native/builtin_functions.go | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index e10ba3c856..fb131ccc70 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1632,6 +1632,33 @@ func movingMedian(ctx *common.Context, _ singlePathSpec, windowSize string) (*bi return original, nil } + return &binaryContextShifter{ + ContextShiftFunc: contextShiftingFn, + BinaryTransformer: transformerFn, + }, nil +} + +func movingMin(ctx *common.Context, _ singlePathSpec, windowSize string) (*binaryContextShifter, error) { + interval, err := common.ParseInterval(windowSize) + if err != nil { + return nil, err + } + if interval <= 0 { + return nil, common.ErrInvalidIntervalFormat + } + + contextShiftingFn := func(c *common.Context) *common.Context { + opts := common.NewChildContextOptions() + opts.AdjustTimeRange(0, 0, interval, 0) + childCtx := c.NewChildContext(opts) + return childCtx + } + + bootstrapStartTime, bootstrapEndTime := ctx.StartTime.Add(-interval), ctx.StartTime + + + + return &binaryContextShifter{ ContextShiftFunc: contextShiftingFn, BinaryTransformer: transformerFn, From 8ce6dc3fd7471e08456baf216fa12bc682d8ce9d Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Thu, 27 Aug 2020 16:53:26 -0700 Subject: [PATCH 03/23] more work on the moving min func --- src/query/graphite/common/context.go | 1 + src/query/graphite/common/percentiles.go | 10 ++++++++++ src/query/graphite/native/builtin_functions.go | 7 +------ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/query/graphite/common/context.go b/src/query/graphite/common/context.go index 6e4d5ba09a..9a873b62cb 100644 --- a/src/query/graphite/common/context.go +++ b/src/query/graphite/common/context.go @@ -68,6 +68,7 @@ type Context struct { contextBase } + // ContextOptions provides the options to create the context with type ContextOptions struct { Start time.Time diff --git a/src/query/graphite/common/percentiles.go b/src/query/graphite/common/percentiles.go index 1a526fefde..e5ef611a71 100644 --- a/src/query/graphite/common/percentiles.go +++ b/src/query/graphite/common/percentiles.go @@ -71,6 +71,16 @@ func SafeSort(input []float64) int { return nans } +func SafeSum(input []float64) float64 { + sum := 0.0 + for _, v := range input { + if !math.IsNaN(v) { + sum += v + } + } + return sum +} + // GetPercentile computes the percentile cut off for an array of floats func GetPercentile(input []float64, percentile float64, interpolate bool) float64 { nans := SafeSort(input) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 221369a922..0c11ee774c 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1636,12 +1636,7 @@ func movingMedian(ctx *common.Context, _ singlePathSpec, windowSize string) (*bi window[idx] = bootstrap.ValueAt(j) } - nans := common.SafeSort(window) - if nans < windowPoints { - index := (windowPoints - nans) / 2 - median := window[nans+index] - vals.SetValueAt(i, median) - } + vals.SetValueAt(i, common.SafeSum(window)) } name := fmt.Sprintf("movingMedian(%s,%q)", series.Name(), windowSize) newSeries := ts.NewSeries(ctx, name, series.StartTime(), vals) From cf9288f774c37a39e18494596884215095434b9e Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 28 Aug 2020 03:54:12 -0700 Subject: [PATCH 04/23] updated movingMedian --- .../graphite/native/builtin_functions.go | 62 ++++++++++++++++++- .../graphite/native/builtin_functions_test.go | 5 ++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 0c11ee774c..f309c5ba47 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1609,6 +1609,7 @@ func movingMedian(ctx *common.Context, _ singlePathSpec, windowSize string) (*bi } results := make([]*ts.Series, 0, original.Len()) + for i, bootstrap := range bootstrapList.Values { series := original.Values[i] windowPoints := int(interval / (time.Duration(series.MillisPerStep()) * time.Millisecond)) @@ -1636,7 +1637,12 @@ func movingMedian(ctx *common.Context, _ singlePathSpec, windowSize string) (*bi window[idx] = bootstrap.ValueAt(j) } - vals.SetValueAt(i, common.SafeSum(window)) + nans := common.SafeSort(window) + if nans < windowPoints { + index := (windowPoints - nans) / 2 + median := window[nans+index] + vals.SetValueAt(i, median) + } } name := fmt.Sprintf("movingMedian(%s,%q)", series.Name(), windowSize) newSeries := ts.NewSeries(ctx, name, series.StartTime(), vals) @@ -1653,11 +1659,16 @@ func movingMedian(ctx *common.Context, _ singlePathSpec, windowSize string) (*bi }, nil } -func movingMin(ctx *common.Context, _ singlePathSpec, windowSize string) (*binaryContextShifter, error) { +// movingSum takes one metric or a wildcard seriesList followed by a a quoted string +// with a length of time like '1hour' or '5min'. Graphs the sum of the preceding +// datapoints for each point on the graph. All previous datapoints are set to None at +// the beginning of the graph. +func movingSum(ctx *common.Context, _ singlePathSpec, windowSize string) (*binaryContextShifter, error) { interval, err := common.ParseInterval(windowSize) if err != nil { return nil, err } + if interval <= 0 { return nil, common.ErrInvalidIntervalFormat } @@ -1670,9 +1681,53 @@ func movingMin(ctx *common.Context, _ singlePathSpec, windowSize string) (*binar } bootstrapStartTime, bootstrapEndTime := ctx.StartTime.Add(-interval), ctx.StartTime - + transformerFn := func(bootstrapped, original ts.SeriesList) (ts.SeriesList, error) { + bootstrapList, err := combineBootstrapWithOriginal(ctx, + bootstrapStartTime, bootstrapEndTime, + bootstrapped, singlePathSpec(original)) + if err != nil { + return ts.NewSeriesList(), err + } + + results := make([]*ts.Series, 0, original.Len()) + + for i, bootstrap := range bootstrapList.Values { + series := original.Values[i] + windowPoints := int(interval / (time.Duration(series.MillisPerStep()) * time.Millisecond)) + if windowPoints <= 0 { + err := errors.NewInvalidParamsError(fmt.Errorf( + "non positive window points, windowSize=%s, stepSize=%d", + windowSize, series.MillisPerStep())) + return ts.NewSeriesList(), err + } + window := make([]float64, windowPoints) + util.Memset(window, math.NaN()) + numSteps := series.Len() + offset := bootstrap.Len() - numSteps + vals := ts.NewValues(ctx, series.MillisPerStep(), numSteps) + for i := 0; i < numSteps; i++ { + for j := i + offset - windowPoints; j < i+offset; j++ { + if j < 0 || j >= bootstrap.Len() { + continue + } + + idx := j - i - offset + windowPoints + if idx < 0 || idx > len(window)-1 { + continue + } + window[idx] = bootstrap.ValueAt(j) + } + vals.SetValueAt(i, common.SafeSum(window)) + } + name := fmt.Sprintf("movingMedian(%s,%q)", series.Name(), windowSize) + newSeries := ts.NewSeries(ctx, name, series.StartTime(), vals) + results = append(results, newSeries) + } + original.Values = results + return original, nil + } return &binaryContextShifter{ ContextShiftFunc: contextShiftingFn, @@ -1680,6 +1735,7 @@ func movingMin(ctx *common.Context, _ singlePathSpec, windowSize string) (*binar }, nil } + // legendValue takes one metric or a wildcard seriesList and a string in quotes. // Appends a value to the metric name in the legend. Currently one or several of: // "last", "avg", "total", "min", "max". diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 6a68bf59be..4d40e712a5 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -646,7 +646,10 @@ func TestMovingAverageSuccess(t *testing.T) { values := []float64{12.0, 19.0, -10.0, math.NaN(), 10.0} bootstrap := []float64{3.0, 4.0, 5.0} expected := []float64{4.0, 7.0, 12.0, 7.0, 4.5} + expectedMovingSum := []float64{4.0, 7.0, 12.0, 7.0, 4.5} + testMovingAverage(t, "movingAverage(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrap, expected) + testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,\"30s\")", values, bootstrap, expectedMovingSum) testMovingAverage(t, "movingAverage(foo.bar.baz, 3)", "movingAverage(foo.bar.baz,3)", values, bootstrap, expected) testMovingAverage(t, "movingAverage(foo.bar.baz, 3)", "movingAverage(foo.bar.baz,3)", nil, nil, nil) @@ -2453,6 +2456,8 @@ func TestChanged(t *testing.T) { expected, results.Values) } + + func TestMovingMedian(t *testing.T) { ctrl := xgomock.NewController(t) defer ctrl.Finish() From 1ebe6291aa10f613102830fcc554d898a1f039dc Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Fri, 28 Aug 2020 03:55:39 -0700 Subject: [PATCH 05/23] Apply suggestions from code review --- src/query/graphite/common/context.go | 1 - src/query/graphite/native/builtin_functions.go | 1 - src/query/graphite/native/builtin_functions_test.go | 2 -- 3 files changed, 4 deletions(-) diff --git a/src/query/graphite/common/context.go b/src/query/graphite/common/context.go index 9a873b62cb..6e4d5ba09a 100644 --- a/src/query/graphite/common/context.go +++ b/src/query/graphite/common/context.go @@ -68,7 +68,6 @@ type Context struct { contextBase } - // ContextOptions provides the options to create the context with type ContextOptions struct { Start time.Time diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index f309c5ba47..a5cda55041 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1609,7 +1609,6 @@ func movingMedian(ctx *common.Context, _ singlePathSpec, windowSize string) (*bi } results := make([]*ts.Series, 0, original.Len()) - for i, bootstrap := range bootstrapList.Values { series := original.Values[i] windowPoints := int(interval / (time.Duration(series.MillisPerStep()) * time.Millisecond)) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 4d40e712a5..0a0c2c65a0 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2456,8 +2456,6 @@ func TestChanged(t *testing.T) { expected, results.Values) } - - func TestMovingMedian(t *testing.T) { ctrl := xgomock.NewController(t) defer ctrl.Finish() From ebaafce11c0e294c2f0a85f95ca02b6c2a6f3aca Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 28 Aug 2020 04:08:16 -0700 Subject: [PATCH 06/23] testMovingFunction --- src/query/graphite/native/builtin_functions.go | 3 ++- src/query/graphite/native/builtin_functions_test.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index a5cda55041..db4af512f0 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1719,7 +1719,7 @@ func movingSum(ctx *common.Context, _ singlePathSpec, windowSize string) (*binar } vals.SetValueAt(i, common.SafeSum(window)) } - name := fmt.Sprintf("movingMedian(%s,%q)", series.Name(), windowSize) + name := fmt.Sprintf("movingSum(%s,%q)", series.Name(), windowSize) newSeries := ts.NewSeries(ctx, name, series.StartTime(), vals) results = append(results, newSeries) } @@ -1983,6 +1983,7 @@ func init() { MustRegisterFunction(mostDeviant) MustRegisterFunction(movingAverage) MustRegisterFunction(movingMedian) + MustRegisterFunction(movingSum) MustRegisterFunction(multiplySeries) MustRegisterFunction(nonNegativeDerivative).WithDefaultParams(map[uint8]interface{}{ 2: math.NaN(), // maxValue diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 0a0c2c65a0..67044b2e4e 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -646,7 +646,7 @@ func TestMovingAverageSuccess(t *testing.T) { values := []float64{12.0, 19.0, -10.0, math.NaN(), 10.0} bootstrap := []float64{3.0, 4.0, 5.0} expected := []float64{4.0, 7.0, 12.0, 7.0, 4.5} - expectedMovingSum := []float64{4.0, 7.0, 12.0, 7.0, 4.5} + expectedMovingSum := []float64{12.0, 21.0, 36.0, 21.0, 9.0} testMovingAverage(t, "movingAverage(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrap, expected) testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,\"30s\")", values, bootstrap, expectedMovingSum) @@ -2550,7 +2550,7 @@ func TestMovingMedianInvalidLimits(t *testing.T) { func TestMovingMismatchedLimits(t *testing.T) { // NB: this tests the behavior when query limits do not snap exactly to data // points. When limits do not snap exactly, the first point should be omitted. - for _, fn := range []string{"movingAverage", "movingMedian"} { + for _, fn := range []string{"movingAverage", "movingMedian", "movingSum"} { for i := time.Duration(0); i < time.Minute; i += time.Second { testMovingAverageInvalidLimits(t, fn, i) } @@ -2963,6 +2963,7 @@ func TestFunctionsRegistered(t *testing.T) { "mostDeviant", "movingAverage", "movingMedian", + "movingSum", "multiplySeries", "nonNegativeDerivative", "nPercentile", From 2a0643c19bc17c6c815b4cee6093a973973b73ba Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 28 Aug 2020 04:42:50 -0700 Subject: [PATCH 07/23] wrote test movingSum function --- .../graphite/native/builtin_functions_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 67044b2e4e..c934c344f3 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -688,6 +688,23 @@ func TestMovingAverageError(t *testing.T) { testMovingAverageError(t, "movingAverage(foo.bar.baz, 0)") } +func TestMovingSumSuccess(t *testing.T) { + values := []float64{12.0, 19.0, -10.0, math.NaN(), 10.0} + bootstrap := []float64{3.0, 4.0, 5.0} + expected := []float64{12.0, 21.0, 36.0, 21.0, 9.0} // (3+4+5), (4+5+12), (5+12+19), (12+19-10), (19-10+Nan) + + testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrap, expected) + testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,3)", nil, nil, nil) + + bootstrapEntireSeries := []float64{3.0, 4.0, 5.0, 12.0, 19.0, -10.0, math.NaN(), 10.0} + testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrapEntireSeries, expected) +} + +func TestMovingSumError(t *testing.T) { + testMovingAverageError(t, "movingSum(foo.bar.baz, '-30s')") + testMovingAverageError(t, "movingSum(foo.bar.baz, 0)") +} + func TestIsNonNull(t *testing.T) { ctx := common.NewTestContext() defer ctx.Close() From cb088b2fa6b58f53bb5798fae349f21c2c3fa852 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 28 Aug 2020 05:37:25 -0700 Subject: [PATCH 08/23] Added delay function --- .../graphite/native/builtin_functions.go | 1 + .../graphite/native/builtin_functions_test.go | 51 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index e10ba3c856..fe8fc85f49 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -214,6 +214,7 @@ func timeShift( } shift, err := common.ParseInterval(timeShiftS) + if err != nil { return nil, errors.NewInvalidParamsError(fmt.Errorf("invalid timeShift parameter %s: %v", timeShiftS, err)) } diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 6b76608f90..efc35dbd67 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2710,6 +2710,57 @@ func testTimeShift(t *testing.T) { []common.TestSeries{expected}, res.Values) } +func TestDelay(t *testing.T) { + values := [3][6]float64{ + {54.0, 48.0, 92.0, 54.0, 14.0, 1.2}, + {4.0, 5.0, math.NaN(), 6.4, 7.2, math.NaN()}, + {math.NaN(), 8.0, 9.0, 10.6, 11.2, 12.2}, + } + expected := [3][6]float64{ + {math.NaN(), math.NaN(), math.NaN(), 54.0, 48.0, 92.0}, + {math.NaN(), math.NaN(), math.NaN(), 4.0, 5.0, math.NaN()}, + {math.NaN(), math.NaN(), math.NaN(), math.NaN(), 8.0, 9.0}, + } + + for index, value := range values { + e := expected[index] + testDelay(t, "delay(foo.bar.baz, 3)", "delay(foo.bar.baz,3)", value, e) + + } +} + +func testDelay(t *testing.T, target, expectedName string, values, output []float64) { + ctx := common.NewTestContext() + defer ctx.Close() + + engine := NewEngine( + &common.MovingAverageStorage{ + StepMillis: 10000, + Values: values, + }, + ) + phonyContext := common.NewContext(common.ContextOptions{ + Start: testMovingAverageStart, + End: testMovingAverageEnd, + Engine: engine, + }) + + expr, err := phonyContext.Engine.(*Engine).Compile(target) + require.NoError(t, err) + res, err := expr.Execute(phonyContext) + require.NoError(t, err) + var expected []common.TestSeries + if output != nil { + expectedSeries := common.TestSeries{ + Name: expectedName, + Data: output, + } + expected = append(expected, expectedSeries) + } + common.CompareOutputsAndExpected(t, 10000, testMovingAverageStart, + expected, res.Values) +} + func TestDashed(t *testing.T) { ctx := common.NewTestContext() defer ctx.Close() From 2e183a55f91890695105ad34a71901fa4510680f Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 28 Aug 2020 11:09:30 -0700 Subject: [PATCH 09/23] built out more of the core logic for delay function --- .../graphite/native/builtin_functions.go | 39 ++++++++++++++++++- .../graphite/native/builtin_functions_test.go | 23 ++++++++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index fe8fc85f49..4aca29b2d4 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -198,7 +198,6 @@ func limit(_ *common.Context, series singlePathSpec, n int) (ts.SeriesList, erro return r, nil } -// timeShift draws the selected metrics shifted in time. If no sign is given, a minus sign ( - ) is // implied which will shift the metric back in time. If a plus sign ( + ) is given, the metric will // be shifted forward in time func timeShift( @@ -242,6 +241,43 @@ func timeShift( }, nil } +// implied which will shift the metric back in time. If a plus sign ( + ) is given, the metric will +// be shifted forward in time +func delay( + ctx *common.Context, + _ singlePathSpec, + steps int, +) (*unaryContextShifter, error) { + + contextShiftingFn := func(c *common.Context) *common.Context { + opts := common.NewChildContextOptions() + opts.AdjustTimeRange(0, 0, 0, 0) + childCtx := c.NewChildContext(opts) + return childCtx + } + + transformerFn := func(input ts.SeriesList) (ts.SeriesList, error) { + output := make([]*ts.Series, input.Len()) + + for i, series := range input.Values { + + delayedVals := delayValues(series, steps) + delayedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), delayedVals) + output[i] = delayedSeries + } + input.Values = output + return input, nil + } + + return &unaryContextShifter{ + ContextShiftFunc: contextShiftingFn, + UnaryTransformer: transformerFn, + }, nil +} + +func delayValues(vals ts.Values, steps int ) (ts.Values) { + +} // absolute returns the absolute value of each element in the series. func absolute(ctx *common.Context, input singlePathSpec) (ts.SeriesList, error) { return transform(ctx, input, @@ -1853,6 +1889,7 @@ func init() { MustRegisterFunction(dashed).WithDefaultParams(map[uint8]interface{}{ 2: 5.0, // dashLength }) + MustRegisterFunction(delay) MustRegisterFunction(derivative) MustRegisterFunction(diffSeries) MustRegisterFunction(divideSeries) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index efc35dbd67..505dfda92f 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -662,6 +662,8 @@ func testMovingAverageError(t *testing.T, target string) { Bootstrap: []float64{1.0}, BootstrapStart: testMovingAverageBootstrap, Values: []float64{1.0}, + Type: Type: common.MovingFunctionStorageType, + }, ) phonyContext := common.NewContext(common.ContextOptions{ @@ -2711,12 +2713,12 @@ func testTimeShift(t *testing.T) { } func TestDelay(t *testing.T) { - values := [3][6]float64{ + var values = [3][]float64{ {54.0, 48.0, 92.0, 54.0, 14.0, 1.2}, {4.0, 5.0, math.NaN(), 6.4, 7.2, math.NaN()}, {math.NaN(), 8.0, 9.0, 10.6, 11.2, 12.2}, } - expected := [3][6]float64{ + expected := [3][]float64{ {math.NaN(), math.NaN(), math.NaN(), 54.0, 48.0, 92.0}, {math.NaN(), math.NaN(), math.NaN(), 4.0, 5.0, math.NaN()}, {math.NaN(), math.NaN(), math.NaN(), math.NaN(), 8.0, 9.0}, @@ -2725,23 +2727,31 @@ func TestDelay(t *testing.T) { for index, value := range values { e := expected[index] testDelay(t, "delay(foo.bar.baz, 3)", "delay(foo.bar.baz,3)", value, e) - } } +var ( + testDelayBootstrap = testMovingAverageStart.Add(-30 * time.Second) + testDelayStart = time.Now().Truncate(time.Minute) + testDelayEnd = testMovingAverageStart.Add(time.Minute) +) + func testDelay(t *testing.T, target, expectedName string, values, output []float64) { ctx := common.NewTestContext() defer ctx.Close() + emptyBootstrap := []float64{} engine := NewEngine( &common.MovingAverageStorage{ StepMillis: 10000, Values: values, + Bootstrap: emptyBootstrap, + BootstrapStart: testDelayBootstrap, }, ) phonyContext := common.NewContext(common.ContextOptions{ - Start: testMovingAverageStart, - End: testMovingAverageEnd, + Start: testDelayStart, + End: testDelayEnd, Engine: engine, }) @@ -2757,7 +2767,7 @@ func testDelay(t *testing.T, target, expectedName string, values, output []float } expected = append(expected, expectedSeries) } - common.CompareOutputsAndExpected(t, 10000, testMovingAverageStart, + common.CompareOutputsAndExpected(t, 10000, testDelayStart, expected, res.Values) } @@ -2848,6 +2858,7 @@ func TestFunctionsRegistered(t *testing.T) { "currentAbove", "currentBelow", "dashed", + "delay", "derivative", "diffSeries", "divideSeries", From dbe7b33d072ebe9feefc875f1468d1ecaa3c922d Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 28 Aug 2020 12:21:52 -0700 Subject: [PATCH 10/23] fixed up delay test --- src/query/graphite/native/builtin_functions.go | 15 +++++++++++---- .../graphite/native/builtin_functions_test.go | 3 +-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 4aca29b2d4..560471360d 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -261,9 +261,11 @@ func delay( for i, series := range input.Values { - delayedVals := delayValues(series, steps) + delayedVals := delayValues(ctx, *series, steps) delayedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), delayedVals) - output[i] = delayedSeries + renamedSeries := delayedSeries.RenamedTo(fmt.Sprintf("delay(%s, %s)", delayedSeries.Name(), steps)) + output[i] = renamedSeries + } input.Values = output return input, nil @@ -275,9 +277,14 @@ func delay( }, nil } -func delayValues(vals ts.Values, steps int ) (ts.Values) { - +func delayValues(ctx *common.Context, series ts.Series, steps int ) (ts.Values) { + output := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) + for i := steps; i < series.Len(); i++ { + output.SetValueAt(i, series.ValueAt(i - steps)) + } + return output } + // absolute returns the absolute value of each element in the series. func absolute(ctx *common.Context, input singlePathSpec) (ts.SeriesList, error) { return transform(ctx, input, diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 505dfda92f..ddd7b1892f 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -662,8 +662,6 @@ func testMovingAverageError(t *testing.T, target string) { Bootstrap: []float64{1.0}, BootstrapStart: testMovingAverageBootstrap, Values: []float64{1.0}, - Type: Type: common.MovingFunctionStorageType, - }, ) phonyContext := common.NewContext(common.ContextOptions{ @@ -2760,6 +2758,7 @@ func testDelay(t *testing.T, target, expectedName string, values, output []float res, err := expr.Execute(phonyContext) require.NoError(t, err) var expected []common.TestSeries + if output != nil { expectedSeries := common.TestSeries{ Name: expectedName, From 8c2b6bffb99f8a67d091c9aff3b5ec66dce99930 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Mon, 31 Aug 2020 08:10:58 -0700 Subject: [PATCH 11/23] updated tests --- .../graphite/native/builtin_functions_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 26f5a8650c..0689bc1628 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -690,16 +690,16 @@ func TestMovingSumSuccess(t *testing.T) { bootstrap := []float64{3.0, 4.0, 5.0} expected := []float64{12.0, 21.0, 36.0, 21.0, 9.0} // (3+4+5), (4+5+12), (5+12+19), (12+19-10), (19-10+Nan) - testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrap, expected) - testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,3)", nil, nil, nil) + testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrap, expected) + testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,3)", nil, nil, nil) bootstrapEntireSeries := []float64{3.0, 4.0, 5.0, 12.0, 19.0, -10.0, math.NaN(), 10.0} - testMovingAverage(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrapEntireSeries, expected) + testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrapEntireSeries, expected) } func TestMovingSumError(t *testing.T) { - testMovingAverageError(t, "movingSum(foo.bar.baz, '-30s')") - testMovingAverageError(t, "movingSum(foo.bar.baz, 0)") + testMovingFunctionError(t, "movingSum(foo.bar.baz, '-30s')") + testMovingFunctionError(t, "movingSum(foo.bar.baz, 0)") } func TestIsNonNull(t *testing.T) { @@ -2566,12 +2566,12 @@ func TestMovingMismatchedLimits(t *testing.T) { // points. When limits do not snap exactly, the first point should be omitted. for _, fn := range []string{"movingAverage", "movingMedian", "movingSum"} { for i := time.Duration(0); i < time.Minute; i += time.Second { - testMovingAverageInvalidLimits(t, fn, i) + testMovingFunctionInvalidLimits(t, fn, i) } } } -func testMovingAverageInvalidLimits(t *testing.T, fn string, offset time.Duration) { +func testMovingFunctionInvalidLimits(t *testing.T, fn string, offset time.Duration) { ctrl := xgomock.NewController(t) defer ctrl.Finish() From 01792e7f4bdced831442d884c7adcd46e84cf1f8 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Mon, 31 Aug 2020 09:29:53 -0700 Subject: [PATCH 12/23] Fixed up tests --- src/query/graphite/native/builtin_functions_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 0689bc1628..c597b291b0 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -690,16 +690,15 @@ func TestMovingSumSuccess(t *testing.T) { bootstrap := []float64{3.0, 4.0, 5.0} expected := []float64{12.0, 21.0, 36.0, 21.0, 9.0} // (3+4+5), (4+5+12), (5+12+19), (12+19-10), (19-10+Nan) - testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrap, expected) - testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,3)", nil, nil, nil) + testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,\"30s\")", values, bootstrap, expected) + testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,3)", nil, nil, nil) bootstrapEntireSeries := []float64{3.0, 4.0, 5.0, 12.0, 19.0, -10.0, math.NaN(), 10.0} - testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingAverage(foo.bar.baz,\"30s\")", values, bootstrapEntireSeries, expected) + testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,\"30s\")", values, bootstrapEntireSeries, expected) } func TestMovingSumError(t *testing.T) { testMovingFunctionError(t, "movingSum(foo.bar.baz, '-30s')") - testMovingFunctionError(t, "movingSum(foo.bar.baz, 0)") } func TestIsNonNull(t *testing.T) { From 29a032dda1261a26fa0eb7535e6439442a85c73d Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Mon, 31 Aug 2020 09:39:59 -0700 Subject: [PATCH 13/23] Update percentiles.go --- src/query/graphite/common/percentiles.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/query/graphite/common/percentiles.go b/src/query/graphite/common/percentiles.go index e5ef611a71..1a526fefde 100644 --- a/src/query/graphite/common/percentiles.go +++ b/src/query/graphite/common/percentiles.go @@ -71,16 +71,6 @@ func SafeSort(input []float64) int { return nans } -func SafeSum(input []float64) float64 { - sum := 0.0 - for _, v := range input { - if !math.IsNaN(v) { - sum += v - } - } - return sum -} - // GetPercentile computes the percentile cut off for an array of floats func GetPercentile(input []float64, percentile float64, interpolate bool) float64 { nans := SafeSort(input) From dba69f413e523ef67c81684809a4d7536969a32a Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Mon, 31 Aug 2020 09:40:34 -0700 Subject: [PATCH 14/23] Apply suggestions from code review --- src/query/graphite/native/builtin_functions.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index a8080ee412..8e26122d59 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -214,7 +214,6 @@ func timeShift( } shift, err := common.ParseInterval(timeShiftS) - if err != nil { return nil, errors.NewInvalidParamsError(fmt.Errorf("invalid timeShift parameter %s: %v", timeShiftS, err)) } From 117e24fa0689d13ef2951f4ca8b0af66b035d5fb Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Mon, 31 Aug 2020 15:10:13 -0400 Subject: [PATCH 15/23] Update builtin_functions.go --- .../graphite/native/builtin_functions.go | 77 ------------------- 1 file changed, 77 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 8e26122d59..8aa9929007 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1701,83 +1701,6 @@ func movingMedian(ctx *common.Context, _ singlePathSpec, windowSize string) (*bi }, nil } -// movingSum takes one metric or a wildcard seriesList followed by a a quoted string -// with a length of time like '1hour' or '5min'. Graphs the sum of the preceding -// datapoints for each point on the graph. All previous datapoints are set to None at -// the beginning of the graph. -func movingSum(ctx *common.Context, _ singlePathSpec, windowSize string) (*binaryContextShifter, error) { - interval, err := common.ParseInterval(windowSize) - if err != nil { - return nil, err - } - - if interval <= 0 { - return nil, common.ErrInvalidIntervalFormat - } - - contextShiftingFn := func(c *common.Context) *common.Context { - opts := common.NewChildContextOptions() - opts.AdjustTimeRange(0, 0, interval, 0) - childCtx := c.NewChildContext(opts) - return childCtx - } - - bootstrapStartTime, bootstrapEndTime := ctx.StartTime.Add(-interval), ctx.StartTime - transformerFn := func(bootstrapped, original ts.SeriesList) (ts.SeriesList, error) { - bootstrapList, err := combineBootstrapWithOriginal(ctx, - bootstrapStartTime, bootstrapEndTime, - bootstrapped, singlePathSpec(original)) - if err != nil { - return ts.NewSeriesList(), err - } - - results := make([]*ts.Series, 0, original.Len()) - - for i, bootstrap := range bootstrapList.Values { - series := original.Values[i] - windowPoints := int(interval / (time.Duration(series.MillisPerStep()) * time.Millisecond)) - if windowPoints <= 0 { - err := errors.NewInvalidParamsError(fmt.Errorf( - "non positive window points, windowSize=%s, stepSize=%d", - windowSize, series.MillisPerStep())) - return ts.NewSeriesList(), err - } - window := make([]float64, windowPoints) - util.Memset(window, math.NaN()) - numSteps := series.Len() - offset := bootstrap.Len() - numSteps - vals := ts.NewValues(ctx, series.MillisPerStep(), numSteps) - for i := 0; i < numSteps; i++ { - for j := i + offset - windowPoints; j < i+offset; j++ { - if j < 0 || j >= bootstrap.Len() { - continue - } - - idx := j - i - offset + windowPoints - if idx < 0 || idx > len(window)-1 { - continue - } - - window[idx] = bootstrap.ValueAt(j) - } - vals.SetValueAt(i, common.SafeSum(window)) - } - name := fmt.Sprintf("movingSum(%s,%q)", series.Name(), windowSize) - newSeries := ts.NewSeries(ctx, name, series.StartTime(), vals) - results = append(results, newSeries) - } - - original.Values = results - return original, nil - } - - return &binaryContextShifter{ - ContextShiftFunc: contextShiftingFn, - BinaryTransformer: transformerFn, - }, nil -} - - // legendValue takes one metric or a wildcard seriesList and a string in quotes. // Appends a value to the metric name in the legend. Currently one or several of: // "last", "avg", "total", "min", "max". From a3b7abba82fefc0ffb5a1e211a1e6f1b3926f035 Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Mon, 31 Aug 2020 15:12:20 -0400 Subject: [PATCH 16/23] Apply suggestions from code review --- .../graphite/native/builtin_functions.go | 1 - .../graphite/native/builtin_functions_test.go | 23 +++---------------- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 8aa9929007..d156feee7e 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1950,7 +1950,6 @@ func init() { MustRegisterFunction(mostDeviant) MustRegisterFunction(movingAverage) MustRegisterFunction(movingMedian) - MustRegisterFunction(movingSum) MustRegisterFunction(multiplySeries) MustRegisterFunction(nonNegativeDerivative).WithDefaultParams(map[uint8]interface{}{ 2: math.NaN(), // maxValue diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 4231214eca..3f28525a08 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -685,22 +685,6 @@ func TestMovingAverageError(t *testing.T) { testMovingFunctionError(t, "movingAverage(foo.bar.baz, 0)") } -func TestMovingSumSuccess(t *testing.T) { - values := []float64{12.0, 19.0, -10.0, math.NaN(), 10.0} - bootstrap := []float64{3.0, 4.0, 5.0} - expected := []float64{12.0, 21.0, 36.0, 21.0, 9.0} // (3+4+5), (4+5+12), (5+12+19), (12+19-10), (19-10+Nan) - - testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,\"30s\")", values, bootstrap, expected) - testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,3)", nil, nil, nil) - - bootstrapEntireSeries := []float64{3.0, 4.0, 5.0, 12.0, 19.0, -10.0, math.NaN(), 10.0} - testMovingFunction(t, "movingSum(foo.bar.baz, '30s')", "movingSum(foo.bar.baz,\"30s\")", values, bootstrapEntireSeries, expected) -} - -func TestMovingSumError(t *testing.T) { - testMovingFunctionError(t, "movingSum(foo.bar.baz, '-30s')") -} - func TestIsNonNull(t *testing.T) { ctx := common.NewTestContext() defer ctx.Close() @@ -2563,14 +2547,14 @@ func TestMovingMedianInvalidLimits(t *testing.T) { func TestMovingMismatchedLimits(t *testing.T) { // NB: this tests the behavior when query limits do not snap exactly to data // points. When limits do not snap exactly, the first point should be omitted. - for _, fn := range []string{"movingAverage", "movingMedian", "movingSum"} { + for _, fn := range []string{"movingAverage", "movingMedian"} { for i := time.Duration(0); i < time.Minute; i += time.Second { - testMovingFunctionInvalidLimits(t, fn, i) + testMovingAverageInvalidLimits(t, fn, i) } } } -func testMovingFunctionInvalidLimits(t *testing.T, fn string, offset time.Duration) { +func testMovingAverageInvalidLimits(t *testing.T, fn string, offset time.Duration) { ctrl := xgomock.NewController(t) defer ctrl.Finish() @@ -3037,7 +3021,6 @@ func TestFunctionsRegistered(t *testing.T) { "mostDeviant", "movingAverage", "movingMedian", - "movingSum", "multiplySeries", "nonNegativeDerivative", "nPercentile", From a2ab5aa51508fb515c11d382de323f2353f31d7e Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Mon, 31 Aug 2020 15:13:39 -0400 Subject: [PATCH 17/23] Apply suggestions from code review --- src/query/graphite/native/builtin_functions.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index d156feee7e..0bea7192b7 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -199,6 +199,7 @@ func limit(_ *common.Context, series singlePathSpec, n int) (ts.SeriesList, erro return r, nil } +// timeShift draws the selected metrics shifted in time. If no sign is given, a minus sign ( - ) is // implied which will shift the metric back in time. If a plus sign ( + ) is given, the metric will // be shifted forward in time func timeShift( @@ -241,8 +242,6 @@ func timeShift( }, nil } -// implied which will shift the metric back in time. If a plus sign ( + ) is given, the metric will -// be shifted forward in time func delay( ctx *common.Context, _ singlePathSpec, From b9a5ecceafcecc0b6d76108eadd9ee96d0b97429 Mon Sep 17 00:00:00 2001 From: Theodore Date: Tue, 1 Sep 2020 15:40:14 -0400 Subject: [PATCH 18/23] clean up delay and add comments --- .../graphite/native/builtin_functions.go | 22 +++++++++++-------- .../graphite/native/builtin_functions_test.go | 6 ++--- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 99d3fa0f1d..45dd4e16ba 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -242,6 +242,12 @@ func timeShift( }, nil } + +// delay shifts all samples later by an integer number of steps. This can be used +// for custom derivative calculations, among other things. Note: this will pad +// the early end of the data with NaN for every step shifted. delay complements +// other time-displacement functions such as timeShift and timeSlice, in that +// delay is indifferent about the step intervals being shifted. func delay( ctx *common.Context, _ singlePathSpec, @@ -249,22 +255,18 @@ func delay( ) (*unaryContextShifter, error) { contextShiftingFn := func(c *common.Context) *common.Context { - opts := common.NewChildContextOptions() - opts.AdjustTimeRange(0, 0, 0, 0) - childCtx := c.NewChildContext(opts) - return childCtx + // no need to shift the context here + return c; } transformerFn := func(input ts.SeriesList) (ts.SeriesList, error) { output := make([]*ts.Series, input.Len()) for i, series := range input.Values { - - delayedVals := delayValues(ctx, *series, steps) + delayedVals := delayValuesHelper(ctx, *series, steps) delayedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), delayedVals) - renamedSeries := delayedSeries.RenamedTo(fmt.Sprintf("delay(%s, %s)", delayedSeries.Name(), steps)) + renamedSeries := delayedSeries.RenamedTo(fmt.Sprintf("delay(%s,%d)", delayedSeries.Name(), steps)) output[i] = renamedSeries - } input.Values = output return input, nil @@ -276,7 +278,9 @@ func delay( }, nil } -func delayValues(ctx *common.Context, series ts.Series, steps int ) (ts.Values) { +// delayValuesHelper takes a series and returns a copy of the values after +// delaying the values by `steps` number of steps +func delayValuesHelper(ctx *common.Context, series ts.Series, steps int ) (ts.Values) { output := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) for i := steps; i < series.Len(); i++ { output.SetValueAt(i, series.ValueAt(i - steps)) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 3f28525a08..8fac166efd 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2859,9 +2859,9 @@ func TestDelay(t *testing.T) { } var ( - testDelayBootstrap = testMovingAverageStart.Add(-30 * time.Second) + testDelayBootstrap = testMovingFunctionStart.Add(-30 * time.Second) testDelayStart = time.Now().Truncate(time.Minute) - testDelayEnd = testMovingAverageStart.Add(time.Minute) + testDelayEnd = testMovingFunctionEnd.Add(time.Minute) ) func testDelay(t *testing.T, target, expectedName string, values, output []float64) { @@ -2870,7 +2870,7 @@ func testDelay(t *testing.T, target, expectedName string, values, output []float emptyBootstrap := []float64{} engine := NewEngine( - &common.MovingAverageStorage{ + &common.MovingFunctionStorage{ StepMillis: 10000, Values: values, Bootstrap: emptyBootstrap, From 9be8bfad80a0378da7a6df056d9e0a578f3df1f7 Mon Sep 17 00:00:00 2001 From: Theodore Date: Wed, 2 Sep 2020 22:05:06 -0400 Subject: [PATCH 19/23] removed contextShift from delay function --- src/query/graphite/native/builtin_functions.go | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 45dd4e16ba..670b659005 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -250,16 +250,10 @@ func timeShift( // delay is indifferent about the step intervals being shifted. func delay( ctx *common.Context, - _ singlePathSpec, + singlePath singlePathSpec, steps int, -) (*unaryContextShifter, error) { - - contextShiftingFn := func(c *common.Context) *common.Context { - // no need to shift the context here - return c; - } - - transformerFn := func(input ts.SeriesList) (ts.SeriesList, error) { +) (ts.SeriesList, error) { + input := ts.SeriesList(singlePath) output := make([]*ts.Series, input.Len()) for i, series := range input.Values { @@ -270,12 +264,6 @@ func delay( } input.Values = output return input, nil - } - - return &unaryContextShifter{ - ContextShiftFunc: contextShiftingFn, - UnaryTransformer: transformerFn, - }, nil } // delayValuesHelper takes a series and returns a copy of the values after From 99852b2936ca72adcfd242e6b1a44f73522dcccb Mon Sep 17 00:00:00 2001 From: Theodore Date: Wed, 2 Sep 2020 22:07:20 -0400 Subject: [PATCH 20/23] updated testdelay function --- src/query/graphite/native/builtin_functions_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 8fac166efd..ea57d268c2 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2859,7 +2859,6 @@ func TestDelay(t *testing.T) { } var ( - testDelayBootstrap = testMovingFunctionStart.Add(-30 * time.Second) testDelayStart = time.Now().Truncate(time.Minute) testDelayEnd = testMovingFunctionEnd.Add(time.Minute) ) @@ -2868,13 +2867,10 @@ func testDelay(t *testing.T, target, expectedName string, values, output []float ctx := common.NewTestContext() defer ctx.Close() - emptyBootstrap := []float64{} engine := NewEngine( &common.MovingFunctionStorage{ StepMillis: 10000, Values: values, - Bootstrap: emptyBootstrap, - BootstrapStart: testDelayBootstrap, }, ) phonyContext := common.NewContext(common.ContextOptions{ @@ -2896,8 +2892,7 @@ func testDelay(t *testing.T, target, expectedName string, values, output []float } expected = append(expected, expectedSeries) } - common.CompareOutputsAndExpected(t, 10000, testDelayStart, - expected, res.Values) + common.CompareOutputsAndExpected(t, 10000, testDelayStart, expected, res.Values) } func TestDashed(t *testing.T) { From f5e30d3f4e1274bb4ac8a549c88ff5238d523284 Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Fri, 4 Sep 2020 13:02:11 -0400 Subject: [PATCH 21/23] Update src/query/graphite/native/builtin_functions.go Co-authored-by: Rob Skillington --- .../graphite/native/builtin_functions.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 479c39e287..e5cda21477 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -253,17 +253,17 @@ func delay( singlePath singlePathSpec, steps int, ) (ts.SeriesList, error) { - input := ts.SeriesList(singlePath) - output := make([]*ts.Series, input.Len()) + input := ts.SeriesList(singlePath) + output := make([]*ts.Series, input.Len()) - for i, series := range input.Values { - delayedVals := delayValuesHelper(ctx, *series, steps) - delayedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), delayedVals) - renamedSeries := delayedSeries.RenamedTo(fmt.Sprintf("delay(%s,%d)", delayedSeries.Name(), steps)) - output[i] = renamedSeries - } - input.Values = output - return input, nil + for i, series := range input.Values { + delayedVals := delayValuesHelper(ctx, *series, steps) + delayedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), delayedVals) + renamedSeries := delayedSeries.RenamedTo(fmt.Sprintf("delay(%s,%d)", delayedSeries.Name(), steps)) + output[i] = renamedSeries + } + input.Values = output + return input, nil } // delayValuesHelper takes a series and returns a copy of the values after From 1d5443939be769cb363f9d5bbd823e516c6595a0 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 4 Sep 2020 10:16:52 -0700 Subject: [PATCH 22/23] Updated delay after PR review --- src/query/graphite/native/builtin_functions.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index e5cda21477..207514a3c8 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -254,13 +254,13 @@ func delay( steps int, ) (ts.SeriesList, error) { input := ts.SeriesList(singlePath) - output := make([]*ts.Series, input.Len()) + output := make([]*ts.Series, 0, input.Len()) - for i, series := range input.Values { - delayedVals := delayValuesHelper(ctx, *series, steps) + for _, series := range input.Values { + delayedVals := delayValuesHelper(ctx, series, steps) delayedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), delayedVals) renamedSeries := delayedSeries.RenamedTo(fmt.Sprintf("delay(%s,%d)", delayedSeries.Name(), steps)) - output[i] = renamedSeries + output = append(output, renamedSeries) } input.Values = output return input, nil @@ -268,7 +268,7 @@ func delay( // delayValuesHelper takes a series and returns a copy of the values after // delaying the values by `steps` number of steps -func delayValuesHelper(ctx *common.Context, series ts.Series, steps int ) (ts.Values) { +func delayValuesHelper(ctx *common.Context, series *ts.Series, steps int) ts.Values { output := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) for i := steps; i < series.Len(); i++ { output.SetValueAt(i, series.ValueAt(i - steps)) From 5aa39eea7467d8376a68cc97977e65c559a3a0c2 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Tue, 8 Sep 2020 06:29:38 -0400 Subject: [PATCH 23/23] Update src/query/graphite/native/builtin_functions.go --- src/query/graphite/native/builtin_functions.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index bccda35367..2c89ffa7cc 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -242,7 +242,6 @@ func timeShift( }, nil } - // delay shifts all samples later by an integer number of steps. This can be used // for custom derivative calculations, among other things. Note: this will pad // the early end of the data with NaN for every step shifted. delay complements