From 4e901d7cc780e1d7dfc414585c51fad5f01be11b Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Wed, 26 Aug 2020 21:13:48 -0700 Subject: [PATCH 01/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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 88260f7c5b8428358e831802bb5b3f31d503f787 Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Wed, 2 Sep 2020 20:14:22 -0400 Subject: [PATCH 08/21] Apply suggestions from code review --- src/query/graphite/common/percentiles.go | 10 --- .../graphite/native/builtin_functions.go | 77 ------------------- .../graphite/native/builtin_functions_test.go | 19 +---- 3 files changed, 1 insertion(+), 105 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) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index ef24be393f..ab0569f91d 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1671,83 +1671,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". diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 26f5a8650c..8fad20fd35 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -685,23 +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) - - 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() @@ -2564,7 +2547,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", "movingSum"} { + for _, fn := range []string{"movingAverage", "movingMedian"} { for i := time.Duration(0); i < time.Minute; i += time.Second { testMovingAverageInvalidLimits(t, fn, i) } From 73e279ff2f88f7438bdb6239d9bede3d1ed9ae94 Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Wed, 2 Sep 2020 20:14:30 -0400 Subject: [PATCH 09/21] Apply suggestions from code review --- src/query/graphite/native/builtin_functions_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 8fad20fd35..777b192120 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2960,7 +2960,6 @@ func TestFunctionsRegistered(t *testing.T) { "mostDeviant", "movingAverage", "movingMedian", - "movingSum", "multiplySeries", "nonNegativeDerivative", "nPercentile", From 007e83e39678186f9c09e3af543b1efe3317daf5 Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Wed, 2 Sep 2020 20:14:42 -0400 Subject: [PATCH 10/21] 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 ab0569f91d..89092bd88d 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -1919,7 +1919,6 @@ func init() { MustRegisterFunction(mostDeviant) MustRegisterFunction(movingAverage) MustRegisterFunction(movingMedian) - MustRegisterFunction(movingSum) MustRegisterFunction(multiplySeries) MustRegisterFunction(nonNegativeDerivative).WithDefaultParams(map[uint8]interface{}{ 2: math.NaN(), // maxValue From dafe31b78f2a4884198385fbb82979375ec2873e Mon Sep 17 00:00:00 2001 From: Theodore Date: Wed, 2 Sep 2020 20:15:13 -0400 Subject: [PATCH 11/21] added graphite timeslice function --- .../graphite/native/builtin_functions.go | 54 +++++++++++ .../graphite/native/builtin_functions_test.go | 91 +++++++++++++++++-- 2 files changed, 137 insertions(+), 8 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index db4af512f0..ff4651f56f 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -32,6 +32,7 @@ import ( "github.com/m3db/m3/src/query/graphite/common" "github.com/m3db/m3/src/query/graphite/errors" + "github.com/m3db/m3/src/query/graphite/graphite" "github.com/m3db/m3/src/query/graphite/ts" "github.com/m3db/m3/src/query/util" ) @@ -242,6 +243,56 @@ func timeShift( }, nil } + +func timeSlice(ctx *common.Context, input singlePathSpec, start string, end string) (*unaryContextShifter, error) { + + contextShiftingFn := func(c *common.Context) *common.Context { + // no need to shift the context here + return c; + } + + tzOffsetForAbsoluteTime := time.Duration(0) + startTime, err := graphite.ParseTime(start, time.Now(), tzOffsetForAbsoluteTime) + endTime, err := graphite.ParseTime(end, time.Now(), tzOffsetForAbsoluteTime) + + if (err != nil) { + return nil, err + } + + transformerFn := func(input ts.SeriesList) (ts.SeriesList, error) { + output := make([]*ts.Series, input.Len()) + + for i, series := range input.Values { + nanoSecondsPerStep := series.MillisPerStep() * 1000000 + stepDuration := time.Duration(nanoSecondsPerStep) + truncatedValues := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) + + currentTime := series.StartTime() + for i := 0; i < series.Len(); i++ { + ctString := currentTime.String() + fmt.Println(ctString) + if ( currentTime.After(startTime) && currentTime.Before(endTime)) { + truncatedValues.SetValueAt(i, series.ValueAtTime(currentTime)) + } + currentTime = currentTime.Add(stepDuration) + } + + slicedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), truncatedValues) + renamedSlicedSeries := slicedSeries.RenamedTo(fmt.Sprintf("timeSlice(%s,%s, %s)", slicedSeries.Name(), start, end)) + output[i] = renamedSlicedSeries + } + input.Values = output + return input, nil + } + + return &unaryContextShifter{ + ContextShiftFunc: contextShiftingFn, + UnaryTransformer: transformerFn, + }, nil +} + + + // 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, @@ -2037,6 +2088,9 @@ func init() { MustRegisterFunction(timeShift).WithDefaultParams(map[uint8]interface{}{ 3: true, // resetEnd }) + MustRegisterFunction(timeSlice).WithDefaultParams(map[uint8]interface{}{ + 3: "now", // endTime + }) MustRegisterFunction(transformNull).WithDefaultParams(map[uint8]interface{}{ 2: 0.0, // defaultValue }) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 26f5a8650c..ef4a270fd9 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -605,7 +605,7 @@ func TestTransformNull(t *testing.T) { var ( testMovingFunctionBootstrap = testMovingFunctionStart.Add(-30 * time.Second) testMovingFunctionStart = time.Now().Truncate(time.Minute) - testMovingFunctionEnd = testMovingFunctionStart.Add(time.Minute) + testMovingFunctionEnd = testMovingFunctionStart.Add(time.Minute * 120) ) func testMovingFunction(t *testing.T, target, expectedName string, values, bootstrap, output []float64) { @@ -642,6 +642,44 @@ func testMovingFunction(t *testing.T, target, expectedName string, values, boots expected, res.Values) } +var ( + testGeneralFunctionStart = time.Now().Add(time.Minute * -11).Truncate(time.Minute) + testGeneralFunctionEnd = time.Now().Add(time.Minute * -3).Truncate(time.Minute) +) + +// testGeneralFunction is a copy of testMovingFunction but without any logic for bootstrapping values +func testGeneralFunction(t *testing.T, target, expectedName string, values, output []float64) { + ctx := common.NewTestContext() + defer ctx.Close() + + engine := NewEngine( + &common.MovingFunctionStorage{ + StepMillis: 60000, + Values: values, + }, + ) + phonyContext := common.NewContext(common.ContextOptions{ + Start: testGeneralFunctionStart, + End: testGeneralFunctionEnd, + 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, 60000, testMovingFunctionStart, + expected, res.Values) +} + 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} @@ -690,16 +728,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 +2604,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() @@ -2857,6 +2895,42 @@ func TestTimeShift(t *testing.T) { []common.TestSeries{expected}, res.Values) } +/* + + def test_timeSlice(self): + # series starts at 60 seconds past the epoch and continues for 600 seconds (ten minutes) + # steps are every 60 seconds + seriesList = self._gen_series_list_with_data( + key='test.value', + start=0, + end=600, + step=60, + data=[None,1,2,3,None,5,6,None,7,8,9] + ) + + # we're going to slice such that we only include minutes 3 to 8 (of 0 to 9) + expectedResult = [ + TimeSeries('timeSlice(test.value, 180, 480)',0,600,60,[None,None,None,3,None,5,6,None,7,None,None]) + ] + + results = functions.timeSlice( + self._build_requestContext( + startTime=datetime(1970, 1, 1, 0, 0, 0, 0, pytz.timezone(settings.TIME_ZONE)), + endTime=datetime(1970, 1, 1, 0, 9, 0, 0, pytz.timezone(settings.TIME_ZONE)) + ), + seriesList, + '00:03 19700101', + '00:08 19700101' + ) + self.assertEqual(results, expectedResult) +*/ +func TestTimeSlice(t *testing.T) { + values := []float64{math.NaN(),1.0,2.0,3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,8.0,9.0} + expected := []float64{math.NaN(),math.NaN(),math.NaN(),3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,math.NaN(),math.NaN()} + + testGeneralFunction(t, "timeSlice(foo.bar.baz, '-8min','-2min')", "timeSlice(foo.bar.baz,-8min, -2min)", values, expected) +} + func TestDashed(t *testing.T) { ctx := common.NewTestContext() defer ctx.Close() @@ -3007,6 +3081,7 @@ func TestFunctionsRegistered(t *testing.T) { "time", "timeFunction", "timeShift", + "timeSlice", "transformNull", "weightedAverage", } From 17c42bd3cb38047c84655523965f3f395729b6ad Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Wed, 2 Sep 2020 20:18:24 -0400 Subject: [PATCH 12/21] Apply suggestions from code review --- src/query/graphite/native/builtin_functions_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 90e0cb80c9..7e81bfc0ee 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -605,7 +605,7 @@ func TestTransformNull(t *testing.T) { var ( testMovingFunctionBootstrap = testMovingFunctionStart.Add(-30 * time.Second) testMovingFunctionStart = time.Now().Truncate(time.Minute) - testMovingFunctionEnd = testMovingFunctionStart.Add(time.Minute * 120) + testMovingFunctionEnd = testMovingFunctionStart.Add(time.Minute) ) func testMovingFunction(t *testing.T, target, expectedName string, values, bootstrap, output []float64) { From f246f8271fbd4d32c95eca349ea5c8d2d6d572c9 Mon Sep 17 00:00:00 2001 From: Theodore Date: Wed, 2 Sep 2020 20:27:48 -0400 Subject: [PATCH 13/21] Finished the timeSlice function --- src/query/graphite/native/builtin_functions.go | 2 -- src/query/graphite/native/builtin_functions_test.go | 5 ++--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 084c5f4abc..29bab882c7 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -269,8 +269,6 @@ func timeSlice(ctx *common.Context, input singlePathSpec, start string, end stri currentTime := series.StartTime() for i := 0; i < series.Len(); i++ { - ctString := currentTime.String() - fmt.Println(ctString) if ( currentTime.After(startTime) && currentTime.Before(endTime)) { truncatedValues.SetValueAt(i, series.ValueAtTime(currentTime)) } diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 7e81bfc0ee..c4112dae99 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -676,8 +676,7 @@ func testGeneralFunction(t *testing.T, target, expectedName string, values, outp } expected = append(expected, expectedSeries) } - common.CompareOutputsAndExpected(t, 60000, testMovingFunctionStart, - expected, res.Values) + common.CompareOutputsAndExpected(t, 60000, testGeneralFunctionStart, expected, res.Values) } func TestMovingAverageSuccess(t *testing.T) { @@ -2911,7 +2910,7 @@ func TestTimeSlice(t *testing.T) { values := []float64{math.NaN(),1.0,2.0,3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,8.0,9.0} expected := []float64{math.NaN(),math.NaN(),math.NaN(),3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,math.NaN(),math.NaN()} - testGeneralFunction(t, "timeSlice(foo.bar.baz, '-8min','-2min')", "timeSlice(foo.bar.baz,-8min, -2min)", values, expected) + testGeneralFunction(t, "timeSlice(foo.bar.baz, '-9min','-3min')", "timeSlice(foo.bar.baz,-9min, -3min)", values, expected) } func TestDashed(t *testing.T) { From 8f60dca10c089f3dda97b9ce42152c53618f563a Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Wed, 2 Sep 2020 20:30:10 -0400 Subject: [PATCH 14/21] Update src/query/graphite/native/builtin_functions_test.go --- .../graphite/native/builtin_functions_test.go | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index c4112dae99..68b2e12594 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2877,35 +2877,6 @@ func TestTimeShift(t *testing.T) { []common.TestSeries{expected}, res.Values) } -/* - - def test_timeSlice(self): - # series starts at 60 seconds past the epoch and continues for 600 seconds (ten minutes) - # steps are every 60 seconds - seriesList = self._gen_series_list_with_data( - key='test.value', - start=0, - end=600, - step=60, - data=[None,1,2,3,None,5,6,None,7,8,9] - ) - - # we're going to slice such that we only include minutes 3 to 8 (of 0 to 9) - expectedResult = [ - TimeSeries('timeSlice(test.value, 180, 480)',0,600,60,[None,None,None,3,None,5,6,None,7,None,None]) - ] - - results = functions.timeSlice( - self._build_requestContext( - startTime=datetime(1970, 1, 1, 0, 0, 0, 0, pytz.timezone(settings.TIME_ZONE)), - endTime=datetime(1970, 1, 1, 0, 9, 0, 0, pytz.timezone(settings.TIME_ZONE)) - ), - seriesList, - '00:03 19700101', - '00:08 19700101' - ) - self.assertEqual(results, expectedResult) -*/ func TestTimeSlice(t *testing.T) { values := []float64{math.NaN(),1.0,2.0,3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,8.0,9.0} expected := []float64{math.NaN(),math.NaN(),math.NaN(),3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,math.NaN(),math.NaN()} From ca19d919bf40fc337d039da51b9bcd63bb3d853c Mon Sep 17 00:00:00 2001 From: Theodore Date: Wed, 2 Sep 2020 22:13:51 -0400 Subject: [PATCH 15/21] fixed up the timeSlice code --- .../graphite/native/builtin_functions.go | 50 +++++++------------ .../graphite/native/builtin_functions_test.go | 2 +- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 29bab882c7..d735e03162 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -244,49 +244,37 @@ func timeShift( } -func timeSlice(ctx *common.Context, input singlePathSpec, start string, end string) (*unaryContextShifter, error) { - - contextShiftingFn := func(c *common.Context) *common.Context { - // no need to shift the context here - return c; - } - +func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end string) (ts.SeriesList, error) { tzOffsetForAbsoluteTime := time.Duration(0) startTime, err := graphite.ParseTime(start, time.Now(), tzOffsetForAbsoluteTime) endTime, err := graphite.ParseTime(end, time.Now(), tzOffsetForAbsoluteTime) if (err != nil) { - return nil, err + return ts.NewSeriesList(), err } - transformerFn := func(input ts.SeriesList) (ts.SeriesList, error) { - output := make([]*ts.Series, input.Len()) + input := ts.SeriesList(inputPath) + output := make([]*ts.Series, input.Len()) - for i, series := range input.Values { - nanoSecondsPerStep := series.MillisPerStep() * 1000000 - stepDuration := time.Duration(nanoSecondsPerStep) - truncatedValues := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) + for i, series := range input.Values { + nanoSecondsPerStep := series.MillisPerStep() * 1000000 + stepDuration := time.Duration(nanoSecondsPerStep) + truncatedValues := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) - currentTime := series.StartTime() - for i := 0; i < series.Len(); i++ { - if ( currentTime.After(startTime) && currentTime.Before(endTime)) { - truncatedValues.SetValueAt(i, series.ValueAtTime(currentTime)) - } - currentTime = currentTime.Add(stepDuration) + currentTime := series.StartTime() + for i := 0; i < series.Len(); i++ { + if ( currentTime.After(startTime) && currentTime.Before(endTime)) { + truncatedValues.SetValueAt(i, series.ValueAtTime(currentTime)) } - - slicedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), truncatedValues) - renamedSlicedSeries := slicedSeries.RenamedTo(fmt.Sprintf("timeSlice(%s,%s, %s)", slicedSeries.Name(), start, end)) - output[i] = renamedSlicedSeries + currentTime = currentTime.Add(stepDuration) } - input.Values = output - return input, nil - } - return &unaryContextShifter{ - ContextShiftFunc: contextShiftingFn, - UnaryTransformer: transformerFn, - }, nil + slicedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), truncatedValues) + renamedSlicedSeries := slicedSeries.RenamedTo(fmt.Sprintf("timeSlice(%s, %s, %s)", slicedSeries.Name(), start, end)) + output[i] = renamedSlicedSeries + } + input.Values = output + return input, nil } diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index 68b2e12594..caa262911f 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2881,7 +2881,7 @@ func TestTimeSlice(t *testing.T) { values := []float64{math.NaN(),1.0,2.0,3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,8.0,9.0} expected := []float64{math.NaN(),math.NaN(),math.NaN(),3.0,math.NaN(),5.0,6.0,math.NaN(),7.0,math.NaN(),math.NaN()} - testGeneralFunction(t, "timeSlice(foo.bar.baz, '-9min','-3min')", "timeSlice(foo.bar.baz,-9min, -3min)", values, expected) + testGeneralFunction(t, "timeSlice(foo.bar.baz, '-9min','-3min')", "timeSlice(foo.bar.baz, -9min, -3min)", values, expected) } func TestDashed(t *testing.T) { From 9f0e2c7cdeeda7be43f73aff8e359aa0adb904c2 Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Wed, 2 Sep 2020 22:15:14 -0400 Subject: [PATCH 16/21] Apply suggestions from code review --- src/query/graphite/native/builtin_functions_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go index caa262911f..fe6213212d 100644 --- a/src/query/graphite/native/builtin_functions_test.go +++ b/src/query/graphite/native/builtin_functions_test.go @@ -2586,12 +2586,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"} { 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() From 902accc30ba7fcb5714e9589cd4770b8a4f2255f Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Fri, 4 Sep 2020 14:40:19 -0400 Subject: [PATCH 17/21] Update src/query/graphite/native/builtin_functions.go Co-authored-by: Rob Skillington --- src/query/graphite/native/builtin_functions.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 2b7f363e41..616ae28dac 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -245,11 +245,16 @@ func timeShift( func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end string) (ts.SeriesList, error) { - tzOffsetForAbsoluteTime := time.Duration(0) - startTime, err := graphite.ParseTime(start, time.Now(), tzOffsetForAbsoluteTime) - endTime, err := graphite.ParseTime(end, time.Now(), tzOffsetForAbsoluteTime) - - if (err != nil) { + var ( + now = time.Now() + tzOffsetForAbsoluteTime time.Duration + ) + startTime, err := graphite.ParseTime(start, now, tzOffsetForAbsoluteTime) + if err != nil { + return ts.NewSeriesList(), err + } + endTime, err := graphite.ParseTime(end, now, tzOffsetForAbsoluteTime) + if err != nil { return ts.NewSeriesList(), err } From 381dcec7bb506a89a9334d2a397c3a20f4e209c0 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 4 Sep 2020 11:44:26 -0700 Subject: [PATCH 18/21] added method description --- src/query/graphite/native/builtin_functions.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 2b7f363e41..43ffd4b6e0 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -243,7 +243,9 @@ func timeShift( }, nil } - +// timeSlice takes one metric or a wildcard metric, followed by a quoted string with the time to start the line and +// another quoted string with the time to end the line. The start and end times are inclusive. +// Useful for filtering out a part of a series of data from a wider range of data. func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end string) (ts.SeriesList, error) { tzOffsetForAbsoluteTime := time.Duration(0) startTime, err := graphite.ParseTime(start, time.Now(), tzOffsetForAbsoluteTime) From 89a960019e375b5800ce903bee14ff5321e86104 Mon Sep 17 00:00:00 2001 From: teddywahle <69990143+teddywahle@users.noreply.github.com> Date: Fri, 4 Sep 2020 14:49:10 -0400 Subject: [PATCH 19/21] Update src/query/graphite/native/builtin_functions.go Co-authored-by: Rob Skillington --- 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 616ae28dac..a05152dc35 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -262,8 +262,7 @@ func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end output := make([]*ts.Series, input.Len()) for i, series := range input.Values { - nanoSecondsPerStep := series.MillisPerStep() * 1000000 - stepDuration := time.Duration(nanoSecondsPerStep) + stepDuration := time.Duration(series.MillisPerStep()) * time.Millisecond truncatedValues := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) currentTime := series.StartTime() From 26595658cd7af7e3edd9553ca04f301293fa9671 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 4 Sep 2020 11:51:59 -0700 Subject: [PATCH 20/21] Added suggestions from code review --- src/query/graphite/native/builtin_functions.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index a7d24105f3..87a921afb8 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -261,7 +261,7 @@ func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end } input := ts.SeriesList(inputPath) - output := make([]*ts.Series, input.Len()) + output := make([]*ts.Series, 0, input.Len()) for i, series := range input.Values { stepDuration := time.Duration(series.MillisPerStep()) * time.Millisecond @@ -269,7 +269,9 @@ func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end currentTime := series.StartTime() for i := 0; i < series.Len(); i++ { - if ( currentTime.After(startTime) && currentTime.Before(endTime)) { + equalOrAfterStart := currentTime.Equal(startTime) || currentTime.After(startTime) + beforeOrEqualEnd := currentTime.Before(endTime) || currentTime.Equal(endTime) + if equalOrAfterStart && beforeOrEqualEnd { truncatedValues.SetValueAt(i, series.ValueAtTime(currentTime)) } currentTime = currentTime.Add(stepDuration) @@ -277,6 +279,7 @@ func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end slicedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), truncatedValues) renamedSlicedSeries := slicedSeries.RenamedTo(fmt.Sprintf("timeSlice(%s, %s, %s)", slicedSeries.Name(), start, end)) + output[i] = renamedSlicedSeries } input.Values = output From 9e558a8340ea89790b6dd5962f30bb0692d4d137 Mon Sep 17 00:00:00 2001 From: Theodore Wahle Date: Fri, 4 Sep 2020 11:57:23 -0700 Subject: [PATCH 21/21] fixed indexing bug --- src/query/graphite/native/builtin_functions.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go index 87a921afb8..82ad0277e8 100644 --- a/src/query/graphite/native/builtin_functions.go +++ b/src/query/graphite/native/builtin_functions.go @@ -263,7 +263,7 @@ func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end input := ts.SeriesList(inputPath) output := make([]*ts.Series, 0, input.Len()) - for i, series := range input.Values { + for _, series := range input.Values { stepDuration := time.Duration(series.MillisPerStep()) * time.Millisecond truncatedValues := ts.NewValues(ctx, series.MillisPerStep(), series.Len()) @@ -279,8 +279,7 @@ func timeSlice(ctx *common.Context, inputPath singlePathSpec, start string, end slicedSeries := ts.NewSeries(ctx, series.Name(), series.StartTime(), truncatedValues) renamedSlicedSeries := slicedSeries.RenamedTo(fmt.Sprintf("timeSlice(%s, %s, %s)", slicedSeries.Name(), start, end)) - - output[i] = renamedSlicedSeries + output = append(output, renamedSlicedSeries) } input.Values = output return input, nil