-
Notifications
You must be signed in to change notification settings - Fork 850
Add per-tenant Grafana Explore URL format for alert GeneratorURL #7302
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,7 @@ import ( | |||||||
| "github.com/prometheus/prometheus/promql" | ||||||||
| "github.com/prometheus/prometheus/rules" | ||||||||
| "github.com/prometheus/prometheus/storage" | ||||||||
| "github.com/prometheus/prometheus/util/strutil" | ||||||||
| "github.com/weaveworks/common/httpgrpc" | ||||||||
| "github.com/weaveworks/common/user" | ||||||||
|
|
||||||||
|
|
@@ -164,6 +165,10 @@ type RulesLimits interface { | |||||||
| RulerQueryOffset(userID string) time.Duration | ||||||||
| DisabledRuleGroups(userID string) validation.DisabledRuleGroups | ||||||||
| RulerExternalLabels(userID string) labels.Labels | ||||||||
| RulerExternalURL(userID string) string | ||||||||
| RulerAlertGeneratorURLFormat(userID string) string | ||||||||
| RulerGrafanaDatasourceUID(userID string) string | ||||||||
| RulerGrafanaOrgID(userID string) int64 | ||||||||
| } | ||||||||
|
|
||||||||
| type QueryExecutor func(ctx context.Context, qs string, t time.Time) (promql.Vector, error) | ||||||||
|
|
@@ -369,11 +374,25 @@ func DefaultTenantManagerFactory(cfg Config, p Pusher, q storage.Queryable, engi | |||||||
| Appendable: NewPusherAppendable(p, userID, overrides, | ||||||||
| evalMetrics.TotalWritesVec.WithLabelValues(userID), | ||||||||
| evalMetrics.FailedWritesVec.WithLabelValues(userID)), | ||||||||
| Queryable: q, | ||||||||
| QueryFunc: queryFunc, | ||||||||
| Context: prometheusContext, | ||||||||
| ExternalURL: cfg.ExternalURL.URL, | ||||||||
| NotifyFunc: SendAlerts(notifier, cfg.ExternalURL.URL.String()), | ||||||||
| Queryable: q, | ||||||||
| QueryFunc: queryFunc, | ||||||||
| Context: prometheusContext, | ||||||||
| ExternalURL: cfg.ExternalURL.URL, | ||||||||
| NotifyFunc: SendAlerts(notifier, func(expr string) string { | ||||||||
| externalURL := cfg.ExternalURL.String() | ||||||||
| if tenantURL := overrides.RulerExternalURL(userID); tenantURL != "" { | ||||||||
| externalURL = tenantURL | ||||||||
| } | ||||||||
| if overrides.RulerAlertGeneratorURLFormat(userID) == "grafana-explore" { | ||||||||
| datasourceUID := overrides.RulerGrafanaDatasourceUID(userID) | ||||||||
| orgID := overrides.RulerGrafanaOrgID(userID) | ||||||||
| if orgID == 0 { | ||||||||
| orgID = 1 | ||||||||
| } | ||||||||
|
Comment on lines
+389
to
+391
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Should not be needed after my other comment |
||||||||
| return grafanaExploreLink(externalURL, expr, datasourceUID, orgID) | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think is better we make grafanaExploreLink return errors if the url cannot be generated. For example: empty datasource |
||||||||
| } | ||||||||
| return externalURL + strutil.TableLinkForExpression(expr) | ||||||||
| }), | ||||||||
| Logger: util_log.GoKitLogToSlog(log.With(logger, "user", userID)), | ||||||||
| Registerer: reg, | ||||||||
| OutageTolerance: cfg.OutageTolerance, | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package ruler | ||
|
|
||
| import ( | ||
| "sync" | ||
| ) | ||
|
|
||
| // userExternalURL tracks per-user resolved external URLs and detects changes. | ||
| type userExternalURL struct { | ||
| global string | ||
| limits RulesLimits | ||
|
|
||
| mtx sync.Mutex | ||
| users map[string]string | ||
| } | ||
|
|
||
| func newUserExternalURL(global string, limits RulesLimits) *userExternalURL { | ||
| return &userExternalURL{ | ||
| global: global, | ||
| limits: limits, | ||
|
|
||
| mtx: sync.Mutex{}, | ||
| users: map[string]string{}, | ||
| } | ||
| } | ||
|
|
||
| func (e *userExternalURL) update(userID string) (string, bool) { | ||
| tenantURL := e.limits.RulerExternalURL(userID) | ||
| resolved := e.global | ||
| if tenantURL != "" { | ||
| resolved = tenantURL | ||
| } | ||
|
|
||
| e.mtx.Lock() | ||
| defer e.mtx.Unlock() | ||
|
|
||
| if prev, ok := e.users[userID]; ok && prev == resolved { | ||
| return resolved, false | ||
| } | ||
|
|
||
| e.users[userID] = resolved | ||
| return resolved, true | ||
| } | ||
|
|
||
| func (e *userExternalURL) remove(user string) { | ||
| e.mtx.Lock() | ||
| defer e.mtx.Unlock() | ||
| delete(e.users, user) | ||
| } | ||
|
|
||
| func (e *userExternalURL) cleanup() { | ||
| e.mtx.Lock() | ||
| defer e.mtx.Unlock() | ||
| for user := range e.users { | ||
| delete(e.users, user) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package ruler | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestUserExternalURL(t *testing.T) { | ||
| limits := ruleLimits{} | ||
| e := newUserExternalURL("http://global:9090", &limits) | ||
|
|
||
| const userID = "test-user" | ||
|
|
||
| t.Run("global URL used when no per-tenant override", func(t *testing.T) { | ||
| e.remove(userID) | ||
| url, changed := e.update(userID) | ||
| require.True(t, changed) | ||
| require.Equal(t, "http://global:9090", url) | ||
| }) | ||
|
|
||
| t.Run("no change on second update", func(t *testing.T) { | ||
| url, changed := e.update(userID) | ||
| require.False(t, changed) | ||
| require.Equal(t, "http://global:9090", url) | ||
| }) | ||
|
|
||
| t.Run("per-tenant URL overrides global", func(t *testing.T) { | ||
| limits.mtx.Lock() | ||
| limits.externalURL = "http://tenant:3000" | ||
| limits.mtx.Unlock() | ||
|
|
||
| url, changed := e.update(userID) | ||
| require.True(t, changed) | ||
| require.Equal(t, "http://tenant:3000", url) | ||
| }) | ||
|
|
||
| t.Run("no change when per-tenant URL is the same", func(t *testing.T) { | ||
| url, changed := e.update(userID) | ||
| require.False(t, changed) | ||
| require.Equal(t, "http://tenant:3000", url) | ||
| }) | ||
|
|
||
| t.Run("revert to global when per-tenant override removed", func(t *testing.T) { | ||
| limits.mtx.Lock() | ||
| limits.externalURL = "" | ||
| limits.mtx.Unlock() | ||
|
|
||
| url, changed := e.update(userID) | ||
| require.True(t, changed) | ||
| require.Equal(t, "http://global:9090", url) | ||
| }) | ||
|
|
||
| t.Run("remove and cleanup lifecycle", func(t *testing.T) { | ||
| e.remove(userID) | ||
| // After remove, next update should report changed | ||
| url, changed := e.update(userID) | ||
| require.True(t, changed) | ||
| require.Equal(t, "http://global:9090", url) | ||
|
|
||
| e.cleanup() | ||
| // After cleanup, next update should report changed | ||
| url, changed = e.update(userID) | ||
| require.True(t, changed) | ||
| require.Equal(t, "http://global:9090", url) | ||
| }) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ package ruler | |
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "flag" | ||
| "fmt" | ||
| "hash/fnv" | ||
|
|
@@ -26,7 +27,6 @@ import ( | |
| "github.com/prometheus/prometheus/notifier" | ||
| "github.com/prometheus/prometheus/promql/parser" | ||
| promRules "github.com/prometheus/prometheus/rules" | ||
| "github.com/prometheus/prometheus/util/strutil" | ||
| "github.com/weaveworks/common/user" | ||
| "golang.org/x/sync/errgroup" | ||
|
|
||
|
|
@@ -506,7 +506,7 @@ type sender interface { | |
| // It filters any non-firing alerts from the input. | ||
| // | ||
| // Copied from Prometheus's main.go. | ||
| func SendAlerts(n sender, externalURL string) promRules.NotifyFunc { | ||
| func SendAlerts(n sender, generatorURLFn func(expr string) string) promRules.NotifyFunc { | ||
| return func(ctx context.Context, expr string, alerts ...*promRules.Alert) { | ||
| var res []*notifier.Alert | ||
|
|
||
|
|
@@ -515,7 +515,7 @@ func SendAlerts(n sender, externalURL string) promRules.NotifyFunc { | |
| StartsAt: alert.FiredAt, | ||
| Labels: alert.Labels, | ||
| Annotations: alert.Annotations, | ||
| GeneratorURL: externalURL + strutil.TableLinkForExpression(expr), | ||
| GeneratorURL: generatorURLFn(expr), | ||
| } | ||
| if !alert.ResolvedAt.IsZero() { | ||
| a.EndsAt = alert.ResolvedAt | ||
|
|
@@ -531,6 +531,34 @@ func SendAlerts(n sender, externalURL string) promRules.NotifyFunc { | |
| } | ||
| } | ||
|
|
||
| // grafanaExploreLink builds a Grafana Explore URL for the given expression. | ||
| func grafanaExploreLink(baseURL, expr, datasourceUID string, orgID int64) string { | ||
| panes := map[string]any{ | ||
| "default": map[string]any{ | ||
| "datasource": datasourceUID, | ||
| "queries": []map[string]any{ | ||
| { | ||
| "refId": "A", | ||
| "expr": expr, | ||
| "datasource": map[string]string{"uid": datasourceUID, "type": "prometheus"}, | ||
| "editorMode": "code", | ||
| }, | ||
| }, | ||
| "range": map[string]string{ | ||
| "from": "now-1h", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks like it should be configurable. Maybe there is a way to configure the whole pane as a configurable json. At the end of the day, we don't care for this |
||
| "to": "now", | ||
| }, | ||
| }, | ||
| } | ||
| panesJSON, _ := json.Marshal(panes) | ||
|
|
||
| return fmt.Sprintf("%s/explore?schemaVersion=1&panes=%s&orgId=%d", | ||
| strings.TrimRight(baseURL, "/"), | ||
| url.QueryEscape(string(panesJSON)), | ||
| orgID, | ||
| ) | ||
| } | ||
|
|
||
| func ruleGroupDisabled(ruleGroup *rulespb.RuleGroupDesc, disabledRuleGroupsForUser validation.DisabledRuleGroups) bool { | ||
| for _, disabledRuleGroupForUser := range disabledRuleGroupsForUser { | ||
| if ruleGroup.Namespace == disabledRuleGroupForUser.Namespace && | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not clear looking at this configuration that empty is not valid.