Repository: prometheus/prometheus
Pull request #7256 (2020-05-13 16:05:26)

API: Allow AlertmanagerRetriever and RulesRetriever to receive a Context
This is a follow up from #7125 and expands on the discussion #7103.

Initially, #7125 only took into account `TargetRetriever` but the same can be applied to other dependencies. Given this pattern is quite unobtrusive and only belongs in the API, I hope this is better suited than #4999

cc: @brian-brazil 

Signed-off-by: gotjosh <josue@grafana.com>


Patch:
diff --git a/Makefile.common b/Makefile.common
index b978dfc50..9320176ca 100644
--- a/Makefile.common
+++ b/Makefile.common
@@ -150,6 +150,17 @@ else
 	$(GO) get $(GOOPTS) -t ./...
 endif
 
+.PHONY: update-go-deps
+update-go-deps:
+	@echo ">> updating Go dependencies"
+	@for m in $$($(GO) list -mod=readonly -m -f '{{ if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all); do \
+		$(GO) get $$m; \
+	done
+	GO111MODULE=$(GO111MODULE) $(GO) mod tidy
+ifneq (,$(wildcard vendor))
+	GO111MODULE=$(GO111MODULE) $(GO) mod vendor
+endif
+
 .PHONY: common-test-short
 common-test-short: $(GOTEST_DIR)
 	@echo ">> running short tests"
diff --git a/RELEASE.md b/RELEASE.md
index 1e49b36c9..f40a925e2 100644
--- a/RELEASE.md
+++ b/RELEASE.md
@@ -61,11 +61,9 @@ Maintaining the release branches for older minor releases happens on a best effo
 A few days before a major or minor release, consider updating the dependencies:
 
 ```
-export GO111MODULE=on
-go get -u ./...
-go mod tidy
-go mod vendor
+make update-go-deps
 git add go.mod go.sum vendor
+git commit -m "Update dependencies"
 ```
 
 Then create a pull request against the master branch.
diff --git a/go.mod b/go.mod
index b370bea10..29417f2fc 100644
--- a/go.mod
+++ b/go.mod
@@ -53,7 +53,7 @@ require (
 	github.com/prometheus/alertmanager v0.20.0
 	github.com/prometheus/client_golang v1.6.0
 	github.com/prometheus/client_model v0.2.0
-	github.com/prometheus/common v0.9.1
+	github.com/prometheus/common v0.10.0
 	github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da
 	github.com/shurcooL/httpfs v0.0.0-20190707220628-8d4bc4ba7749
 	github.com/shurcooL/vfsgen v0.0.0-20181202132449-6a9ea43bcacd
diff --git a/go.sum b/go.sum
index 7f1160803..613ccd7cf 100644
--- a/go.sum
+++ b/go.sum
@@ -670,6 +670,8 @@ github.com/prometheus/common v0.7.0 h1:L+1lyG48J1zAQXA3RBX/nG/B3gjlHq0zTt2tlbJLy
 github.com/prometheus/common v0.7.0/go.mod h1:DjGbpBbp5NYNiECxcL/VnbXCCaQpKd3tt26CguLLsqA=
 github.com/prometheus/common v0.9.1 h1:KOMtN28tlbam3/7ZKEYKHhKoJZYYj3gMH4uc62x7X7U=
 github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8bs7vj7HSQ4=
+github.com/prometheus/common v0.10.0 h1:RyRA7RzGXQZiW+tGMr7sxa85G1z0yOpM1qq5c8lNawc=
+github.com/prometheus/common v0.10.0/go.mod h1:Tlit/dnDKsSWFlCLTWaA1cyBgKHSMdTB80sz/V91rCo=
 github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA=
diff --git a/vendor/github.com/prometheus/common/config/http_config.go b/vendor/github.com/prometheus/common/config/http_config.go
index 5c373d7a6..9b1fad93f 100644
--- a/vendor/github.com/prometheus/common/config/http_config.go
+++ b/vendor/github.com/prometheus/common/config/http_config.go
@@ -29,6 +29,7 @@ import (
 	"time"
 
 	"github.com/mwitkow/go-conntrack"
+	"golang.org/x/net/http2"
 	"gopkg.in/yaml.v2"
 )
 
@@ -153,6 +154,11 @@ func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAli
 				conntrack.DialWithName(name),
 			),
 		}
