Skip to content

Commit

Permalink
query/api: properly pass downsampling param (#1144)
Browse files Browse the repository at this point in the history
* query/api: properly pass downsampling param

As compact.ResolutionLevel{Raw,5m,1h} says the resolution levels are
expressed in milliseconds. Currently, parseDownsamplingParam() calls
parseDuration() and friends which express this value in nanoseconds.
Thus, we need to divide the value by 1000*1000 to have them in
milliseconds.

Add test cases to check all of this.

* CHANGELOG: add information about #1144
  • Loading branch information
Giedrius Statkevičius authored and brancz committed May 15, 2019
1 parent ce19c5d commit 7a767ef
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ We use *breaking* word for marking changes that are not backward compatible (rel

## Unreleased

### Fixed
- [#1144](https://github.com/improbable-eng/thanos/pull/1144) Query/API: properly pass the downsampling parameter. Before this, wrong max resolution of the metrics data might have been selected.

## [v0.4.0](https://github.com/improbable-eng/thanos/releases/tag/v0.4.0) - 2019.05.3

:warning: **IMPORTANT** :warning: This is the last release that supports gossip. From Thanos v0.5.0, gossip will be completely removed.
Expand Down
3 changes: 3 additions & 0 deletions pkg/query/api/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,9 @@ func (api *API) parseDownsamplingParam(r *http.Request, step time.Duration) (max
return 0, &ApiError{errorBadData, errors.Errorf("negative '%s' is not accepted. Try a positive integer", maxSourceResolutionParam)}
}

/// We need this in milliseconds.
maxSourceResolution = maxSourceResolution / (1000 * 1000)

return maxSourceResolution, nil
}

Expand Down
69 changes: 69 additions & 0 deletions pkg/query/api/v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"time"

"github.com/go-kit/kit/log"
"github.com/improbable-eng/thanos/pkg/compact"
"github.com/improbable-eng/thanos/pkg/query"
"github.com/improbable-eng/thanos/pkg/testutil"
"github.com/opentracing/opentracing-go"
Expand Down Expand Up @@ -716,6 +717,74 @@ func TestParseTime(t *testing.T) {
}
}

func TestParseDownsamplingParam(t *testing.T) {
var tests = []struct {
maxSourceResolution string
result time.Duration
step time.Duration
fail bool
enableAutodownsampling bool
}{
{
maxSourceResolution: "0s",
enableAutodownsampling: false,
step: time.Hour,
result: time.Duration(compact.ResolutionLevelRaw),
fail: false,
},
{
maxSourceResolution: "5m",
step: time.Hour,
enableAutodownsampling: false,
result: time.Duration(compact.ResolutionLevel5m),
fail: false,
},
{
maxSourceResolution: "1h",
step: time.Hour,
enableAutodownsampling: false,
result: time.Duration(compact.ResolutionLevel1h),
fail: false,
},
{
maxSourceResolution: "",
enableAutodownsampling: true,
step: time.Hour,
result: time.Duration(time.Hour / (5 * 1000 * 1000)),
fail: false,
},
{
maxSourceResolution: "",
enableAutodownsampling: true,
step: time.Hour,
result: time.Duration((1 * time.Hour) / 6),
fail: true,
},
{
maxSourceResolution: "",
enableAutodownsampling: true,
step: time.Hour,
result: time.Duration((1 * time.Hour) / 6),
fail: true,
},
}

for i, test := range tests {
api := API{enableAutodownsampling: test.enableAutodownsampling}
v := url.Values{}
v.Set("max_source_resolution", test.maxSourceResolution)
r := http.Request{PostForm: v}

maxSourceRes, _ := api.parseDownsamplingParam(&r, test.step)
if test.fail == false {
testutil.Assert(t, maxSourceRes == test.result, "case %v: expected %v to be equal to %v", i, maxSourceRes, test.result)
} else {
testutil.Assert(t, maxSourceRes != test.result, "case %v: expected %v not to be equal to %v", i, maxSourceRes, test.result)
}

}
}

func TestParseDuration(t *testing.T) {
var tests = []struct {
input string
Expand Down

0 comments on commit 7a767ef

Please sign in to comment.