Merge pull request #1942 from prometheus/beorn7/revert-graph-params

Revert /graph changes.

rules/alerting_test.go

@@ -27,11 +27,16 @@ func TestAlertingRuleHTMLSnippet(t *testing.T) {
 	}
 	rule := NewAlertingRule("testrule", expr, 0, model.LabelSet{"html": "<b>BOLD</b>"}, model.LabelSet{"html": "<b>BOLD</b>"})
 
-	const want = `ALERT <a href="/test/prefix/graph?g0.expr=ALERTS%7Balertname%3D%22testrule%22%7D&g0.tab=0">testrule</a>
+	// This is valid once the /graph changes have been reintroduced:
-  IF <a href="/test/prefix/graph?g0.expr=foo%7Bhtml%3D%22%3Cb%3EBOLD%3Cb%3E%22%7D&g0.tab=0">foo{html=&#34;&lt;b&gt;BOLD&lt;b&gt;&#34;}</a>
+	// const want = `ALERT <a href="/test/prefix/graph?g0.expr=ALERTS%7Balertname%3D%22testrule%22%7D&g0.tab=0">testrule</a>
+	//  IF <a href="/test/prefix/graph?g0.expr=foo%7Bhtml%3D%22%3Cb%3EBOLD%3Cb%3E%22%7D&g0.tab=0">foo{html=&#34;&lt;b&gt;BOLD&lt;b&gt;&#34;}</a>
+	//  LABELS {html=&#34;&lt;b&gt;BOLD&lt;/b&gt;&#34;}
+	//  ANNOTATIONS {html=&#34;&lt;b&gt;BOLD&lt;/b&gt;&#34;}`
+	// This is valid once the /graph changes have been reintroduced:
+	const want = `ALERT <a href="/test/prefix/graph#%5B%7B%22expr%22%3A%22ALERTS%7Balertname%3D%5C%22testrule%5C%22%7D%22%2C%22tab%22%3A0%7D%5D">testrule</a>
+  IF <a href="/test/prefix/graph#%5B%7B%22expr%22%3A%22foo%7Bhtml%3D%5C%22%3Cb%3EBOLD%3Cb%3E%5C%22%7D%22%2C%22tab%22%3A0%7D%5D">foo{html=&#34;&lt;b&gt;BOLD&lt;b&gt;&#34;}</a>
   LABELS {html=&#34;&lt;b&gt;BOLD&lt;/b&gt;&#34;}
   ANNOTATIONS {html=&#34;&lt;b&gt;BOLD&lt;/b&gt;&#34;}`
