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] Allow Graphite log and logarithm to accept float64 base arg #3145

Merged
merged 5 commits into from
Mar 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/query/graphite/native/builtin_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1203,16 +1203,16 @@ func pow(ctx *common.Context, input singlePathSpec, factor float64) (ts.SeriesLi

// logarithm takes one metric or a wildcard seriesList, and draws the y-axis in
// logarithmic format.
func logarithm(ctx *common.Context, input singlePathSpec, base int) (ts.SeriesList, error) {
func logarithm(ctx *common.Context, input singlePathSpec, base float64) (ts.SeriesList, error) {
if base <= 0 {
err := errors.NewInvalidParamsError(fmt.Errorf("invalid log base %d", base))
err := errors.NewInvalidParamsError(fmt.Errorf("invalid log base %f", base))
return ts.NewSeriesList(), err
}

results := make([]*ts.Series, 0, len(input.Values))
for _, series := range input.Values {
vals := ts.NewValues(ctx, series.MillisPerStep(), series.Len())
newName := fmt.Sprintf("log(%s, %d)", series.Name(), base)
newName := fmt.Sprintf("log(%s, %f)", series.Name(), base)
if series.AllNaN() {
results = append(results, ts.NewSeries(ctx, newName, series.StartTime(), vals))
continue
Expand All @@ -1221,7 +1221,7 @@ func logarithm(ctx *common.Context, input singlePathSpec, base int) (ts.SeriesLi
for i := 0; i < series.Len(); i++ {
n := series.ValueAt(i)
if !math.IsNaN(n) && n > 0 {
vals.SetValueAt(i, math.Log10(n)/math.Log10(float64(base)))
vals.SetValueAt(i, math.Log10(n)/math.Log10(base))
}
}

Expand Down
26 changes: 19 additions & 7 deletions src/query/graphite/native/builtin_functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2181,7 +2181,8 @@ func TestAsPercentWithSeriesList(t *testing.T) {
}
}

func testLogarithm(t *testing.T, base int, indices []int) {
// nolint: thelper
func testLogarithm(t *testing.T, base float64, asserts func(*ts.Series)) {
ctx := common.NewTestContext()
defer func() { _ = ctx.Close() }()

Expand All @@ -2200,18 +2201,29 @@ func testLogarithm(t *testing.T, base int, indices []int) {

output := r.Values
require.Equal(t, 1, len(output))
assert.Equal(t, fmt.Sprintf("log(hello, %d)", base), output[0].Name())
assert.Equal(t, fmt.Sprintf("log(hello, %f)", base), output[0].Name())
assert.Equal(t, series.StartTime(), output[0].StartTime())
require.Equal(t, len(invals), output[0].Len())
xtest.Equalish(t, math.NaN(), output[0].ValueAt(0))
xtest.Equalish(t, 0, output[0].ValueAt(indices[0]))
xtest.Equalish(t, 1, output[0].ValueAt(indices[1]))
xtest.Equalish(t, 2, output[0].ValueAt(indices[2]))
asserts(output[0])
}

func TestLogarithm(t *testing.T) {
testLogarithm(t, 10, []int{1, 10, 100})
testLogarithm(t, 2, []int{1, 2, 4})
testLogarithm(t, 10, func(output *ts.Series) {
xtest.Equalish(t, 0, output.ValueAt(1))
xtest.Equalish(t, 1, output.ValueAt(10))
xtest.Equalish(t, 2, output.ValueAt(100))
})
testLogarithm(t, 2, func(output *ts.Series) {
xtest.Equalish(t, 0, output.ValueAt(1))
xtest.Equalish(t, 1, output.ValueAt(2))
xtest.Equalish(t, 2, output.ValueAt(4))
})
testLogarithm(t, 3.142, func(output *ts.Series) {
xtest.Equalish(t, 0, output.ValueAt(1))
xtest.Equalish(t, 0.6054429879326457, output.ValueAt(2))
Copy link
Collaborator

Choose a reason for hiding this comment

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

mind leaving a note how these values are derived?

xtest.Equalish(t, 0.9596044321978149, output.ValueAt(3))
})

_, err := logarithm(nil, singlePathSpec{}, -1)
require.NotNil(t, err)
Expand Down