[query] Fix Graphite log and logarithm to accept float64 base arg (#3145

)
m3db · Mar 17, 2021 · 6b91769 · 6b91769
1 parent 965f566
commit 6b91769
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 11 deletions.
diff --git a/src/query/graphite/native/builtin_functions.go b/src/query/graphite/native/builtin_functions.go
@@ -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
@@ -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))
 			}
 		}
 

diff --git a/src/query/graphite/native/builtin_functions_test.go b/src/query/graphite/native/builtin_functions_test.go
@@ -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() }()
 
@@ -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))
+		xtest.Equalish(t, 0.9596044321978149, output.ValueAt(3))
+	})
 
 	_, err := logarithm(nil, singlePathSpec{}, -1)
 	require.NotNil(t, err)