diff --git a/monitoring/definitions/executor.go b/monitoring/definitions/executor.go index 346364647c8f..b196da9fa811 100644 --- a/monitoring/definitions/executor.go +++ b/monitoring/definitions/executor.go @@ -22,9 +22,13 @@ func Executor() *monitoring.Container { Options: []string{"batches", "codeintel"}, }, { - Label: "Compute instance", - Name: "instance", - Query: "label_values(node_exporter_build_info{job=\"sourcegraph-executor-nodes\"}, instance)", + Label: "Compute instance", + Name: "instance", + OptionsQuery: "label_values(node_exporter_build_info{job=\"sourcegraph-executor-nodes\"}, instance)", + + // The options query can generate a massive result set that can cause issues. + // shared.NewNodeExporterGroup filters by job as well so this is safe to use + WildcardAllValue: true, }, }, Groups: []monitoring.Group{ diff --git a/monitoring/definitions/git_server.go b/monitoring/definitions/git_server.go index 80e85edcf17c..c2c5987417b0 100644 --- a/monitoring/definitions/git_server.go +++ b/monitoring/definitions/git_server.go @@ -27,10 +27,10 @@ func GitServer() *monitoring.Container { Description: "Stores, manages, and operates Git repositories.", Variables: []monitoring.ContainerVariable{ { - Label: "Shard", - Name: "shard", - Query: "label_values(src_gitserver_exec_running, instance)", - Multi: true, + Label: "Shard", + Name: "shard", + OptionsQuery: "label_values(src_gitserver_exec_running, instance)", + Multi: true, }, }, Groups: []monitoring.Group{ diff --git a/monitoring/definitions/zoekt.go b/monitoring/definitions/zoekt.go index 6814852658e6..fa293eea691d 100644 --- a/monitoring/definitions/zoekt.go +++ b/monitoring/definitions/zoekt.go @@ -24,10 +24,10 @@ func Zoekt() *monitoring.Container { NoSourcegraphDebugServer: true, Variables: []monitoring.ContainerVariable{ { - Label: "Instance", - Name: "instance", - Query: "label_values(index_num_assigned, instance)", - Multi: true, + Label: "Instance", + Name: "instance", + OptionsQuery: "label_values(index_num_assigned, instance)", + Multi: true, }, }, Groups: []monitoring.Group{ diff --git a/monitoring/monitoring/monitoring.go b/monitoring/monitoring/monitoring.go index 62c8d652536b..e386aeab62b4 100644 --- a/monitoring/monitoring/monitoring.go +++ b/monitoring/monitoring/monitoring.go @@ -347,13 +347,25 @@ type ContainerVariable struct { // Label is a human-readable name for the variable, e.g. "Alert level" Label string - // Query is the query to generate the possible values. Cannot be used in conjunction - // with Options - Query string + // OptionsQuery is the query to generate the possible values for this variable. Cannot + // be used in conjunction with Options + OptionsQuery string // Options are the pre-defined possible values for this variable. Cannot be used in // conjunction with Query Options []string + // WildcardAllValue indicates to Grafana that is should NOT use Query or Options to + // generate a concatonated 'All' value for the variable, and use a '.*' wildcard + // instead. Setting this to true primarily useful if you use Query and expect it to be + // a large enough result set to cause issues when viewing the dashboard. + // + // We allow Grafana to generate a value by default because simply using '.*' wildcard + // can pull in unintended metrics if adequate filtering is not performed on the query, + // for example if multiple services export the same metric. If set to true, make sure + // the queries that use this variable perform adequate filtering to avoid pulling in + // unintended metrics. + WildcardAllValue bool + // Multi indicates whether or not to allow multi-selection for this variable filter Multi bool } @@ -365,7 +377,7 @@ func (c *ContainerVariable) validate() error { if c.Label == "" { return errors.New("ContainerVariable.Label is required") } - if c.Query == "" && len(c.Options) == 0 { + if c.OptionsQuery == "" && len(c.Options) == 0 { return errors.New("ContainerVariable.Query and ContainerVariable.Options cannot both be set") } return nil @@ -382,29 +394,31 @@ func (c *ContainerVariable) toGrafanaTemplateVar() sdk.TemplateVar { Datasource: StringPtr("Prometheus"), IncludeAll: true, - AllValue: ".*", // Apply the AllValue to a template variable by default Current: sdk.Current{Text: &sdk.StringSliceString{Value: []string{"all"}, Valid: true}, Value: "$__all"}, } + if c.WildcardAllValue { + variable.AllValue = ".*" + } else { + // Rely on Grafana to create a union of only the values + // generated by the specified query. + // + // See https://grafana.com/docs/grafana/latest/variables/formatting-multi-value-variables/#multi-value-variables-with-a-prometheus-or-influxdb-data-source + // for more information. + variable.AllValue = "" + } + switch { - case c.Query != "": + case c.OptionsQuery != "": variable.Type = "query" - variable.Query = c.Query + variable.Query = c.OptionsQuery variable.Refresh = sdk.BoolInt{ Flag: true, Value: Int64Ptr(2), // Refresh on time range change } variable.Sort = 3 - // Rely on Grafana to create a union of only the values - // generated by the specified query. Otherwise, the '.*' - // wildcard can pull in unintended metrics. - // - // See https://grafana.com/docs/grafana/latest/variables/formatting-multi-value-variables/#multi-value-variables-with-a-prometheus-or-influxdb-data-source - // for more information. - variable.AllValue = "" - case len(c.Options) > 0: variable.Type = "custom" variable.Query = strings.Join(c.Options, ",")