Skip to content

Commit

Permalink
[query] Add support for dropping NaNs when rendering results (#1458)
Browse files Browse the repository at this point in the history
  • Loading branch information
benraskin92 authored Mar 15, 2019
1 parent 753eb9e commit 2d067d9
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 3 deletions.
10 changes: 10 additions & 0 deletions src/cmd/services/m3query/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ type Configuration struct {

// Cache configurations.
Cache CacheConfiguration `yaml:"cache"`

// ResultOptions are the results options for query.
ResultOptions ResultOptions `yaml:"resultOptions"`
}

// Filter is a query filter type.
Expand All @@ -146,6 +149,13 @@ type FilterConfiguration struct {
CompleteTags Filter `yaml:"completeTags"`
}

// ResultOptions are the result options for query.
type ResultOptions struct {
// KeepNans keeps NaNs before returning query results.
// The default is false, which matches Prometheus
KeepNans bool `yaml:"keepNans"`
}

// CacheConfiguration is the cache configurations.
type CacheConfiguration struct {
// QueryConversion cache policy.
Expand Down
10 changes: 10 additions & 0 deletions src/cmd/services/m3query/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,13 @@ func TestNilQueryConversionSize(t *testing.T) {
err := q.Validate()
require.NoError(t, err)
}

func TestKeepNaNsDefault(t *testing.T) {
r := ResultOptions{
KeepNans: true,
}
assert.Equal(t, true, r.KeepNans)

r = ResultOptions{}
assert.Equal(t, false, r.KeepNans)
}
8 changes: 8 additions & 0 deletions src/query/api/v1/handler/prometheus/native/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package native
import (
"fmt"
"io"
"math"
"net/http"
"strconv"
"strings"
Expand Down Expand Up @@ -231,6 +232,7 @@ func renderResultsJSON(
w io.Writer,
series []*ts.Series,
params models.RequestParams,
keepNans bool,
) {
jw := json.NewWriter(w)
jw.BeginObject()
Expand Down Expand Up @@ -262,6 +264,12 @@ func renderResultsJSON(
length := s.Len()
for i := 0; i < length; i++ {
dp := vals.DatapointAt(i)

// If keepNaNs is set to false and the value is NaN, drop it from the response.
if !keepNans && math.IsNaN(dp.Value) {
continue
}

// Skip points before the query boundary. Ideal place to adjust these would be at the result node but that would make it inefficient
// since we would need to create another block just for the sake of restricting the bounds.
// Each series have the same start time so we just need to calculate the correct startIdx once
Expand Down
111 changes: 109 additions & 2 deletions src/query/api/v1/handler/prometheus/native/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package native
import (
"bytes"
"encoding/json"
"math"
"net/http"
"net/url"
"testing"
Expand Down Expand Up @@ -158,9 +159,12 @@ func TestRenderResultsJSON(t *testing.T) {
start := time.Unix(1535948880, 0)
buffer := bytes.NewBuffer(nil)
params := models.RequestParams{}
valsWithNaN := ts.NewFixedStepValues(10*time.Second, 2, 1, start)
valsWithNaN.SetValueAt(1, math.NaN())

series := []*ts.Series{
ts.NewSeries([]byte("foo"),
ts.NewFixedStepValues(10*time.Second, 2, 1, start), test.TagSliceToTags([]models.Tag{
valsWithNaN, test.TagSliceToTags([]models.Tag{
models.Tag{Name: []byte("bar"), Value: []byte("baz")},
models.Tag{Name: []byte("qux"), Value: []byte("qaz")},
})),
Expand All @@ -169,9 +173,14 @@ func TestRenderResultsJSON(t *testing.T) {
models.Tag{Name: []byte("baz"), Value: []byte("bar")},
models.Tag{Name: []byte("qaz"), Value: []byte("qux")},
})),
ts.NewSeries([]byte("foobar"),
ts.NewFixedStepValues(10*time.Second, 2, math.NaN(), start), test.TagSliceToTags([]models.Tag{
models.Tag{Name: []byte("biz"), Value: []byte("baz")},
models.Tag{Name: []byte("qux"), Value: []byte("qaz")},
})),
}

renderResultsJSON(buffer, series, params)
renderResultsJSON(buffer, series, params, true)

expected := mustPrettyJSON(t, `
{
Expand All @@ -191,6 +200,95 @@ func TestRenderResultsJSON(t *testing.T) {
],
[
1535948890,
"NaN"
]
],
"step_size_ms": 10000
},
{
"metric": {
"baz": "bar",
"qaz": "qux"
},
"values": [
[
1535948880,
"2"
],
[
1535948890,
"2"
]
],
"step_size_ms": 10000
},
{
"metric": {
"biz": "baz",
"qux": "qaz"
},
"values": [
[
1535948880,
"NaN"
],
[
1535948890,
"NaN"
]
],
"step_size_ms": 10000
}
]
}
}
`)

actual := mustPrettyJSON(t, buffer.String())
assert.Equal(t, expected, actual, xtest.Diff(expected, actual))
}

func TestRenderResultsJSONWithDroppedNaNs(t *testing.T) {
start := time.Unix(1535948880, 0)
buffer := bytes.NewBuffer(nil)
params := models.RequestParams{}
valsWithNaN := ts.NewFixedStepValues(10*time.Second, 2, 1, start)
valsWithNaN.SetValueAt(1, math.NaN())

series := []*ts.Series{
ts.NewSeries([]byte("foo"),
valsWithNaN, test.TagSliceToTags([]models.Tag{
models.Tag{Name: []byte("bar"), Value: []byte("baz")},
models.Tag{Name: []byte("qux"), Value: []byte("qaz")},
})),
ts.NewSeries([]byte("bar"),
ts.NewFixedStepValues(10*time.Second, 2, 2, start), test.TagSliceToTags([]models.Tag{
models.Tag{Name: []byte("baz"), Value: []byte("bar")},
models.Tag{Name: []byte("qaz"), Value: []byte("qux")},
})),
ts.NewSeries([]byte("foobar"),
ts.NewFixedStepValues(10*time.Second, 2, math.NaN(), start), test.TagSliceToTags([]models.Tag{
models.Tag{Name: []byte("biz"), Value: []byte("baz")},
models.Tag{Name: []byte("qux"), Value: []byte("qaz")},
})),
}

renderResultsJSON(buffer, series, params, false)

expected := mustPrettyJSON(t, `
{
"status": "success",
"data": {
"resultType": "matrix",
"result": [
{
"metric": {
"bar": "baz",
"qux": "qaz"
},
"values": [
[
1535948880,
"1"
]
],
Expand All @@ -212,11 +310,20 @@ func TestRenderResultsJSON(t *testing.T) {
]
],
"step_size_ms": 10000
},
{
"metric": {
"biz": "baz",
"qux": "qaz"
},
"values": [],
"step_size_ms": 10000
}
]
}
}
`)

actual := mustPrettyJSON(t, buffer.String())
assert.Equal(t, expected, actual, xtest.Diff(expected, actual))
}
Expand Down
5 changes: 4 additions & 1 deletion src/query/api/v1/handler/prometheus/native/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type PromReadHandler struct {
limitsCfg *config.LimitsConfiguration
promReadMetrics promReadMetrics
timeoutOps *prometheus.TimeoutOpts
keepNans bool
}

type promReadMetrics struct {
Expand Down Expand Up @@ -109,13 +110,15 @@ func NewPromReadHandler(
limitsCfg *config.LimitsConfiguration,
scope tally.Scope,
timeoutOpts *prometheus.TimeoutOpts,
keepNans bool,
) *PromReadHandler {
h := &PromReadHandler{
engine: engine,
tagOpts: tagOpts,
limitsCfg: limitsCfg,
promReadMetrics: newPromReadMetrics(scope),
timeoutOps: timeoutOpts,
keepNans: keepNans,
}

h.promReadMetrics.maxDatapoints.Update(float64(limitsCfg.MaxComputedDatapoints()))
Expand All @@ -142,7 +145,7 @@ func (h *PromReadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
h.promReadMetrics.fetchSuccess.Inc(1)
timer.Stop()
// TODO: Support multiple result types
renderResultsJSON(w, result, params)
renderResultsJSON(w, result, params, h.keepNans)
}

// ServeHTTPWithEngine returns query results from the storage
Expand Down
1 change: 1 addition & 0 deletions src/query/api/v1/handler/prometheus/native/read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ func newTestSetup() *testSetup {
&config.LimitsConfiguration{},
tally.NewTestScope("", nil),
timeoutOpts,
false,
),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ func newServer() (*httptest.Server, *PromDebugHandler) {
&config.LimitsConfiguration{},
tally.NewTestScope("test", nil),
timeoutOpts,
true,
), tally.NewTestScope("test", nil),
defaultLookbackDuration,
)
Expand Down
1 change: 1 addition & 0 deletions src/query/api/v1/httpd/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ func (h *Handler) RegisterRoutes() error {
&h.config.Limits,
h.scope.Tagged(nativeSource),
h.timeoutOpts,
h.config.ResultOptions.KeepNans,
)

h.router.HandleFunc(remote.PromReadURL,
Expand Down
1 change: 1 addition & 0 deletions src/query/models/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type RequestParams struct {
Step time.Duration
Query string
Debug bool
KeepNans bool
IncludeEnd bool
BlockType FetchedBlockType
FormatType FormatType
Expand Down

0 comments on commit 2d067d9

Please sign in to comment.