-
 	got := rule.HTMLSnippet("/test/prefix")
 	if got != want {
 		t.Fatalf("incorrect HTML snippet; want:\n\n|%v|\n\ngot:\n\n|%v|", want, got)

rules/recording_test.go

@@ -76,7 +76,10 @@ func TestRecordingRuleHTMLSnippet(t *testing.T) {
 	}
 	rule := NewRecordingRule("testrule", expr, model.LabelSet{"html": "<b>BOLD</b>"})
 
-	const want = `<a href="/test/prefix/graph?g0.expr=testrule&g0.tab=0">testrule</a>{html=&#34;&lt;b&gt;BOLD&lt;/b&gt;&#34;} = <a href="/test/prefix/graph?g0.expr=foo%7Bhtml%3D%22%3Cb%3EBOLD%3Cb%3E%22%7D&g0.tab=0">foo{html=&#34;&lt;b&gt;BOLD&lt;b&gt;&#34;}</a>`
+	// This is valid once the /graph changes have been reintroduced:
+	// const want = `<a href="/test/prefix/graph?g0.expr=testrule&g0.tab=0">testrule</a>{html=&#34;&lt;b&gt;BOLD&lt;/b&gt;&#34;} = <a href="/test/prefix/graph?g0.expr=foo%7Bhtml%3D%22%3Cb%3EBOLD%3Cb%3E%22%7D&g0.tab=0">foo{html=&#34;&lt;b&gt;BOLD&lt;b&gt;&#34;}</a>`
+	// This is what we need for now:
+	const want = `<a href="/test/prefix/graph#%5B%7B%22expr%22%3A%22testrule%22%2C%22tab%22%3A0%7D%5D">testrule</a>{html=&#34;&lt;b&gt;BOLD&lt;/b&gt;&#34;} = <a href="/test/prefix/graph#%5B%7B%22expr%22%3A%22foo%7Bhtml%3D%5C%22%3Cb%3EBOLD%3Cb%3E%5C%22%7D%22%2C%22tab%22%3A0%7D%5D">foo{html=&#34;&lt;b&gt;BOLD&lt;b&gt;&#34;}</a>`
 
 	got := rule.HTMLSnippet("/test/prefix")
 	if got != want {

template/template_test.go

@@ -182,12 +182,12 @@ func TestTemplateExpansion(t *testing.T) {
 		{
 			// graphLink.
 			text:   "{{ graphLink \"up\" }}",
-			output: "/graph?g0.expr=up&g0.tab=0",
+			output: "/graph#%5B%7B%22expr%22%3A%22up%22%2C%22tab%22%3A0%7D%5D",
 		},
 		{
 			// tableLink.
 			text:   "{{ tableLink \"up\" }}",
-			output: "/graph?g0.expr=up&g0.tab=1",
+			output: "/graph#%5B%7B%22expr%22%3A%22up%22%2C%22tab%22%3A1%7D%5D",
 		},
 		{
 			// tmpl.

util/strutil/strconv.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"net/url"
 	"regexp"
+	"strings"
 )
 
 var (
@@ -26,15 +27,22 @@ var (
 // TableLinkForExpression creates an escaped relative link to the table view of
 // the provided expression.
 func TableLinkForExpression(expr string) string {
-	escapedExpression := url.QueryEscape(expr)
+	// url.QueryEscape percent-escapes everything except spaces, for which it
-	return fmt.Sprintf("/graph?g0.expr=%s&g0.tab=1", escapedExpression)
+	// uses "+". However, in the non-query part of a URI, only percent-escaped
+	// spaces are legal, so we need to manually replace "+" with "%20" after
+	// query-escaping the string.
+	//
+	// See also:
+	// http://stackoverflow.com/questions/1634271/url-encoding-the-space-character-or-20.
+	urlData := url.QueryEscape(fmt.Sprintf(`[{"expr":%q,"tab":1}]`, expr))
+	return fmt.Sprintf("/graph#%s", strings.Replace(urlData, "+", "%20", -1))
 }
 
 // GraphLinkForExpression creates an escaped relative link to the graph view of
 // the provided expression.
 func GraphLinkForExpression(expr string) string {
-	escapedExpression := url.QueryEscape(expr)
+	urlData := url.QueryEscape(fmt.Sprintf(`[{"expr":%q,"tab":0}]`, expr))
-	return fmt.Sprintf("/graph?g0.expr=%s&g0.tab=0", escapedExpression)
+	return fmt.Sprintf("/graph#%s", strings.Replace(urlData, "+", "%20", -1))
 }
 
 // SanitizeLabelName replaces anything that doesn't match

util/strutil/strconv_test.go

@@ -1,49 +0,0 @@
-// Copyright 2016 The Prometheus Authors
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package strutil
-
-import (
-	"testing"
-)
-
-type linkTest struct {
-	expression        string
-	expectedGraphLink string
-	expectedTableLink string
-}
-
-var linkTests = []linkTest{
-	{
-		"sum(incoming_http_requests_total) by (system)",
-		"/graph?g0.expr=sum%28incoming_http_requests_total%29+by+%28system%29&g0.tab=0",
-		"/graph?g0.expr=sum%28incoming_http_requests_total%29+by+%28system%29&g0.tab=1",
-	},
-	{
-		"sum(incoming_http_requests_total{system=\"trackmetadata\"})",
-		"/graph?g0.expr=sum%28incoming_http_requests_total%7Bsystem%3D%22trackmetadata%22%7D%29&g0.tab=0",
-		"/graph?g0.expr=sum%28incoming_http_requests_total%7Bsystem%3D%22trackmetadata%22%7D%29&g0.tab=1",
-	},
-}
-
-func TestLink(t *testing.T) {
-	for _, tt := range linkTests {
-		if graphLink := GraphLinkForExpression(tt.expression); graphLink != tt.expectedGraphLink {
-			t.Errorf("GraphLinkForExpression failed for expression (%#q), want %q got %q", tt.expression, tt.expectedGraphLink, graphLink)
-		}
-
-		if tableLink := TableLinkForExpression(tt.expression); tableLink != tt.expectedTableLink {
-			t.Errorf("TableLinkForExpression failed for expression (%#q), want %q got %q", tt.expression, tt.expectedTableLink, tableLink)
-		}
-	}
-}

web/ui/static/js/graph.js

@@ -1,18 +1,16 @@
 var Prometheus = Prometheus || {};
+var graphs = [];
 var graphTemplate;
 
 var SECOND = 1000;
 
 Handlebars.registerHelper('pathPrefix', function() { return PATH_PREFIX; });
 
-Prometheus.Graph = function(element, options, handleChange, handleRemove) {
+Prometheus.Graph = function(element, options) {
   this.el = element;
   this.graphHTML = null;
   this.options = options;
-  this.handleChange = handleChange;
+  this.changeHandler = null;
-  this.handleRemove = function() {
-    handleRemove(this);
-  };
   this.rickshawGraph = null;
   this.data = [];
 
@@ -71,7 +69,7 @@ Prometheus.Graph.prototype.initialize = function() {
     };
     $(this).on('keyup input', function() { resizeTextarea(this); });
   });
-  self.expr.change(self.handleChange);
+  self.expr.change(storeGraphOptionsInURL);
 
   self.rangeInput = self.queryForm.find("input[name=range_input]");
   self.stackedBtn = self.queryForm.find(".stacked_btn");
@@ -87,7 +85,7 @@ Prometheus.Graph.prototype.initialize = function() {
   self.tabs.on("shown.bs.tab", function(e) {
     var target = $(e.target);
     self.options.tab = target.parent().index();
-    self.handleChange();
+    storeGraphOptionsInURL();
     if ($("#" + target.attr("aria-controls")).hasClass("reload")) {
       self.submitQuery();
     }
@@ -210,6 +208,10 @@ Prometheus.Graph.prototype.populateInsertableMetrics = function() {
   });
 };
 
+Prometheus.Graph.prototype.onChange = function(handler) {
+  this.changeHandler = handler;
+};
+
 Prometheus.Graph.prototype.getOptions = function() {
   var self = this;
   var options = {};
@@ -542,7 +544,7 @@ Prometheus.Graph.prototype.updateGraph = function() {
     legend: legend
   });
 
-  self.handleChange();
+  self.changeHandler();
 };
 
 Prometheus.Graph.prototype.resizeGraph = function() {
@@ -620,10 +622,41 @@ Prometheus.Graph.prototype.handleConsoleResponse = function(data, textStatus) {
 Prometheus.Graph.prototype.remove = function() {
   var self = this;
   $(self.graphHTML).remove();
-  self.handleRemove();
+  graphs = graphs.filter(function(e) {return e !== self});
-  self.handleChange();
+  storeGraphOptionsInURL();
 };
 
+function parseGraphOptionsFromURL() {
+  var hashOptions = window.location.hash.slice(1);
+  if (!hashOptions) {
+    return [];
+  }
+  var optionsJSON = decodeURIComponent(window.location.hash.slice(1));
+  options = JSON.parse(optionsJSON);
+  return options;
+}
+
+// NOTE: This needs to be kept in sync with rules/helpers.go:GraphLinkForExpression!
+function storeGraphOptionsInURL() {
+  var allGraphsOptions = [];
+  for (var i = 0; i < graphs.length; i++) {
+    allGraphsOptions.push(graphs[i].getOptions());
+  }
+  var optionsJSON = JSON.stringify(allGraphsOptions);
+  window.location.hash = encodeURIComponent(optionsJSON);
+}
+
+function addGraph(options) {
+  var graph = new Prometheus.Graph($("#graph_container"), options);
+  graphs.push(graph);
+  graph.onChange(function() {
+    storeGraphOptionsInURL();
+  });
+  $(window).resize(function() {
+    graph.resizeGraph();
+  });
+}
+
 function escapeHTML(string) {
   var entityMap = {
     "&": "&amp;",
@@ -639,135 +672,6 @@ function escapeHTML(string) {
   });
 }
 
-Prometheus.Page = function() {
-  this.graphs = [];
-};
-
-Prometheus.Page.prototype.init = function() {
-  var graphOptions = this.parseURL();
-  if (graphOptions.length === 0) {
-    graphOptions.push({});
-  }
-
-  graphOptions.forEach(this.addGraph, this);
-
-  $("#add_graph").click(this.addGraph.bind(this, {}));
-};
-
-Prometheus.Page.prototype.parseURL = function() {
-  if (window.location.search == "") {
-    return [];
-  }
-
-  var queryParams = window.location.search.substring(1).split('&');
-  var queryParamHelper = new Prometheus.Page.QueryParamHelper();
-  return queryParamHelper.parseQueryParams(queryParams);
-};
-
-Prometheus.Page.prototype.addGraph = function(options) {
-  var graph = new Prometheus.Graph(
-    $("#graph_container"),
-    options,
-    this.updateURL.bind(this),
-    this.removeGraph.bind(this)
-  );
-
-  this.graphs.push(graph);
-  $(window).resize(function() {
-    graph.resizeGraph();
-  });
-};
-
-// NOTE: This needs to be kept in sync with /util/strutil/strconv.go:GraphLinkForExpression
-Prometheus.Page.prototype.updateURL = function() {
-  var queryString = this.graphs.map(function(graph, index) {
-    var graphOptions = graph.getOptions();
-    var queryParamHelper = new Prometheus.Page.QueryParamHelper();
-    var queryObject = queryParamHelper.generateQueryObject(graphOptions, index);
-    return $.param(queryObject);
-  }, this).join("&");
-
-  history.pushState({}, "", "graph?" + queryString);
-};
-
-Prometheus.Page.prototype.removeGraph = function(graph) {
-  this.graphs = this.graphs.filter(function(g) {return g !== graph});
-};
-
-Prometheus.Page.QueryParamHelper = function() {};
-
-Prometheus.Page.QueryParamHelper.prototype.parseQueryParams = function(queryParams) {
-  var orderedQueryParams = this.filterInvalidParams(queryParams).sort();
-  return this.fetchOptionsFromOrderedParams(orderedQueryParams, 0);
-};
-
-Prometheus.Page.QueryParamHelper.queryParamFormat = /^g\d+\..+=.+$/;
-
-Prometheus.Page.QueryParamHelper.prototype.filterInvalidParams = function(paramTuples) {
-  return paramTuples.filter(function(paramTuple) {
-    return Prometheus.Page.QueryParamHelper.queryParamFormat.test(paramTuple);
-  });
-};
-
-Prometheus.Page.QueryParamHelper.prototype.fetchOptionsFromOrderedParams = function(queryParams, graphIndex) {
-  if (queryParams.length == 0) {
-    return [];
-  }
-
-  var prefixOfThisIndex = this.queryParamPrefix(graphIndex);
-  var numberOfParamsForThisGraph = queryParams.filter(function(paramTuple) {
-    return paramTuple.startsWith(prefixOfThisIndex);
-  }).length;
-
-  if (numberOfParamsForThisGraph == 0) {
-    return [];
-  }
-
-  var paramsForThisGraph = queryParams.splice(0, numberOfParamsForThisGraph);
-
-  paramsForThisGraph = paramsForThisGraph.map(function(paramTuple) {
-    return paramTuple.substring(prefixOfThisIndex.length);
-  });
-
-  var options = this.parseQueryParamsOfOneGraph(paramsForThisGraph);
-  var optionAccumulator = this.fetchOptionsFromOrderedParams(queryParams, graphIndex + 1);
-  optionAccumulator.unshift(options);
-
-  return optionAccumulator;
-};
-
-Prometheus.Page.QueryParamHelper.prototype.parseQueryParamsOfOneGraph = function(queryParams) {
-  var options = {};
-  queryParams.forEach(function(tuple) {
-    var optionNameAndValue = tuple.split('=');
-    var optionName = optionNameAndValue[0];
-    var optionValue = decodeURIComponent(optionNameAndValue[1]);
-
-    optionValue = optionValue.replace(/\+/g, " "); // $.param turns spaces into pluses
-
-    if (optionName == "tab") {
-      optionValue = parseInt(optionValue); // tab is integer
-    }
-
-    options[optionName] = optionValue;
-  });
-
-  return options;
-};
-
-Prometheus.Page.QueryParamHelper.prototype.queryParamPrefix = function(index) {
-  return "g" + index + ".";
-};
-
-Prometheus.Page.QueryParamHelper.prototype.generateQueryObject = function(graphOptions, index) {
-  var prefix = this.queryParamPrefix(index);
-  var queryObject = {};
-  Object.keys(graphOptions).forEach(function(key) {
-    queryObject[prefix + key] = graphOptions[key];
-  });
-  return queryObject;
-};
-
 function init() {
   $.ajaxSetup({
     cache: false
@@ -777,8 +681,14 @@ function init() {
     url: PATH_PREFIX + "/static/js/graph_template.handlebar",
     success: function(data) {
       graphTemplate = Handlebars.compile(data);
-      var Page = new Prometheus.Page();
+      var options = parseGraphOptionsFromURL();
-      Page.init();
+      if (options.length === 0) {
+        options.push({});
+      }
+      for (var i = 0; i < options.length; i++) {
+        addGraph(options[i]);
+      }
+      $("#add_graph").click(function() { addGraph({}); });
     }
   });
 }