+		// TODO: use ForceAttemptHTTP2 when we move to Go 1.13+.
+		err := http2.ConfigureTransport(rt.(*http.Transport))
+		if err != nil {
+			return nil, err
+		}
 
 		// If a bearer token is provided, create a round tripper that will set the
 		// Authorization header correctly on each request.
diff --git a/vendor/github.com/prometheus/common/model/time.go b/vendor/github.com/prometheus/common/model/time.go
index 7b0064fdb..490a0240c 100644
--- a/vendor/github.com/prometheus/common/model/time.go
+++ b/vendor/github.com/prometheus/common/model/time.go
@@ -186,6 +186,10 @@ var durationRE = regexp.MustCompile("^([0-9]+)(y|w|d|h|m|s|ms)$")
 // ParseDuration parses a string into a time.Duration, assuming that a year
 // always has 365d, a week always has 7d, and a day always has 24h.
 func ParseDuration(durationStr string) (Duration, error) {
+	// Allow 0 without a unit.
+	if durationStr == "0" {
+		return 0, nil
+	}
 	matches := durationRE.FindStringSubmatch(durationStr)
 	if len(matches) != 3 {
 		return 0, fmt.Errorf("not a valid duration string: %q", durationStr)
diff --git a/vendor/github.com/prometheus/common/promlog/log.go b/vendor/github.com/prometheus/common/promlog/log.go
index 45fcbc408..5062e2805 100644
--- a/vendor/github.com/prometheus/common/promlog/log.go
+++ b/vendor/github.com/prometheus/common/promlog/log.go
@@ -94,13 +94,15 @@ type Config struct {
 // with a timestamp. The output always goes to stderr.
 func New(config *Config) log.Logger {
 	var l log.Logger
-	if config.Format.s == "logfmt" {
-		l = log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr))
-	} else {
+	if config.Format != nil && config.Format.s == "json" {
 		l = log.NewJSONLogger(log.NewSyncWriter(os.Stderr))
+	} else {
+		l = log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr))
 	}
 
-	l = level.NewFilter(l, config.Level.o)
+	if config.Level != nil {
+		l = level.NewFilter(l, config.Level.o)
+	}
 	l = log.With(l, "ts", timestampFormat, "caller", log.DefaultCaller)
 	return l
 }
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 58c970f91..829d09113 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -286,7 +286,7 @@ github.com/prometheus/client_golang/prometheus/testutil
 github.com/prometheus/client_golang/prometheus/testutil/promlint
 # github.com/prometheus/client_model v0.2.0
 github.com/prometheus/client_model/go
-# github.com/prometheus/common v0.9.1
+# github.com/prometheus/common v0.10.0
 github.com/prometheus/common/config
 github.com/prometheus/common/expfmt
 github.com/prometheus/common/internal/bitbucket.org/ww/goautoneg
diff --git a/web/api/v1/api.go b/web/api/v1/api.go
index a0ae68527..62bca6d78 100644
--- a/web/api/v1/api.go
+++ b/web/api/v1/api.go
@@ -105,12 +105,14 @@ type TargetRetriever interface {
 	TargetsDropped() map[string][]*scrape.Target
 }
 
