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

Fix downsampling option in querier URL #1562

Merged
merged 3 commits into from
Oct 28, 2019
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
7 changes: 3 additions & 4 deletions pkg/query/api/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,10 @@ func (api *API) parseDownsamplingParamMillis(r *http.Request, defaultVal time.Du
const maxSourceResolutionParam = "max_source_resolution"
maxSourceResolution := 0 * time.Second

if api.enableAutodownsampling {
val := r.FormValue(maxSourceResolutionParam)
if api.enableAutodownsampling || (val == "auto") {
maxSourceResolution = defaultVal
}

if val := r.FormValue(maxSourceResolutionParam); val != "" {
} else if val != "" {
var err error
maxSourceResolution, err = parseDuration(val)
if err != nil {
Expand Down
142 changes: 71 additions & 71 deletions pkg/ui/bindata.go

Large diffs are not rendered by default.

13 changes: 11 additions & 2 deletions pkg/ui/static/js/graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ Prometheus.Graph.prototype.initialize = function() {
if (self.options.tab === undefined) {
self.options.tab = 1;
}
if (self.options.max_source_resolution === undefined) {
self.options.max_source_resolution = "0s";
}

// Draw graph controls and container from Handlebars template.

Expand Down Expand Up @@ -93,6 +96,8 @@ Prometheus.Graph.prototype.initialize = function() {
self.stacked = self.queryForm.find("input[name=stacked]");
self.insertMetric = self.queryForm.find("select[name=insert_metric]");
self.refreshInterval = self.queryForm.find("select[name=refresh]");
self.maxSourceResolutionInput = self.queryForm.find("select[name=max_source_resolution_input]");


self.consoleTab = graphWrapper.find(".console");
self.graphTab = graphWrapper.find(".graph_container");
Expand Down Expand Up @@ -231,6 +236,8 @@ Prometheus.Graph.prototype.initialize = function() {
stylePartialResponseBtn();
});

self.maxSourceResolutionInput.val(self.options.max_source_resolution);

self.queryForm.submit(function() {
self.consoleTab.addClass("reload");
self.graphTab.addClass("reload");
Expand Down Expand Up @@ -377,7 +384,6 @@ Prometheus.Graph.prototype.getOptions = function() {
"range_input",
"end_input",
"step_input",
"downsample_input",
"stacked",
"moment_input"
];
Expand All @@ -390,6 +396,9 @@ Prometheus.Graph.prototype.getOptions = function() {
}
}
});

options.max_source_resolution = self.maxSourceResolutionInput.val();

options.expr = self.expr.val();
options.tab = self.options.tab;
return options;
Expand Down Expand Up @@ -521,7 +530,7 @@ Prometheus.Graph.prototype.submitQuery = function() {
var startTime = new Date().getTime();
var rangeSeconds = self.parseDuration(self.rangeInput.val());
var resolution = parseInt(self.queryForm.find("input[name=step_input]").val()) || Math.max(Math.floor(rangeSeconds / 250), 1);
var maxSourceResolution = self.queryForm.find("select[name=max_source_resolution_input]").val();
var maxSourceResolution = self.maxSourceResolutionInput.val()
var endDate = self.getEndDate() / 1000;
var moment = self.getMoment() / 1000;

Expand Down
4 changes: 2 additions & 2 deletions pkg/ui/static/js/graph_template.handlebar
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@

<div class="prometheus_input_group pull-left">
<select name="max_source_resolution_input" title="Downsampling option" id="max_source_resolution_input{{id}}">
<option value="">Auto downsampling</option>
<option selected="selected" value="0s">Only raw data</option>
<option value="auto">Auto downsampling</option>
<option value="0s">Only raw data</option>
Copy link
Member

Choose a reason for hiding this comment

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

Quick silly question, why we remove selected here again? It changes what is by default set, right?

Copy link
Contributor Author

@obiesmans obiesmans Oct 21, 2019

Choose a reason for hiding this comment

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

I think that which option is selected by default should be driven from the javascript only.
(same place where the other UI driving logic is).

It doesn't change, the default which is still "0s" if the parameter is not set :
https://github.com/thanos-io/thanos/pull/1562/files#diff-b1f64ac291243b832fd99d257dcb04feR47-R49

Let me know if you agree with this choice, or if I should add this back in the UI.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense IMHO that we keep all of that logic together in that file instead of the HTML code, thanks for pointing this out!

<option value="5m">Max 5m downsampling</option>
<option value="1h">Max 1h downsampling</option>
</select>
Expand Down