Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[query] Implemented the Graphite delay function #2567

Merged
merged 38 commits into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
4e901d7
Added the moving movingMin function
teddywahle Aug 27, 2020
f42685a
Merge branch 'master' into twahle-moving-min
teddywahle Aug 27, 2020
f0df4ce
worked on moving min
teddywahle Aug 27, 2020
bef6377
Merge branch 'twahle-moving-min' of https://github.com/teddywahle/m3 …
teddywahle Aug 27, 2020
8ce6dc3
more work on the moving min func
teddywahle Aug 27, 2020
30ef946
Merge branch 'master' into twahle-moving-min
teddywahle Aug 28, 2020
cf9288f
updated movingMedian
teddywahle Aug 28, 2020
50b0ae9
Merge branch 'twahle-moving-min' of https://github.com/teddywahle/m3 …
teddywahle Aug 28, 2020
1ebe629
Apply suggestions from code review
teddywahle Aug 28, 2020
ebaafce
testMovingFunction
teddywahle Aug 28, 2020
2a0643c
wrote test movingSum function
teddywahle Aug 28, 2020
cb088b2
Added delay function
teddywahle Aug 28, 2020
2e183a5
built out more of the core logic for delay function
teddywahle Aug 28, 2020
dbe7b33
fixed up delay test
teddywahle Aug 28, 2020
53222b9
Merge branch 'master' into twahle-moving-min
teddywahle Aug 31, 2020
8c2b6bf
updated tests
teddywahle Aug 31, 2020
01792e7
Fixed up tests
teddywahle Aug 31, 2020
a1f7cad
Merge branch 'master' into graphite-delay-function
teddywahle Aug 31, 2020
2ea3fa8
Merge branch 'graphite-delay-function' of https://github.com/teddywah…
teddywahle Aug 31, 2020
29a032d
Update percentiles.go
teddywahle Aug 31, 2020
dba69f4
Apply suggestions from code review
teddywahle Aug 31, 2020
28799cb
Merge branch 'master' into graphite-delay-function
teddywahle Aug 31, 2020
117e24f
Update builtin_functions.go
teddywahle Aug 31, 2020
a3b7abb
Apply suggestions from code review
teddywahle Aug 31, 2020
a2ab5aa
Apply suggestions from code review
teddywahle Aug 31, 2020
5ff388b
Merge branch 'master' into graphite-delay-function
teddywahle Sep 1, 2020
b9a5ecc
clean up delay and add comments
teddywahle Sep 1, 2020
09ed1ce
Merge branch 'master' into graphite-delay-function
teddywahle Sep 2, 2020
9e70f82
Merge branch 'master' into graphite-delay-function
teddywahle Sep 3, 2020
9be8bfa
removed contextShift from delay function
teddywahle Sep 3, 2020
99852b2
updated testdelay function
teddywahle Sep 3, 2020
0184b23
Merge branch 'master' into graphite-delay-function
teddywahle Sep 3, 2020
ad6fc10
Merge branch 'master' into graphite-delay-function
teddywahle Sep 4, 2020
f5e30d3
Update src/query/graphite/native/builtin_functions.go
teddywahle Sep 4, 2020
1d54439
Updated delay after PR review
teddywahle Sep 4, 2020
c545823
Merge branch 'master' into graphite-delay-function
teddywahle Sep 7, 2020
d20b425
Merge branch 'master' into graphite-delay-function
robskillington Sep 8, 2020
5aa39ee
Update src/query/graphite/native/builtin_functions.go
robskillington Sep 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/query/graphite/native/builtin_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,40 @@ func timeShift(
}, nil
}


// delay shifts all samples later by an integer number of steps. This can be used
robskillington marked this conversation as resolved.
Show resolved Hide resolved
// 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,
singlePath singlePathSpec,
steps int,
) (ts.SeriesList, error) {
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
Copy link
Collaborator

@robskillington robskillington Sep 4, 2020

Choose a reason for hiding this comment

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

nit: Prefer to pre-allocate slice but use append for collecting values.

e.g.

output := make([]*ts.Series, 0, input.Len())
// ...
output = append(output, renamedSeries)

We find using this wherever possible causes less bugs in general (i.e. if you have a continue that skips some entries in future in the middle of the loop, etc).

}
input.Values = output
return input, nil
teddywahle marked this conversation as resolved.
Show resolved Hide resolved
}

// 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) {
Copy link
Collaborator

@robskillington robskillington Sep 4, 2020

Choose a reason for hiding this comment

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

Can you change series to be series *ts.Series so that it doesn't have to be dereferenced when called? Don't seem to need a reason to dereference the type instead of just passing the ref directly.

nit: Remove the extra space here steps int ) should be steps int).

also nit: Should be just ts.Values not (ts.Values) since it's single return type.

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,
Expand Down Expand Up @@ -1885,6 +1919,7 @@ func init() {
MustRegisterFunction(dashed).WithDefaultParams(map[uint8]interface{}{
2: 5.0, // dashLength
})
MustRegisterFunction(delay)
MustRegisterFunction(derivative)
MustRegisterFunction(diffSeries)
MustRegisterFunction(divideSeries)
Expand Down
56 changes: 56 additions & 0 deletions src/query/graphite/native/builtin_functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2840,6 +2840,61 @@ func TestTimeShift(t *testing.T) {
[]common.TestSeries{expected}, res.Values)
}

func TestDelay(t *testing.T) {
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][]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)
}
}

var (
testDelayStart = time.Now().Truncate(time.Minute)
testDelayEnd = testMovingFunctionEnd.Add(time.Minute)
)

func testDelay(t *testing.T, target, expectedName string, values, output []float64) {
ctx := common.NewTestContext()
defer ctx.Close()

engine := NewEngine(
&common.MovingFunctionStorage{
StepMillis: 10000,
Values: values,
},
)
phonyContext := common.NewContext(common.ContextOptions{
Start: testDelayStart,
End: testDelayEnd,
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, testDelayStart, expected, res.Values)
}

func TestDashed(t *testing.T) {
ctx := common.NewTestContext()
defer ctx.Close()
Expand Down Expand Up @@ -2927,6 +2982,7 @@ func TestFunctionsRegistered(t *testing.T) {
"currentAbove",
"currentBelow",
"dashed",
"delay",
"derivative",
"diffSeries",
"divideSeries",
Expand Down