-type alertmanagerRetriever interface {
+// AlertmanagerRetriever provides a list of all/dropped AlertManager URLs.
+type AlertmanagerRetriever interface {
 	Alertmanagers() []*url.URL
 	DroppedAlertmanagers() []*url.URL
 }
 
-type rulesRetriever interface {
+// RulesRetriever provides a list of active rules and alerts.
+type RulesRetriever interface {
 	RuleGroups() []*rules.Group
 	AlertingRules() []*rules.AlertingRule
 }
@@ -174,8 +176,8 @@ type API struct {
 	QueryEngine *promql.Engine
 
 	targetRetriever       func(context.Context) TargetRetriever
-	alertmanagerRetriever alertmanagerRetriever
-	rulesRetriever        rulesRetriever
+	alertmanagerRetriever func(context.Context) AlertmanagerRetriever
+	rulesRetriever        func(context.Context) RulesRetriever
 	now                   func() time.Time
 	config                func() config.Config
 	flagsMap              map[string]string
@@ -204,7 +206,7 @@ func NewAPI(
 	qe *promql.Engine,
 	q storage.Queryable,
 	tr func(context.Context) TargetRetriever,
-	ar alertmanagerRetriever,
+	ar func(context.Context) AlertmanagerRetriever,
 	configFunc func() config.Config,
 	flagsMap map[string]string,
 	globalURLOptions GlobalURLOptions,
@@ -213,7 +215,7 @@ func NewAPI(
 	dbDir string,
 	enableAdmin bool,
 	logger log.Logger,
-	rr rulesRetriever,
+	rr func(context.Context) RulesRetriever,
 	remoteReadSampleLimit int,
 	remoteReadConcurrencyLimit int,
 	remoteReadMaxBytesInFrame int,
@@ -847,8 +849,8 @@ type AlertmanagerTarget struct {
 }
 
 func (api *API) alertmanagers(r *http.Request) apiFuncResult {
-	urls := api.alertmanagerRetriever.Alertmanagers()
-	droppedURLS := api.alertmanagerRetriever.DroppedAlertmanagers()
+	urls := api.alertmanagerRetriever(r.Context()).Alertmanagers()
+	droppedURLS := api.alertmanagerRetriever(r.Context()).DroppedAlertmanagers()
 	ams := &AlertmanagerDiscovery{ActiveAlertmanagers: make([]*AlertmanagerTarget, len(urls)), DroppedAlertmanagers: make([]*AlertmanagerTarget, len(droppedURLS))}
 	for i, url := range urls {
 		ams.ActiveAlertmanagers[i] = &AlertmanagerTarget{URL: url.String()}
@@ -874,7 +876,7 @@ type Alert struct {
 }
 
 func (api *API) alerts(r *http.Request) apiFuncResult {
-	alertingRules := api.rulesRetriever.AlertingRules()
+	alertingRules := api.rulesRetriever(r.Context()).AlertingRules()
 	alerts := []*Alert{}
 
 	for _, alertingRule := range alertingRules {
@@ -1021,7 +1023,7 @@ type recordingRule struct {
 }
 
 func (api *API) rules(r *http.Request) apiFuncResult {
-	ruleGroups := api.rulesRetriever.RuleGroups()
+	ruleGroups := api.rulesRetriever(r.Context()).RuleGroups()
 	res := &RuleDiscovery{RuleGroups: make([]*RuleGroup, len(ruleGroups))}
 	typeParam := strings.ToLower(r.URL.Query().Get("type"))
 
diff --git a/web/ui/react-app/src/pages/graph/Panel.tsx b/web/ui/react-app/src/pages/graph/Panel.tsx
index 5bd38cf34..ae153fdcb 100644
--- a/web/ui/react-app/src/pages/graph/Panel.tsx
+++ b/web/ui/react-app/src/pages/graph/Panel.tsx
@@ -210,6 +210,10 @@ class Panel extends Component<PanelProps & PathPrefixProps, PanelState> {
   };
 
   handleChangeType = (type: PanelType) => {
+    if (this.props.options.type === type) {
+      return;
+    }
+
     this.setState({ data: null });
     this.setOptions({ type: type });
   };
diff --git a/web/web.go b/web/web.go
index 204bfaf27..1cd308f81 100644
--- a/web/web.go
+++ b/web/web.go
@@ -297,10 +297,11 @@ func New(logger log.Logger, o *Options) *Handler {
 		ready: 0,
 	}
 
-	factoryTr := func(_ context.Context) api_v1.TargetRetriever {
-		return h.scrapeManager
-	}
-	h.apiV1 = api_v1.NewAPI(h.queryEngine, h.storage, factoryTr, h.notifier,
+	factoryTr := func(_ context.Context) api_v1.TargetRetriever { return h.scrapeManager }
+	factoryAr := func(_ context.Context) api_v1.AlertmanagerRetriever { return h.notifier }
+	FactoryRr := func(_ context.Context) api_v1.RulesRetriever { return h.ruleManager }
+
+	h.apiV1 = api_v1.NewAPI(h.queryEngine, h.storage, factoryTr, factoryAr,
 		func() config.Config {
 			h.mtx.RLock()
 			defer h.mtx.RUnlock()
@@ -317,7 +318,7 @@ func New(logger log.Logger, o *Options) *Handler {
 		h.options.TSDBDir,
 		h.options.EnableAdminAPI,
 		logger,
-		h.ruleManager,
+		FactoryRr,
 		h.options.RemoteReadSampleLimit,
 		h.options.RemoteReadConcurrencyLimit,
 		h.options.RemoteReadBytesInFrame,


Test patch:
diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go
index d693feb4e..a27f818ad 100644
--- a/web/api/v1/api_test.go
+++ b/web/api/v1/api_test.go
@@ -164,9 +164,7 @@ func (t *testTargetRetriever) ResetMetadataStore() {
 }
 
 func (t *testTargetRetriever) toFactory() func(context.Context) TargetRetriever {
-	return func(context.Context) TargetRetriever {
-		return t
-	}
+	return func(context.Context) TargetRetriever { return t }
 }
 
 type testAlertmanagerRetriever struct{}
@@ -191,6 +189,10 @@ func (t testAlertmanagerRetriever) DroppedAlertmanagers() []*url.URL {
 	}
 }
 
+func (t testAlertmanagerRetriever) toFactory() func(context.Context) AlertmanagerRetriever {
+	return func(context.Context) AlertmanagerRetriever { return t }
+}
+
 type rulesRetrieverMock struct {
 	testing *testing.T
 }
@@ -276,6 +278,10 @@ func (m rulesRetrieverMock) RuleGroups() []*rules.Group {
 	return []*rules.Group{group}
 }
 
+func (m rulesRetrieverMock) toFactory() func(context.Context) RulesRetriever {
+	return func(context.Context) RulesRetriever { return m }
+}
+
 var samplePrometheusCfg = config.Config{
 	GlobalConfig:       config.GlobalConfig{},
 	AlertingConfig:     config.AlertingConfig{},
@@ -318,12 +324,12 @@ func TestEndpoints(t *testing.T) {
 			Queryable:             suite.Storage(),
 			QueryEngine:           suite.QueryEngine(),
 			targetRetriever:       testTargetRetriever.toFactory(),
-			alertmanagerRetriever: testAlertmanagerRetriever{},
+			alertmanagerRetriever: testAlertmanagerRetriever{}.toFactory(),
 			flagsMap:              sampleFlagMap,
 			now:                   func() time.Time { return now },
 			config:                func() config.Config { return samplePrometheusCfg },
 			ready:                 func(f http.HandlerFunc) http.HandlerFunc { return f },
-			rulesRetriever:        algr,
+			rulesRetriever:        algr.toFactory(),
 		}
 
 		testEndpoints(t, api, testTargetRetriever, true)
@@ -382,12 +388,12 @@ func TestEndpoints(t *testing.T) {
 			Queryable:             remote,
 			QueryEngine:           suite.QueryEngine(),
 			targetRetriever:       testTargetRetriever.toFactory(),
-			alertmanagerRetriever: testAlertmanagerRetriever{},
+			alertmanagerRetriever: testAlertmanagerRetriever{}.toFactory(),
 			flagsMap:              sampleFlagMap,
 			now:                   func() time.Time { return now },
 			config:                func() config.Config { return samplePrometheusCfg },
 			ready:                 func(f http.HandlerFunc) http.HandlerFunc { return f },
-			rulesRetriever:        algr,
+			rulesRetriever:        algr.toFactory(),
 		}
 
 		testEndpoints(t, api, testTargetRetriever, false)
diff --git a/web/ui/react-app/src/pages/graph/Panel.test.tsx b/web/ui/react-app/src/pages/graph/Panel.test.tsx
index 2b07f7edc..3fc207272 100644
--- a/web/ui/react-app/src/pages/graph/Panel.test.tsx
+++ b/web/ui/react-app/src/pages/graph/Panel.test.tsx
@@ -65,9 +65,13 @@ describe('Panel', () => {
       const className = tc.active ? 'active' : '';
       expect(link.prop('className')).toEqual(className);
       link.simulate('click');
-      expect(results).toHaveLength(1);
-      expect(results[0].type).toEqual(tc.panelType.toLowerCase());
-      results.pop();
+      if (tc.active) {
+        expect(results).toHaveLength(0);
+      } else {
+        expect(results).toHaveLength(1);
+        expect(results[0].type).toEqual(tc.panelType.toLowerCase());
+        results.pop();
+      }
     });
   });
 
@@ -120,6 +124,23 @@ describe('Panel', () => {
     });
   });
 
+  describe('when clicking on current mode', () => {
+    [PanelType.Table, PanelType.Graph].forEach((mode: PanelType) => {
+      it(`${mode} keeps data`, () => {
+        const props = {
+          ...defaultProps,
+          options: { ...defaultProps.options, type: mode },
+        };
+        const panel = shallow(<Panel {...props} />);
+        const instance: any = panel.instance();
+        panel.setState({ data: 'somedata' });
+        expect(panel.state('data')).toEqual('somedata');
+        instance.handleChangeType(mode);
+        expect(panel.state('data')).toEqual('somedata');
+      });
+    });
+  });
+
   describe('when changing query then time', () => {
     it('executes the new query', () => {
       const initialExpr = 'time()';