From e53e6e7caa29b8caa7bf30c40d662e8c39a9d08d Mon Sep 17 00:00:00 2001 From: Drew Slobodnjak <60050885+drew08t@users.noreply.github.com> Date: Fri, 14 Jun 2024 10:09:47 -0700 Subject: [PATCH 01/47] Geomap: Fix data fit (#89247) --- public/app/plugins/panel/geomap/utils/getLayersExtent.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/public/app/plugins/panel/geomap/utils/getLayersExtent.ts b/public/app/plugins/panel/geomap/utils/getLayersExtent.ts index 300808964b2..425009cbec1 100644 --- a/public/app/plugins/panel/geomap/utils/getLayersExtent.ts +++ b/public/app/plugins/panel/geomap/utils/getLayersExtent.ts @@ -1,6 +1,7 @@ import { createEmpty, extend, Extent } from 'ol/extent'; import LayerGroup from 'ol/layer/Group'; import VectorLayer from 'ol/layer/Vector'; +import VectorImage from 'ol/layer/VectorImage'; import { MapLayerState } from '../types'; @@ -11,12 +12,12 @@ export function getLayersExtent( layer: string | undefined ): Extent { return layers - .filter((l) => l.layer instanceof VectorLayer || l.layer instanceof LayerGroup) + .filter((l) => l.layer instanceof VectorLayer || l.layer instanceof LayerGroup || l.layer instanceof VectorImage) .flatMap((ll) => { const l = ll.layer; if (l instanceof LayerGroup) { return getLayerGroupExtent(l); - } else if (l instanceof VectorLayer) { + } else if (l instanceof VectorLayer || l instanceof VectorImage) { if (allLayers) { // Return everything from all layers return [l.getSource().getExtent()] ?? []; From 6262c56132bc5a2c20f2f6d5890a6a56a3d016f5 Mon Sep 17 00:00:00 2001 From: Dave Henderson Date: Fri, 14 Jun 2024 14:16:36 -0400 Subject: [PATCH 02/47] chore(perf): Pre-allocate where possible (enable prealloc linter) (#88952) * chore(perf): Pre-allocate where possible (enable prealloc linter) Signed-off-by: Dave Henderson * fix TestAlertManagers_buildRedactedAMs Signed-off-by: Dave Henderson * prealloc a slice that appeared after rebase Signed-off-by: Dave Henderson --------- Signed-off-by: Dave Henderson --- .golangci.toml | 15 +++--- pkg/build/cmd/grafanacom.go | 8 +-- pkg/expr/mathexp/exp.go | 8 +-- pkg/infra/log/log.go | 3 +- pkg/modules/modules.go | 2 +- .../angularinspector/angularinspector_test.go | 2 + .../manager/signature/manifest_test.go | 4 +- .../manager/sources/source_local_disk.go | 7 +-- .../manager/sources/source_local_disk_test.go | 2 +- pkg/plugins/manager/sources/sources.go | 11 ++-- pkg/plugins/repo/service_test.go | 8 +-- .../ossaccesscontrol/permissions_services.go | 2 +- .../cloudmigrationimpl/cloudmigration.go | 50 ++++++++++-------- pkg/services/live/live.go | 7 ++- pkg/services/live/runstream/manager.go | 8 ++- pkg/services/ngalert/api/api_prometheus.go | 2 +- .../ngalert/api/compat_contact_points.go | 52 +++++++++++-------- .../ngalert/provisioning/contactpoints.go | 7 +-- .../ngalert/provisioning/templates.go | 4 +- pkg/services/ngalert/remote/alertmanager.go | 9 ++-- pkg/services/ngalert/sender/router.go | 12 +++-- pkg/services/ngalert/sender/router_test.go | 3 +- pkg/services/ngalert/state/compat.go | 2 +- pkg/services/ngalert/store/alert_rule.go | 4 +- .../service/dashboard_updater_test.go | 8 +-- .../pluginconfig/envvars.go | 10 +++- pkg/services/searchV2/bluge.go | 6 ++- pkg/services/searchV2/ngram_test.go | 4 +- .../migrations/external_alertmanagers.go | 5 +- .../sqlstore/permissions/dashboard.go | 6 ++- pkg/services/ssosettings/api/api.go | 2 +- pkg/setting/setting.go | 8 ++- pkg/setting/setting_secure_socks_proxy.go | 2 +- .../azure-resource-graph-datasource.go | 10 ++-- pkg/tsdb/azuremonitor/types/types.go | 9 ++-- pkg/tsdb/cloudwatch/services/utils.go | 2 +- pkg/tsdb/elasticsearch/querydata_test.go | 10 ++-- pkg/tsdb/elasticsearch/response_parser.go | 8 +-- pkg/tsdb/influxdb/fsql/arrow_test.go | 2 +- .../influxql/buffered/response_parser.go | 2 +- pkg/tsdb/tempo/trace_transform_test.go | 7 +-- pkg/util/strings.go | 8 +-- pkg/util/tls.go | 10 ++-- 43 files changed, 211 insertions(+), 140 deletions(-) diff --git a/.golangci.toml b/.golangci.toml index 7c8d17a625e..55c0e4573e2 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -129,14 +129,20 @@ max-func-lines = 60 [linters] disable-all = true +# try to keep this list sorted, please enable = [ + "asciicheck", "bodyclose", "depguard", "dogsled", "errcheck", + "errorlint", + "exhaustive", + "exportloopref", # "gochecknoinits", # "goconst", # "gocritic", # Temporarily disabled on 2022-09-09, running into weird bug "ruleguard: execution error: used Run() with an empty rule set; forgot to call Load() first?" + "gocyclo", "goimports", "goprintffuncname", "gosec", @@ -145,19 +151,14 @@ enable = [ "ineffassign", "misspell", "nakedret", - "exportloopref", + "prealloc", + "revive", "staticcheck", "stylecheck", "typecheck", "unconvert", "unused", "whitespace", - "gocyclo", - "exhaustive", - "typecheck", - "asciicheck", - "errorlint", - "revive", ] # Disabled linters (might want them later) diff --git a/pkg/build/cmd/grafanacom.go b/pkg/build/cmd/grafanacom.go index 9d44b9298c9..0627effcefa 100644 --- a/pkg/build/cmd/grafanacom.go +++ b/pkg/build/cmd/grafanacom.go @@ -156,8 +156,8 @@ func publishPackages(cfg packaging.PublishConfig) error { pth = path.Join(pth, product) baseArchiveURL := fmt.Sprintf("https://dl.grafana.com/%s", pth) - var builds []buildRepr - for _, ba := range packaging.ArtifactConfigs { + builds := make([]buildRepr, len(packaging.ArtifactConfigs)) + for i, ba := range packaging.ArtifactConfigs { u := ba.GetURL(baseArchiveURL, cfg) sha256, err := getSHA256(u) @@ -165,12 +165,12 @@ func publishPackages(cfg packaging.PublishConfig) error { return err } - builds = append(builds, buildRepr{ + builds[i] = buildRepr{ OS: ba.Os, URL: u, SHA256: string(sha256), Arch: ba.Arch, - }) + } } r := releaseRepr{ diff --git a/pkg/expr/mathexp/exp.go b/pkg/expr/mathexp/exp.go index 9908cdc2cbc..055caffa27c 100644 --- a/pkg/expr/mathexp/exp.go +++ b/pkg/expr/mathexp/exp.go @@ -558,8 +558,9 @@ func (e *State) biSeriesSeries(labels data.Labels, op string, aSeries, bSeries S func (e *State) walkFunc(node *parse.FuncNode) (Results, error) { var res Results var err error - var in []reflect.Value - for _, a := range node.Args { + + in := make([]reflect.Value, len(node.Args)) + for i, a := range node.Args { var v any switch t := a.(type) { case *parse.StringNode: @@ -580,7 +581,8 @@ func (e *State) walkFunc(node *parse.FuncNode) (Results, error) { if err != nil { return res, err } - in = append(in, reflect.ValueOf(v)) + + in[i] = reflect.ValueOf(v) } f := reflect.ValueOf(node.F.F) diff --git a/pkg/infra/log/log.go b/pkg/infra/log/log.go index f12009cfe3d..03623e5ee55 100644 --- a/pkg/infra/log/log.go +++ b/pkg/infra/log/log.go @@ -444,7 +444,7 @@ func ReadLoggingConfig(modes []string, logsPath string, cfg *ini.File) error { defaultLevelName, _ := getLogLevelFromConfig("log", "info", cfg) defaultFilters := getFilters(util.SplitString(cfg.Section("log").Key("filters").String())) - var configLoggers []logWithFilters + configLoggers := make([]logWithFilters, 0, len(modes)) for _, mode := range modes { mode = strings.TrimSpace(mode) sec, err := cfg.GetSection("log." + mode) @@ -505,6 +505,7 @@ func ReadLoggingConfig(modes []string, logsPath string, cfg *ini.File) error { handler.filters = modeFilters handler.maxLevel = leveloption + configLoggers = append(configLoggers, handler) } if len(configLoggers) > 0 { diff --git a/pkg/modules/modules.go b/pkg/modules/modules.go index c36cc395f78..8e1a7e995f7 100644 --- a/pkg/modules/modules.go +++ b/pkg/modules/modules.go @@ -71,7 +71,7 @@ func (m *service) Run(ctx context.Context) error { return nil } - var svcs []services.Service + svcs := make([]services.Service, 0, len(m.serviceMap)) for _, s := range m.serviceMap { svcs = append(svcs, s) } diff --git a/pkg/plugins/manager/loader/angular/angularinspector/angularinspector_test.go b/pkg/plugins/manager/loader/angular/angularinspector/angularinspector_test.go index d66fcf2b285..f45ae982d09 100644 --- a/pkg/plugins/manager/loader/angular/angularinspector/angularinspector_test.go +++ b/pkg/plugins/manager/loader/angular/angularinspector/angularinspector_test.go @@ -93,6 +93,8 @@ func TestDefaultStaticDetectorsInspector(t *testing.T) { plugin *plugins.Plugin exp bool } + + //nolint:prealloc // just a test, and it'd require too much refactoring to preallocate var tcs []tc // Angular imports diff --git a/pkg/plugins/manager/signature/manifest_test.go b/pkg/plugins/manager/signature/manifest_test.go index 72425785889..1414f4d61b2 100644 --- a/pkg/plugins/manager/signature/manifest_test.go +++ b/pkg/plugins/manager/signature/manifest_test.go @@ -381,11 +381,13 @@ func TestFSPathSeparatorFiles(t *testing.T) { } func fileList(manifest *PluginManifest) []string { - var keys []string + keys := make([]string, 0, len(manifest.Files)) for k := range manifest.Files { keys = append(keys, k) } + sort.Strings(keys) + return keys } diff --git a/pkg/plugins/manager/sources/source_local_disk.go b/pkg/plugins/manager/sources/source_local_disk.go index 5f6ab1c3dd1..4fddc464fea 100644 --- a/pkg/plugins/manager/sources/source_local_disk.go +++ b/pkg/plugins/manager/sources/source_local_disk.go @@ -62,9 +62,10 @@ func DirAsLocalSources(pluginsPath string, class plugins.Class) ([]*LocalSource, } slices.Sort(pluginDirs) - var sources []*LocalSource - for _, dir := range pluginDirs { - sources = append(sources, NewLocalSource(class, []string{dir})) + sources := make([]*LocalSource, len(pluginDirs)) + for i, dir := range pluginDirs { + sources[i] = NewLocalSource(class, []string{dir}) } + return sources, nil } diff --git a/pkg/plugins/manager/sources/source_local_disk_test.go b/pkg/plugins/manager/sources/source_local_disk_test.go index e380b5d5388..0a903c4c4cc 100644 --- a/pkg/plugins/manager/sources/source_local_disk_test.go +++ b/pkg/plugins/manager/sources/source_local_disk_test.go @@ -46,7 +46,7 @@ func TestDirAsLocalSources(t *testing.T) { { name: "Directory with no subdirectories", pluginsPath: filepath.Join(testdataDir, "pluginRootWithDist", "datasource"), - expected: nil, + expected: []*LocalSource{}, }, { name: "Directory with a symlink to a directory", diff --git a/pkg/plugins/manager/sources/sources.go b/pkg/plugins/manager/sources/sources.go index a3bfe1d4112..353f6fe3046 100644 --- a/pkg/plugins/manager/sources/sources.go +++ b/pkg/plugins/manager/sources/sources.go @@ -38,22 +38,25 @@ func (s *Service) externalPluginSources() []plugins.PluginSource { return []plugins.PluginSource{} } - var srcs []plugins.PluginSource - for _, src := range localSrcs { - srcs = append(srcs, src) + srcs := make([]plugins.PluginSource, len(localSrcs)) + for i, src := range localSrcs { + srcs[i] = src } + return srcs } func (s *Service) pluginSettingSources() []plugins.PluginSource { - var sources []plugins.PluginSource + sources := make([]plugins.PluginSource, 0, len(s.cfg.PluginSettings)) for _, ps := range s.cfg.PluginSettings { path, exists := ps["path"] if !exists || path == "" { continue } + sources = append(sources, NewLocalSource(plugins.ClassExternal, []string{path})) } + return sources } diff --git a/pkg/plugins/repo/service_test.go b/pkg/plugins/repo/service_test.go index c2c0833e7ea..ba050893420 100644 --- a/pkg/plugins/repo/service_test.go +++ b/pkg/plugins/repo/service_test.go @@ -197,9 +197,8 @@ type versionArg struct { } func createPluginVersions(versions ...versionArg) []Version { - var vs []Version - - for _, version := range versions { + vs := make([]Version, len(versions)) + for i, version := range versions { ver := Version{ Version: version.version, } @@ -211,7 +210,8 @@ func createPluginVersions(versions ...versionArg) []Version { } } } - vs = append(vs, ver) + + vs[i] = ver } return vs diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index a9d9325a41c..02361171c82 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -326,7 +326,7 @@ func (e DatasourcePermissionsService) SetBuiltInRolePermission(ctx context.Conte // if an OSS/unlicensed instance is upgraded to Enterprise/licensed. // https://github.com/grafana/identity-access-team/issues/672 func (e DatasourcePermissionsService) SetPermissions(ctx context.Context, orgID int64, resourceID string, commands ...accesscontrol.SetResourcePermissionCommand) ([]accesscontrol.ResourcePermission, error) { - var dbCommands []resourcepermissions.SetResourcePermissionsCommand + dbCommands := make([]resourcepermissions.SetResourcePermissionsCommand, 0, len(commands)) for _, cmd := range commands { // Only set query permissions for built-in roles; do not set permissions for data sources with * as UID, as this would grant wildcard permissions if cmd.Permission != "Query" || cmd.BuiltinRole == "" || resourceID == "*" { diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go index de5982af442..e29844b5d7a 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go @@ -420,21 +420,12 @@ func (s *Service) RunMigration(ctx context.Context, uid string) (*cloudmigration } func (s *Service) getMigrationDataJSON(ctx context.Context) (*cloudmigration.MigrateDataRequest, error) { - var migrationDataSlice []cloudmigration.MigrateDataRequestItem // Data sources dataSources, err := s.getDataSources(ctx) if err != nil { s.log.Error("Failed to get datasources", "err", err) return nil, err } - for _, ds := range dataSources { - migrationDataSlice = append(migrationDataSlice, cloudmigration.MigrateDataRequestItem{ - Type: cloudmigration.DatasourceDataType, - RefID: ds.UID, - Name: ds.Name, - Data: ds, - }) - } // Dashboards dashboards, err := s.getDashboards(ctx) @@ -443,6 +434,26 @@ func (s *Service) getMigrationDataJSON(ctx context.Context) (*cloudmigration.Mig return nil, err } + // Folders + folders, err := s.getFolders(ctx) + if err != nil { + s.log.Error("Failed to get folders", "err", err) + return nil, err + } + + migrationDataSlice := make( + []cloudmigration.MigrateDataRequestItem, 0, + len(dataSources)+len(dashboards)+len(folders), + ) + for _, ds := range dataSources { + migrationDataSlice = append(migrationDataSlice, cloudmigration.MigrateDataRequestItem{ + Type: cloudmigration.DatasourceDataType, + RefID: ds.UID, + Name: ds.Name, + Data: ds, + }) + } + for _, dashboard := range dashboards { dashboard.Data.Del("id") migrationDataSlice = append(migrationDataSlice, cloudmigration.MigrateDataRequestItem{ @@ -453,13 +464,6 @@ func (s *Service) getMigrationDataJSON(ctx context.Context) (*cloudmigration.Mig }) } - // Folders - folders, err := s.getFolders(ctx) - if err != nil { - s.log.Error("Failed to get folders", "err", err) - return nil, err - } - for _, f := range folders { migrationDataSlice = append(migrationDataSlice, cloudmigration.MigrateDataRequestItem{ Type: cloudmigration.FolderDataType, @@ -468,6 +472,7 @@ func (s *Service) getMigrationDataJSON(ctx context.Context) (*cloudmigration.Mig Data: f, }) } + migrationData := &cloudmigration.MigrateDataRequest{ Items: migrationDataSlice, } @@ -521,9 +526,9 @@ func (s *Service) getFolders(ctx context.Context) ([]folder.Folder, error) { return nil, err } - var result []folder.Folder - for _, folder := range folders { - result = append(result, *folder) + result := make([]folder.Folder, len(folders)) + for i, folder := range folders { + result[i] = *folder } return result, nil @@ -535,10 +540,11 @@ func (s *Service) getDashboards(ctx context.Context) ([]dashboards.Dashboard, er return nil, err } - var result []dashboards.Dashboard - for _, dashboard := range dashs { - result = append(result, *dashboard) + result := make([]dashboards.Dashboard, len(dashs)) + for i, dashboard := range dashs { + result[i] = *dashboard } + return result, nil } diff --git a/pkg/services/live/live.go b/pkg/services/live/live.go index cb0b656a142..25632794924 100644 --- a/pkg/services/live/live.go +++ b/pkg/services/live/live.go @@ -332,12 +332,14 @@ func setupRedisLiveEngine(g *GrafanaLive, node *centrifuge.Node) error { redisShardConfigs := []centrifuge.RedisShardConfig{ {Address: redisAddress, Password: redisPassword}, } - var redisShards []*centrifuge.RedisShard + + redisShards := make([]*centrifuge.RedisShard, 0, len(redisShardConfigs)) for _, redisConf := range redisShardConfigs { redisShard, err := centrifuge.NewRedisShard(node, redisConf) if err != nil { return fmt.Errorf("error connecting to Live Redis: %v", err) } + redisShards = append(redisShards, redisShard) } @@ -348,6 +350,7 @@ func setupRedisLiveEngine(g *GrafanaLive, node *centrifuge.Node) error { if err != nil { return fmt.Errorf("error creating Live Redis broker: %v", err) } + node.SetBroker(broker) presenceManager, err := centrifuge.NewRedisPresenceManager(node, centrifuge.RedisPresenceManagerConfig{ @@ -357,7 +360,9 @@ func setupRedisLiveEngine(g *GrafanaLive, node *centrifuge.Node) error { if err != nil { return fmt.Errorf("error creating Live Redis presence manager: %v", err) } + node.SetPresenceManager(presenceManager) + return nil } diff --git a/pkg/services/live/runstream/manager.go b/pkg/services/live/runstream/manager.go index 0d404ca1ead..e2a0033f1f8 100644 --- a/pkg/services/live/runstream/manager.go +++ b/pkg/services/live/runstream/manager.go @@ -116,17 +116,21 @@ func (s *Manager) handleDatasourceEvent(orgID int64, dsUID string, resubmit bool s.mu.RUnlock() return nil } - var resubmitRequests []streamRequest - var waitChannels []chan struct{} + + resubmitRequests := make([]streamRequest, 0, len(dsStreams)) + waitChannels := make([]chan struct{}, 0, len(dsStreams)) for channel := range dsStreams { streamCtx, ok := s.streams[channel] if !ok { continue } + streamCtx.cancelFn() + waitChannels = append(waitChannels, streamCtx.CloseCh) resubmitRequests = append(resubmitRequests, streamCtx.streamRequest) } + s.mu.RUnlock() // Wait for all streams to stop. diff --git a/pkg/services/ngalert/api/api_prometheus.go b/pkg/services/ngalert/api/api_prometheus.go index ac2bbad3718..ccea63ffbb8 100644 --- a/pkg/services/ngalert/api/api_prometheus.go +++ b/pkg/services/ngalert/api/api_prometheus.go @@ -563,8 +563,8 @@ func toRuleGroup(log log.Logger, manager state.AlertInstanceManager, groupKey ng // Returns the whole JSON model as a string if it fails to extract a minimum of 1 query. func ruleToQuery(logger log.Logger, rule *ngmodels.AlertRule) string { var queryErr error - var queries []string + queries := make([]string, 0, len(rule.Data)) for _, q := range rule.Data { q, err := q.GetQuery() if err != nil { diff --git a/pkg/services/ngalert/api/compat_contact_points.go b/pkg/services/ngalert/api/compat_contact_points.go index 26b13fbc5a2..60ff5f2902f 100644 --- a/pkg/services/ngalert/api/compat_contact_points.go +++ b/pkg/services/ngalert/api/compat_contact_points.go @@ -46,156 +46,164 @@ func ContactPointToContactPointExport(cp definitions.ContactPoint) (notify.APIRe // This is needed to keep the API models clean and convert from database model j.RegisterExtension(&contactPointsExtension{}) - var integration []*notify.GrafanaIntegrationConfig + contactPointsLength := len(cp.Alertmanager) + len(cp.Dingding) + len(cp.Discord) + len(cp.Email) + + len(cp.Googlechat) + len(cp.Kafka) + len(cp.Line) + len(cp.Opsgenie) + + len(cp.Pagerduty) + len(cp.OnCall) + len(cp.Pushover) + len(cp.Sensugo) + + len(cp.Sns) + len(cp.Slack) + len(cp.Teams) + len(cp.Telegram) + + len(cp.Threema) + len(cp.Victorops) + len(cp.Webhook) + len(cp.Wecom) + + len(cp.Webex) + + integration := make([]*notify.GrafanaIntegrationConfig, 0, contactPointsLength) var errs []error for _, i := range cp.Alertmanager { el, err := marshallIntegration(j, "prometheus-alertmanager", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Dingding { el, err := marshallIntegration(j, "dingding", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Discord { el, err := marshallIntegration(j, "discord", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Email { el, err := marshallIntegration(j, "email", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Googlechat { el, err := marshallIntegration(j, "googlechat", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Kafka { el, err := marshallIntegration(j, "kafka", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Line { el, err := marshallIntegration(j, "line", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Opsgenie { el, err := marshallIntegration(j, "opsgenie", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Pagerduty { el, err := marshallIntegration(j, "pagerduty", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.OnCall { el, err := marshallIntegration(j, "oncall", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Pushover { el, err := marshallIntegration(j, "pushover", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Sensugo { el, err := marshallIntegration(j, "sensugo", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Sns { el, err := marshallIntegration(j, "sns", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Slack { el, err := marshallIntegration(j, "slack", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Teams { el, err := marshallIntegration(j, "teams", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Telegram { el, err := marshallIntegration(j, "telegram", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Threema { el, err := marshallIntegration(j, "threema", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Victorops { el, err := marshallIntegration(j, "victorops", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Webhook { el, err := marshallIntegration(j, "webhook", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Wecom { el, err := marshallIntegration(j, "wecom", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } for _, i := range cp.Webex { el, err := marshallIntegration(j, "webex", i, i.DisableResolveMessage) - integration = append(integration, el) if err != nil { errs = append(errs, err) } + integration = append(integration, el) } + if len(errs) > 0 { return notify.APIReceiver{}, errors.Join(errs...) } diff --git a/pkg/services/ngalert/provisioning/contactpoints.go b/pkg/services/ngalert/provisioning/contactpoints.go index caf5cefc297..1a57046e526 100644 --- a/pkg/services/ngalert/provisioning/contactpoints.go +++ b/pkg/services/ngalert/provisioning/contactpoints.go @@ -88,13 +88,14 @@ func (ecp *ContactPointService) GetContactPoints(ctx context.Context, q ContactP } } - var contactPoints []apimodels.EmbeddedContactPoint - for _, gr := range grafanaReceivers { + contactPoints := make([]apimodels.EmbeddedContactPoint, len(grafanaReceivers)) + for i, gr := range grafanaReceivers { contactPoint, err := GettableGrafanaReceiverToEmbeddedContactPoint(gr) if err != nil { return nil, err } - contactPoints = append(contactPoints, contactPoint) + + contactPoints[i] = contactPoint } sort.SliceStable(contactPoints, func(i, j int) bool { diff --git a/pkg/services/ngalert/provisioning/templates.go b/pkg/services/ngalert/provisioning/templates.go index 2ed37ae3723..074934af988 100644 --- a/pkg/services/ngalert/provisioning/templates.go +++ b/pkg/services/ngalert/provisioning/templates.go @@ -31,17 +31,19 @@ func (t *TemplateService) GetTemplates(ctx context.Context, orgID int64) ([]defi return nil, err } - var templates []definitions.NotificationTemplate + templates := make([]definitions.NotificationTemplate, 0, len(revision.cfg.TemplateFiles)) for name, tmpl := range revision.cfg.TemplateFiles { tmpl := definitions.NotificationTemplate{ Name: name, Template: tmpl, } + provenance, err := t.provenanceStore.GetProvenance(ctx, &tmpl, orgID) if err != nil { return nil, err } tmpl.Provenance = definitions.Provenance(provenance) + templates = append(templates, tmpl) } diff --git a/pkg/services/ngalert/remote/alertmanager.go b/pkg/services/ngalert/remote/alertmanager.go index 1c9c8afff27..8d460a756cb 100644 --- a/pkg/services/ngalert/remote/alertmanager.go +++ b/pkg/services/ngalert/remote/alertmanager.go @@ -492,13 +492,14 @@ func (am *Alertmanager) GetReceivers(ctx context.Context) ([]apimodels.Receiver, return []apimodels.Receiver{}, err } - var rcvs []apimodels.Receiver - for _, rcv := range res.Payload { - rcvs = append(rcvs, apimodels.Receiver{ + rcvs := make([]apimodels.Receiver, len(res.Payload)) + for i, rcv := range res.Payload { + rcvs[i] = apimodels.Receiver{ Name: *rcv.Name, Integrations: []apimodels.Integration{}, - }) + } } + return rcvs, nil } diff --git a/pkg/services/ngalert/sender/router.go b/pkg/services/ngalert/sender/router.go index 1fe82927bd6..cfb474f63be 100644 --- a/pkg/services/ngalert/sender/router.go +++ b/pkg/services/ngalert/sender/router.go @@ -205,15 +205,17 @@ func (d *AlertsRouter) SyncAndApplyConfigFromDatabase(ctx context.Context) error } func buildRedactedAMs(l log.Logger, alertmanagers []ExternalAMcfg, ordId int64) []string { - var redactedAMs []string + redactedAMs := make([]string, 0, len(alertmanagers)) for _, am := range alertmanagers { parsedAM, err := url.Parse(am.URL) if err != nil { l.Error("Failed to parse alertmanager string", "org", ordId, "error", err) continue } + redactedAMs = append(redactedAMs, parsedAM.Redacted()) } + return redactedAMs } @@ -225,9 +227,6 @@ func asSHA256(strings []string) string { } func (d *AlertsRouter) alertmanagersFromDatasources(orgID int64) ([]ExternalAMcfg, error) { - var ( - alertmanagers []ExternalAMcfg - ) // We might have alertmanager datasources that are acting as external // alertmanager, let's fetch them. query := &datasources.GetDataSourcesByTypeQuery{ @@ -240,6 +239,9 @@ func (d *AlertsRouter) alertmanagersFromDatasources(orgID int64) ([]ExternalAMcf if err != nil { return nil, fmt.Errorf("failed to fetch datasources for org: %w", err) } + + alertmanagers := make([]ExternalAMcfg, 0, len(dataSources)) + for _, ds := range dataSources { if !ds.JsonData.Get(definitions.HandleGrafanaManagedAlerts).MustBool(false) { continue @@ -262,11 +264,13 @@ func (d *AlertsRouter) alertmanagersFromDatasources(orgID int64) ([]ExternalAMcf "error", err) continue } + alertmanagers = append(alertmanagers, ExternalAMcfg{ URL: amURL, Headers: headers, }) } + return alertmanagers, nil } diff --git a/pkg/services/ngalert/sender/router_test.go b/pkg/services/ngalert/sender/router_test.go index 303fa6065d8..c10f88e6001 100644 --- a/pkg/services/ngalert/sender/router_test.go +++ b/pkg/services/ngalert/sender/router_test.go @@ -712,7 +712,7 @@ func TestAlertManagers_buildRedactedAMs(t *testing.T) { amUrls: []string{"1234://user:password@localhost:9094"}, errCalls: 1, errLog: "Failed to parse alertmanager string", - expected: nil, + expected: []string{}, }, } @@ -724,6 +724,7 @@ func TestAlertManagers_buildRedactedAMs(t *testing.T) { URL: url, }) } + require.Equal(t, tt.expected, buildRedactedAMs(&fakeLogger, cfgs, tt.orgId)) require.Equal(t, tt.errCalls, fakeLogger.ErrorLogs.Calls) require.Equal(t, tt.errLog, fakeLogger.ErrorLogs.Message) diff --git a/pkg/services/ngalert/state/compat.go b/pkg/services/ngalert/state/compat.go index a17b6637a02..673e9e56810 100644 --- a/pkg/services/ngalert/state/compat.go +++ b/pkg/services/ngalert/state/compat.go @@ -141,9 +141,9 @@ func errorAlert(labels, annotations data.Labels, alertState *State, urlStr strin func FromStateTransitionToPostableAlerts(firingStates []StateTransition, stateManager *Manager, appURL *url.URL) apimodels.PostableAlerts { alerts := apimodels.PostableAlerts{PostableAlerts: make([]models.PostableAlert, 0, len(firingStates))} - var sentAlerts []*State ts := time.Now() + sentAlerts := make([]*State, 0, len(firingStates)) for _, alertState := range firingStates { if !alertState.NeedsSending(stateManager.ResendDelay) { continue diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 266efc6eb48..d86a149c6e5 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -787,7 +787,8 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI if len(rules) == 0 { return 0, nil } - var updates []ngmodels.UpdateRule + + updates := make([]ngmodels.UpdateRule, 0, len(rules)) for _, rule := range rules { r := ngmodels.CopyRule(rule) for idx := range r.NotificationSettings { @@ -795,6 +796,7 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI r.NotificationSettings[idx].Receiver = newReceiver } } + updates = append(updates, ngmodels.UpdateRule{ Existing: rule, New: *r, diff --git a/pkg/services/plugindashboards/service/dashboard_updater_test.go b/pkg/services/plugindashboards/service/dashboard_updater_test.go index 16170c7ae26..6a8010c39f4 100644 --- a/pkg/services/plugindashboards/service/dashboard_updater_test.go +++ b/pkg/services/plugindashboards/service/dashboard_updater_test.go @@ -401,15 +401,15 @@ type pluginsSettingsServiceMock struct { func (s *pluginsSettingsServiceMock) GetPluginSettings(_ context.Context, args *pluginsettings.GetArgs) ([]*pluginsettings.InfoDTO, error) { s.getPluginSettingsArgs = append(s.getPluginSettingsArgs, args.OrgID) - var res []*pluginsettings.InfoDTO - for _, ps := range s.storedPluginSettings { - res = append(res, &pluginsettings.InfoDTO{ + res := make([]*pluginsettings.InfoDTO, len(s.storedPluginSettings)) + for i, ps := range s.storedPluginSettings { + res[i] = &pluginsettings.InfoDTO{ PluginID: ps.PluginID, OrgID: ps.OrgID, Enabled: ps.Enabled, Pinned: ps.Pinned, PluginVersion: ps.PluginVersion, - }) + } } return res, s.err diff --git a/pkg/services/pluginsintegration/pluginconfig/envvars.go b/pkg/services/pluginsintegration/pluginconfig/envvars.go index fe3579661e1..e29a75f6ec7 100644 --- a/pkg/services/pluginsintegration/pluginconfig/envvars.go +++ b/pkg/services/pluginsintegration/pluginconfig/envvars.go @@ -163,17 +163,23 @@ func (p *EnvVarsProvider) tracingEnvVars(plugin *plugins.Plugin) []string { func (p *EnvVarsProvider) pluginSettingsEnvVars(pluginID string) []string { const customConfigPrefix = "GF_PLUGIN" - var env []string - for k, v := range p.cfg.PluginSettings[pluginID] { + + pluginSettings := p.cfg.PluginSettings[pluginID] + + env := make([]string, 0, len(pluginSettings)) + for k, v := range pluginSettings { if k == "path" || strings.ToLower(k) == "id" { continue } + key := fmt.Sprintf("%s_%s", customConfigPrefix, strings.ToUpper(k)) if value := os.Getenv(key); value != "" { v = value } + env = append(env, fmt.Sprintf("%s=%s", key, v)) } + return env } diff --git a/pkg/services/searchV2/bluge.go b/pkg/services/searchV2/bluge.go index dd290a2ed8a..d1fdeac5424 100644 --- a/pkg/services/searchV2/bluge.go +++ b/pkg/services/searchV2/bluge.go @@ -191,7 +191,9 @@ func getNonFolderDashboardDoc(dash dashboard, location string) *bluge.Document { func getDashboardPanelDocs(dash dashboard, location string) []*bluge.Document { dashURL := fmt.Sprintf("/d/%s/%s", dash.uid, slugify.Slugify(dash.summary.Name)) - var docs []*bluge.Document + // pre-allocating a little bit more than necessary, possibly + docs := make([]*bluge.Document, 0, len(dash.summary.Nested)) + for _, panel := range dash.summary.Nested { if panel.Fields["type"] == "row" { continue // skip rows @@ -239,7 +241,7 @@ func getDashboardPanelDocs(dash dashboard, location string) []*bluge.Document { } // Names need to be indexed a few ways to support key features -func newSearchDocument(uid string, name string, descr string, url string) *bluge.Document { +func newSearchDocument(uid, name, descr, url string) *bluge.Document { doc := bluge.NewDocument(uid) if name != "" { diff --git a/pkg/services/searchV2/ngram_test.go b/pkg/services/searchV2/ngram_test.go index be2d6646e9e..160a956db57 100644 --- a/pkg/services/searchV2/ngram_test.go +++ b/pkg/services/searchV2/ngram_test.go @@ -51,9 +51,11 @@ func Test_punctuationCharFilter_Filter(t1 *testing.T) { func TestNgramIndexAnalyzer(t *testing.T) { stream := ngramIndexAnalyzer.Analyze([]byte("x-rays.and.xRays, and НемногоКириллицы")) expectedTerms := []string{"x", "r", "ra", "ray", "rays", "a", "an", "and", "x", "r", "ra", "ray", "rays", "a", "an", "and", "н", "не", "нем", "немн", "немно", "немног", "немного", "к", "ки", "кир", "кири", "кирил", "кирилл", "кирилли"} - var actualTerms []string + + actualTerms := make([]string, 0, len(stream)) for _, t := range stream { actualTerms = append(actualTerms, string(t.Term)) } + require.Equal(t, expectedTerms, actualTerms) } diff --git a/pkg/services/sqlstore/migrations/external_alertmanagers.go b/pkg/services/sqlstore/migrations/external_alertmanagers.go index 067feee3a17..5f92210fa7f 100644 --- a/pkg/services/sqlstore/migrations/external_alertmanagers.go +++ b/pkg/services/sqlstore/migrations/external_alertmanagers.go @@ -97,9 +97,8 @@ func (e externalAlertmanagerToDatasources) Exec(sess *xorm.Session, mg *migrator } func removeDuplicates(strs []string) []string { - var res []string - found := map[string]bool{} - + found := make(map[string]bool, len(strs)) + res := make([]string, 0, len(strs)) for _, str := range strs { if found[str] { continue diff --git a/pkg/services/sqlstore/permissions/dashboard.go b/pkg/services/sqlstore/permissions/dashboard.go index 1dc5fe990ff..0b782348351 100644 --- a/pkg/services/sqlstore/permissions/dashboard.go +++ b/pkg/services/sqlstore/permissions/dashboard.go @@ -435,8 +435,10 @@ func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSele } func getAllowedUIDs(action string, user identity.Requester, scopePrefix string) []any { - var args []any - for _, uidScope := range user.GetPermissions()[action] { + uidScopes := user.GetPermissions()[action] + + args := make([]any, 0, len(uidScopes)) + for _, uidScope := range uidScopes { if !strings.HasPrefix(uidScope, scopePrefix) { continue } diff --git a/pkg/services/ssosettings/api/api.go b/pkg/services/ssosettings/api/api.go index c5d2540ebb1..e535699e07f 100644 --- a/pkg/services/ssosettings/api/api.go +++ b/pkg/services/ssosettings/api/api.go @@ -101,7 +101,7 @@ func (api *Api) getAuthorizedList(ctx context.Context, identity identity.Request return nil, err } - var authorizedProviders []*models.SSOSettings + authorizedProviders := make([]*models.SSOSettings, 0, len(allProviders)) for _, provider := range allProviders { ev := ac.EvalPermission(ac.ActionSettingsRead, ac.Scope("settings", "auth."+provider.Provider, "*")) hasAccess, err := api.AccessControl.Evaluate(ctx, identity, ev) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index df89c95455e..1fc76c1d54c 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -1986,19 +1986,23 @@ func (cfg *Cfg) readLiveSettings(iniFile *ini.File) error { cfg.LiveHAEngineAddress = section.Key("ha_engine_address").MustString("127.0.0.1:6379") cfg.LiveHAEnginePassword = section.Key("ha_engine_password").MustString("") - var originPatterns []string allowedOrigins := section.Key("allowed_origins").MustString("") - for _, originPattern := range strings.Split(allowedOrigins, ",") { + origins := strings.Split(allowedOrigins, ",") + + originPatterns := make([]string, 0, len(origins)) + for _, originPattern := range origins { originPattern = strings.TrimSpace(originPattern) if originPattern == "" { continue } originPatterns = append(originPatterns, originPattern) } + _, err := GetAllowedOriginGlobs(originPatterns) if err != nil { return err } + cfg.LiveAllowedOrigins = originPatterns return nil } diff --git a/pkg/setting/setting_secure_socks_proxy.go b/pkg/setting/setting_secure_socks_proxy.go index 6e034cfa313..fe94e79e1c5 100644 --- a/pkg/setting/setting_secure_socks_proxy.go +++ b/pkg/setting/setting_secure_socks_proxy.go @@ -75,7 +75,7 @@ func readSecureSocksDSProxySettings(iniFile *ini.File) (SecureSocksDSProxySettin s.ClientKey = string(keyPEMBlock) } - var rootCAs []string + rootCAs := make([]string, 0, len(s.RootCAFilePaths)) for _, rootCAFile := range s.RootCAFilePaths { // nolint:gosec // The gosec G304 warning can be ignored because `rootCAFile` comes from config ini, and we check below if diff --git a/pkg/tsdb/azuremonitor/resourcegraph/azure-resource-graph-datasource.go b/pkg/tsdb/azuremonitor/resourcegraph/azure-resource-graph-datasource.go index ce3373bdbb4..8300a3c423a 100644 --- a/pkg/tsdb/azuremonitor/resourcegraph/azure-resource-graph-datasource.go +++ b/pkg/tsdb/azuremonitor/resourcegraph/azure-resource-graph-datasource.go @@ -88,9 +88,8 @@ type argJSONQuery struct { } func (e *AzureResourceGraphDatasource) buildQueries(queries []backend.DataQuery, dsInfo types.DatasourceInfo) ([]*AzureResourceGraphQuery, error) { - var azureResourceGraphQueries []*AzureResourceGraphQuery - - for _, query := range queries { + azureResourceGraphQueries := make([]*AzureResourceGraphQuery, len(queries)) + for i, query := range queries { queryJSONModel := argJSONQuery{} err := json.Unmarshal(query.JSON, &queryJSONModel) if err != nil { @@ -105,19 +104,18 @@ func (e *AzureResourceGraphDatasource) buildQueries(queries []backend.DataQuery, } interpolatedQuery, err := macros.KqlInterpolate(query, dsInfo, azureResourceGraphTarget.Query) - if err != nil { return nil, err } - azureResourceGraphQueries = append(azureResourceGraphQueries, &AzureResourceGraphQuery{ + azureResourceGraphQueries[i] = &AzureResourceGraphQuery{ RefID: query.RefID, ResultFormat: resultFormat, JSON: query.JSON, InterpolatedQuery: interpolatedQuery, TimeRange: query.TimeRange, QueryType: query.QueryType, - }) + } } return azureResourceGraphQueries, nil diff --git a/pkg/tsdb/azuremonitor/types/types.go b/pkg/tsdb/azuremonitor/types/types.go index 1e3e7f2073e..c0fd5b99095 100644 --- a/pkg/tsdb/azuremonitor/types/types.go +++ b/pkg/tsdb/azuremonitor/types/types.go @@ -129,8 +129,8 @@ type AzureMonitorDimensionFilterBackend struct { } func ConstructFiltersString(a dataquery.AzureMetricDimension) string { - var filterStrings []string - for _, filter := range a.Filters { + filterStrings := make([]string, len(a.Filters)) + for i, filter := range a.Filters { dimension := "" operator := "" if a.Dimension != nil { @@ -139,11 +139,14 @@ func ConstructFiltersString(a dataquery.AzureMetricDimension) string { if a.Operator != nil { operator = *a.Operator } - filterStrings = append(filterStrings, fmt.Sprintf("%v %v '%v'", dimension, operator, filter)) + + filterStrings[i] = fmt.Sprintf("%v %v '%v'", dimension, operator, filter) } + if a.Operator != nil && *a.Operator == "eq" { return strings.Join(filterStrings, " or ") } + return strings.Join(filterStrings, " and ") } diff --git a/pkg/tsdb/cloudwatch/services/utils.go b/pkg/tsdb/cloudwatch/services/utils.go index 4684aa2acad..3ba3fbfcba2 100644 --- a/pkg/tsdb/cloudwatch/services/utils.go +++ b/pkg/tsdb/cloudwatch/services/utils.go @@ -7,7 +7,7 @@ import ( ) func valuesToListMetricRespone[T any](values []T) []resources.ResourceResponse[T] { - var response []resources.ResourceResponse[T] + response := make([]resources.ResourceResponse[T], 0, len(values)) for _, value := range values { response = append(response, resources.ResourceResponse[T]{Value: value}) } diff --git a/pkg/tsdb/elasticsearch/querydata_test.go b/pkg/tsdb/elasticsearch/querydata_test.go index eeb3be175d7..1adf955af3a 100644 --- a/pkg/tsdb/elasticsearch/querydata_test.go +++ b/pkg/tsdb/elasticsearch/querydata_test.go @@ -84,15 +84,16 @@ func newFlowTestQueries(allJsonBytes []byte) ([]backend.DataQuery, error) { return nil, fmt.Errorf("error unmarshaling query-json: %w", err) } - var queries []backend.DataQuery - - for _, jsonBytes := range jsonBytesArray { + queries := make([]backend.DataQuery, len(jsonBytesArray)) + for i, jsonBytes := range jsonBytesArray { // we need to extract some fields from the json-array var jsonInfo queryDataTestQueryJSON + err = json.Unmarshal(jsonBytes, &jsonInfo) if err != nil { return nil, err } + // we setup the DataQuery, with values loaded from the json query := backend.DataQuery{ RefID: jsonInfo.RefID, @@ -101,7 +102,8 @@ func newFlowTestQueries(allJsonBytes []byte) ([]backend.DataQuery, error) { TimeRange: timeRange, JSON: jsonBytes, } - queries = append(queries, query) + + queries[i] = query } return queries, nil } diff --git a/pkg/tsdb/elasticsearch/response_parser.go b/pkg/tsdb/elasticsearch/response_parser.go index 822a655fb32..7701e8fee37 100644 --- a/pkg/tsdb/elasticsearch/response_parser.go +++ b/pkg/tsdb/elasticsearch/response_parser.go @@ -873,16 +873,16 @@ func trimDatapoints(queryResult backend.DataResponse, target *Query) { // we sort the label's pairs by the label-key, // and return the label-values func getSortedLabelValues(labels data.Labels) []string { - var keys []string + keys := make([]string, 0, len(labels)) for key := range labels { keys = append(keys, key) } sort.Strings(keys) - var values []string - for _, key := range keys { - values = append(values, labels[key]) + values := make([]string, len(keys)) + for i, key := range keys { + values[i] = labels[key] } return values diff --git a/pkg/tsdb/influxdb/fsql/arrow_test.go b/pkg/tsdb/influxdb/fsql/arrow_test.go index fa8844acf1d..3a7a27a0489 100644 --- a/pkg/tsdb/influxdb/fsql/arrow_test.go +++ b/pkg/tsdb/influxdb/fsql/arrow_test.go @@ -60,7 +60,7 @@ func TestNewQueryDataResponse(t *testing.T) { newJSONArray(`[0, 1, 2]`, &arrow.TimestampType{}), } - var arr []arrow.Array + arr := make([]arrow.Array, 0, len(strValues)) for _, v := range strValues { tarr, _, err := array.FromJSON( alloc, diff --git a/pkg/tsdb/influxdb/influxql/buffered/response_parser.go b/pkg/tsdb/influxdb/influxql/buffered/response_parser.go index fbd93486511..91fc8b9967f 100644 --- a/pkg/tsdb/influxdb/influxql/buffered/response_parser.go +++ b/pkg/tsdb/influxdb/influxql/buffered/response_parser.go @@ -269,12 +269,12 @@ func transformRowsForTimeSeries(rows []models.Row, query models.Query) data.Fram } func newFrameWithTimeField(row models.Row, column string, colIndex int, query models.Query, frameName []byte) *data.Frame { - var timeArray []time.Time var floatArray []*float64 var stringArray []*string var boolArray []*bool valType := util.Typeof(row.Values, colIndex) + timeArray := make([]time.Time, 0, len(row.Values)) for _, valuePair := range row.Values { timestamp, timestampErr := util.ParseTimestamp(valuePair[0]) // we only add this row if the timestamp is valid diff --git a/pkg/tsdb/tempo/trace_transform_test.go b/pkg/tsdb/tempo/trace_transform_test.go index 2dd60c07867..8b901c6c41c 100644 --- a/pkg/tsdb/tempo/trace_transform_test.go +++ b/pkg/tsdb/tempo/trace_transform_test.go @@ -136,10 +136,11 @@ func rootSpan(frame *BetterFrame) Row { } func fieldNames(frame *data.Frame) []string { - var names []string - for _, f := range frame.Fields { - names = append(names, f.Name) + names := make([]string, len(frame.Fields)) + for i, f := range frame.Fields { + names[i] = f.Name } + return names } diff --git a/pkg/util/strings.go b/pkg/util/strings.go index 9bb688e3ca5..f3a2d35540f 100644 --- a/pkg/util/strings.go +++ b/pkg/util/strings.go @@ -48,11 +48,13 @@ func SplitString(str string) []string { return res } - var result []string matches := stringListItemMatcher.FindAllString(str, -1) - for _, match := range matches { - result = append(result, strings.Trim(match, "\"")) + + result := make([]string, len(matches)) + for i, match := range matches { + result[i] = strings.Trim(match, "\"") } + return result } diff --git a/pkg/util/tls.go b/pkg/util/tls.go index 049d6cf12da..0a5b4227b27 100644 --- a/pkg/util/tls.go +++ b/pkg/util/tls.go @@ -30,22 +30,26 @@ func TlsCiphersToIDs(names []string) ([]uint16, error) { // no ciphers specified, use defaults return nil, nil } - var ids []uint16 - var missing []string ciphers := tls.CipherSuites() - var cipherMap = make(map[string]uint16, len(ciphers)) + + cipherMap := make(map[string]uint16, len(ciphers)) for _, cipher := range ciphers { cipherMap[cipher.Name] = cipher.ID } + missing := []string{} + ids := make([]uint16, 0, len(names)) + for _, name := range names { name = strings.ToUpper(name) + id, ok := cipherMap[name] if !ok { missing = append(missing, name) continue } + ids = append(ids, id) } From 8491e02cafa4f371e0d24955d45bc16dcb037b65 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Fri, 14 Jun 2024 13:24:12 -0500 Subject: [PATCH 03/47] Alerting: Instrument outbound requests for Loki Historian and Remote Alertmanager with tracing (#89185) * Add TracedClient * Handle errors and status codes * Wire up tracing to normal ASH and loki annotation mapping * Add tracing to remote alertmanager * one more spot * and not or * More consistency with other grafana traces, lower cardinality name --- .../annotationsimpl/annotations.go | 4 +- .../annotationsimpl/annotations_test.go | 4 +- .../annotationsimpl/loki/historian_store.go | 5 +- pkg/services/ngalert/client/client.go | 55 ++++++++++++++++++- pkg/services/ngalert/ngalert.go | 20 +++---- pkg/services/ngalert/ngalert_test.go | 18 ++++-- .../multiorg_alertmanager_remote_test.go | 3 +- pkg/services/ngalert/remote/alertmanager.go | 7 ++- .../ngalert/remote/alertmanager_test.go | 16 +++--- .../ngalert/remote/client/alertmanager.go | 8 ++- pkg/services/ngalert/remote/client/mimir.go | 7 ++- pkg/services/ngalert/state/historian/loki.go | 5 +- .../ngalert/state/historian/loki_http.go | 6 +- .../ngalert/state/historian/loki_http_test.go | 7 ++- .../ngalert/state/historian/loki_test.go | 3 +- .../publicdashboards/service/common_test.go | 3 +- 16 files changed, 124 insertions(+), 47 deletions(-) diff --git a/pkg/services/annotations/annotationsimpl/annotations.go b/pkg/services/annotations/annotationsimpl/annotations.go index 3cf88288d27..b6a9e622899 100644 --- a/pkg/services/annotations/annotationsimpl/annotations.go +++ b/pkg/services/annotations/annotationsimpl/annotations.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/tag" @@ -27,6 +28,7 @@ func ProvideService( cfg *setting.Cfg, features featuremgmt.FeatureToggles, tagService tag.Service, + tracer tracing.Tracer, ) *RepositoryImpl { l := log.New("annotations") l.Debug("Initializing annotations service") @@ -35,7 +37,7 @@ func ProvideService( write := xormStore var read readStore - historianStore := loki.NewLokiHistorianStore(cfg.UnifiedAlerting.StateHistory, features, db, log.New("annotations.loki")) + historianStore := loki.NewLokiHistorianStore(cfg.UnifiedAlerting.StateHistory, features, db, log.New("annotations.loki"), tracer) if historianStore != nil { l.Debug("Using composite read store") read = NewCompositeStore(log.New("annotations.composite"), xormStore, historianStore) diff --git a/pkg/services/annotations/annotationsimpl/annotations_test.go b/pkg/services/annotations/annotationsimpl/annotations_test.go index c296ac4ab30..5cbc5ccb30a 100644 --- a/pkg/services/annotations/annotationsimpl/annotations_test.go +++ b/pkg/services/annotations/annotationsimpl/annotations_test.go @@ -48,7 +48,7 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) { features := featuremgmt.WithFeatures() tagService := tagimpl.ProvideService(sql) - repo := ProvideService(sql, cfg, features, tagService) + repo := ProvideService(sql, cfg, features, tagService, tracing.InitializeTracerForTest()) dashboard1 := testutil.CreateDashboard(t, sql, cfg, features, dashboards.SaveDashboardCommand{ UserID: 1, @@ -317,7 +317,7 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) { cfg := setting.NewCfg() cfg.AnnotationMaximumTagsLength = 60 - repo := ProvideService(sql, cfg, tc.features, tagimpl.ProvideService(sql)) + repo := ProvideService(sql, cfg, tc.features, tagimpl.ProvideService(sql), tracing.InitializeTracerForTest()) usr.Permissions = map[int64]map[string][]string{1: tc.permissions} testutil.SetupRBACPermission(t, sql, role, usr) diff --git a/pkg/services/annotations/annotationsimpl/loki/historian_store.go b/pkg/services/annotations/annotationsimpl/loki/historian_store.go index 1fca8d4f176..b4e411b4a54 100644 --- a/pkg/services/annotations/annotationsimpl/loki/historian_store.go +++ b/pkg/services/annotations/annotationsimpl/loki/historian_store.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" ngmetrics "github.com/grafana/grafana/pkg/services/ngalert/metrics" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/state" @@ -53,7 +54,7 @@ type LokiHistorianStore struct { log log.Logger } -func NewLokiHistorianStore(cfg setting.UnifiedAlertingStateHistorySettings, ft featuremgmt.FeatureToggles, db db.DB, log log.Logger) *LokiHistorianStore { +func NewLokiHistorianStore(cfg setting.UnifiedAlertingStateHistorySettings, ft featuremgmt.FeatureToggles, db db.DB, log log.Logger, tracer tracing.Tracer) *LokiHistorianStore { if !useStore(cfg, ft) { return nil } @@ -64,7 +65,7 @@ func NewLokiHistorianStore(cfg setting.UnifiedAlertingStateHistorySettings, ft f } return &LokiHistorianStore{ - client: historian.NewLokiClient(lokiCfg, historian.NewRequester(), ngmetrics.NewHistorianMetrics(prometheus.DefaultRegisterer, subsystem), log), + client: historian.NewLokiClient(lokiCfg, historian.NewRequester(), ngmetrics.NewHistorianMetrics(prometheus.DefaultRegisterer, subsystem), log, tracer), db: db, log: log, } diff --git a/pkg/services/ngalert/client/client.go b/pkg/services/ngalert/client/client.go index 90b080c0bc6..b6c99d32be4 100644 --- a/pkg/services/ngalert/client/client.go +++ b/pkg/services/ngalert/client/client.go @@ -7,6 +7,11 @@ import ( "strconv" "github.com/grafana/dskit/instrument" + "github.com/grafana/grafana/pkg/infra/tracing" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + semconv "go.opentelemetry.io/otel/semconv/v1.17.0" + "go.opentelemetry.io/otel/trace" ) // Requester executes an HTTP request. @@ -14,7 +19,7 @@ type Requester interface { Do(req *http.Request) (*http.Response, error) } -// TimedClient instruments a request. It implements Requester. +// TimedClient instruments a request with metrics. It implements Requester. type TimedClient struct { client Requester collector instrument.Collector @@ -70,3 +75,51 @@ func TimeRequest(ctx context.Context, operation string, coll instrument.Collecto coll, toStatusCode, doRequest) return response, err } + +// TracedClient instruments a request with tracing. It implements Requester. +type TracedClient struct { + client Requester + tracer tracing.Tracer + name string +} + +func NewTracedClient(client Requester, tracer tracing.Tracer, name string) *TracedClient { + return &TracedClient{ + client: client, + tracer: tracer, + name: name, + } +} + +// Do executes the request. +func (c TracedClient) Do(r *http.Request) (*http.Response, error) { + ctx, span := c.tracer.Start(r.Context(), c.name, trace.WithSpanKind(trace.SpanKindClient)) + defer span.End() + + span.SetAttributes(semconv.HTTPURL(r.URL.String())) + span.SetAttributes(semconv.HTTPMethod(r.Method)) + + c.tracer.Inject(ctx, r.Header, span) + + r = r.WithContext(ctx) + resp, err := c.client.Do(r) + if err != nil { + span.SetStatus(codes.Error, "request failed") + span.RecordError(err) + } else { + if resp.ContentLength > 0 { + span.SetAttributes(attribute.Int64("http.content_length", resp.ContentLength)) + } + span.SetAttributes(semconv.HTTPStatusCode(resp.StatusCode)) + if resp.StatusCode >= 400 && resp.StatusCode < 600 { + span.RecordError(fmt.Errorf("error with HTTP status code %d", resp.StatusCode)) + } + } + + return resp, err +} + +// RoundTrip implements the RoundTripper interface. +func (c TracedClient) RoundTrip(r *http.Request) (*http.Response, error) { + return c.Do(r) +} diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 5f87cafc251..ec86f1ed5f0 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -200,7 +200,7 @@ func (ng *AlertNG) init() error { PromoteConfig: true, SyncInterval: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.SyncInterval, } - remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m) + remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m, ng.tracer) if err != nil { moaLogger.Error("Failed to create remote Alertmanager", "err", err) return nil, err @@ -234,7 +234,7 @@ func (ng *AlertNG) init() error { TenantID: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.TenantID, URL: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.URL, } - remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m) + remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m, ng.tracer) if err != nil { moaLogger.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) return internalAM, nil @@ -270,7 +270,7 @@ func (ng *AlertNG) init() error { URL: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.URL, SyncInterval: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.SyncInterval, } - remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m) + remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m, ng.tracer) if err != nil { moaLogger.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) return internalAM, nil @@ -359,7 +359,7 @@ func (ng *AlertNG) init() error { // There are a set of feature toggles available that act as short-circuits for common configurations. // If any are set, override the config accordingly. ApplyStateHistoryFeatureToggles(&ng.Cfg.UnifiedAlerting.StateHistory, ng.FeatureToggles, ng.Log) - history, err := configureHistorianBackend(initCtx, ng.Cfg.UnifiedAlerting.StateHistory, ng.annotationsRepo, ng.dashboardService, ng.store, ng.Metrics.GetHistorianMetrics(), ng.Log) + history, err := configureHistorianBackend(initCtx, ng.Cfg.UnifiedAlerting.StateHistory, ng.annotationsRepo, ng.dashboardService, ng.store, ng.Metrics.GetHistorianMetrics(), ng.Log, ng.tracer) if err != nil { return err } @@ -523,7 +523,7 @@ type Historian interface { state.Historian } -func configureHistorianBackend(ctx context.Context, cfg setting.UnifiedAlertingStateHistorySettings, ar annotations.Repository, ds dashboards.DashboardService, rs historian.RuleStore, met *metrics.Historian, l log.Logger) (Historian, error) { +func configureHistorianBackend(ctx context.Context, cfg setting.UnifiedAlertingStateHistorySettings, ar annotations.Repository, ds dashboards.DashboardService, rs historian.RuleStore, met *metrics.Historian, l log.Logger, tracer tracing.Tracer) (Historian, error) { if !cfg.Enabled { met.Info.WithLabelValues("noop").Set(0) return historian.NewNopHistorian(), nil @@ -538,7 +538,7 @@ func configureHistorianBackend(ctx context.Context, cfg setting.UnifiedAlertingS if backend == historian.BackendTypeMultiple { primaryCfg := cfg primaryCfg.Backend = cfg.MultiPrimary - primary, err := configureHistorianBackend(ctx, primaryCfg, ar, ds, rs, met, l) + primary, err := configureHistorianBackend(ctx, primaryCfg, ar, ds, rs, met, l, tracer) if err != nil { return nil, fmt.Errorf("multi-backend target \"%s\" was misconfigured: %w", cfg.MultiPrimary, err) } @@ -547,7 +547,7 @@ func configureHistorianBackend(ctx context.Context, cfg setting.UnifiedAlertingS for _, b := range cfg.MultiSecondaries { secCfg := cfg secCfg.Backend = b - sec, err := configureHistorianBackend(ctx, secCfg, ar, ds, rs, met, l) + sec, err := configureHistorianBackend(ctx, secCfg, ar, ds, rs, met, l, tracer) if err != nil { return nil, fmt.Errorf("multi-backend target \"%s\" was miconfigured: %w", b, err) } @@ -569,7 +569,7 @@ func configureHistorianBackend(ctx context.Context, cfg setting.UnifiedAlertingS } req := historian.NewRequester() lokiBackendLogger := log.New("ngalert.state.historian", "backend", "loki") - backend := historian.NewRemoteLokiBackend(lokiBackendLogger, lcfg, req, met) + backend := historian.NewRemoteLokiBackend(lokiBackendLogger, lcfg, req, met, tracer) testConnCtx, cancelFunc := context.WithTimeout(ctx, 10*time.Second) defer cancelFunc() @@ -627,8 +627,8 @@ func ApplyStateHistoryFeatureToggles(cfg *setting.UnifiedAlertingStateHistorySet } } -func createRemoteAlertmanager(cfg remote.AlertmanagerConfig, kvstore kvstore.KVStore, decryptFn remote.DecryptFn, autogenFn remote.AutogenFn, m *metrics.RemoteAlertmanager) (*remote.Alertmanager, error) { - return remote.NewAlertmanager(cfg, notifier.NewFileStore(cfg.OrgID, kvstore), decryptFn, autogenFn, m) +func createRemoteAlertmanager(cfg remote.AlertmanagerConfig, kvstore kvstore.KVStore, decryptFn remote.DecryptFn, autogenFn remote.AutogenFn, m *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*remote.Alertmanager, error) { + return remote.NewAlertmanager(cfg, notifier.NewFileStore(cfg.OrgID, kvstore), decryptFn, autogenFn, m, tracer) } func createRecordingWriter(featureToggles featuremgmt.FeatureToggles, settings setting.RecordingRuleSettings) (schedule.RecordingWriter, error) { diff --git a/pkg/services/ngalert/ngalert_test.go b/pkg/services/ngalert/ngalert_test.go index ed7bee52d7c..cb9b9863349 100644 --- a/pkg/services/ngalert/ngalert_test.go +++ b/pkg/services/ngalert/ngalert_test.go @@ -62,12 +62,13 @@ func TestConfigureHistorianBackend(t *testing.T) { t.Run("fail initialization if invalid backend", func(t *testing.T) { met := metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem) logger := log.NewNopLogger() + tracer := tracing.InitializeTracerForTest() cfg := setting.UnifiedAlertingStateHistorySettings{ Enabled: true, Backend: "invalid-backend", } - _, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger) + _, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger, tracer) require.ErrorContains(t, err, "unrecognized") }) @@ -75,13 +76,14 @@ func TestConfigureHistorianBackend(t *testing.T) { t.Run("fail initialization if invalid multi-backend primary", func(t *testing.T) { met := metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem) logger := log.NewNopLogger() + tracer := tracing.InitializeTracerForTest() cfg := setting.UnifiedAlertingStateHistorySettings{ Enabled: true, Backend: "multiple", MultiPrimary: "invalid-backend", } - _, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger) + _, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger, tracer) require.ErrorContains(t, err, "multi-backend target") require.ErrorContains(t, err, "unrecognized") @@ -90,6 +92,7 @@ func TestConfigureHistorianBackend(t *testing.T) { t.Run("fail initialization if invalid multi-backend secondary", func(t *testing.T) { met := metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem) logger := log.NewNopLogger() + tracer := tracing.InitializeTracerForTest() cfg := setting.UnifiedAlertingStateHistorySettings{ Enabled: true, Backend: "multiple", @@ -97,7 +100,7 @@ func TestConfigureHistorianBackend(t *testing.T) { MultiSecondaries: []string{"annotations", "invalid-backend"}, } - _, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger) + _, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger, tracer) require.ErrorContains(t, err, "multi-backend target") require.ErrorContains(t, err, "unrecognized") @@ -106,6 +109,7 @@ func TestConfigureHistorianBackend(t *testing.T) { t.Run("do not fail initialization if pinging Loki fails", func(t *testing.T) { met := metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem) logger := log.NewNopLogger() + tracer := tracing.InitializeTracerForTest() cfg := setting.UnifiedAlertingStateHistorySettings{ Enabled: true, Backend: "loki", @@ -114,7 +118,7 @@ func TestConfigureHistorianBackend(t *testing.T) { LokiWriteURL: "http://gone.invalid", } - h, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger) + h, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger, tracer) require.NotNil(t, h) require.NoError(t, err) @@ -124,12 +128,13 @@ func TestConfigureHistorianBackend(t *testing.T) { reg := prometheus.NewRegistry() met := metrics.NewHistorianMetrics(reg, metrics.Subsystem) logger := log.NewNopLogger() + tracer := tracing.InitializeTracerForTest() cfg := setting.UnifiedAlertingStateHistorySettings{ Enabled: true, Backend: "annotations", } - h, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger) + h, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger, tracer) require.NotNil(t, h) require.NoError(t, err) @@ -146,11 +151,12 @@ grafana_alerting_state_history_info{backend="annotations"} 1 reg := prometheus.NewRegistry() met := metrics.NewHistorianMetrics(reg, metrics.Subsystem) logger := log.NewNopLogger() + tracer := tracing.InitializeTracerForTest() cfg := setting.UnifiedAlertingStateHistorySettings{ Enabled: false, } - h, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger) + h, err := configureHistorianBackend(context.Background(), cfg, nil, nil, nil, met, logger, tracer) require.NotNil(t, h) require.NoError(t, err) diff --git a/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go b/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go index 35810085b11..81573c42061 100644 --- a/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go +++ b/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -66,7 +67,7 @@ func TestMultiorgAlertmanager_RemoteSecondaryMode(t *testing.T) { DefaultConfig: setting.GetAlertmanagerDefaultConfiguration(), } m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - remoteAM, err := remote.NewAlertmanager(externalAMCfg, notifier.NewFileStore(orgID, kvStore), secretsService.Decrypt, remote.NoopAutogenFn, m) + remoteAM, err := remote.NewAlertmanager(externalAMCfg, notifier.NewFileStore(orgID, kvStore), secretsService.Decrypt, remote.NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // Use both Alertmanager implementations in the forked Alertmanager. diff --git a/pkg/services/ngalert/remote/alertmanager.go b/pkg/services/ngalert/remote/alertmanager.go index 8d460a756cb..4feca91ed4b 100644 --- a/pkg/services/ngalert/remote/alertmanager.go +++ b/pkg/services/ngalert/remote/alertmanager.go @@ -25,6 +25,7 @@ import ( "gopkg.in/yaml.v3" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -100,7 +101,7 @@ func (cfg *AlertmanagerConfig) Validate() error { return nil } -func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn DecryptFn, autogenFn AutogenFn, metrics *metrics.RemoteAlertmanager) (*Alertmanager, error) { +func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn DecryptFn, autogenFn AutogenFn, metrics *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*Alertmanager, error) { if err := cfg.Validate(); err != nil { return nil, err } @@ -118,7 +119,7 @@ func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn Decrypt URL: u, PromoteConfig: cfg.PromoteConfig, } - mc, err := remoteClient.New(mcCfg, metrics) + mc, err := remoteClient.New(mcCfg, metrics, tracer) if err != nil { return nil, err } @@ -129,7 +130,7 @@ func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn Decrypt Password: cfg.BasicAuthPassword, Logger: logger, } - amc, err := remoteClient.NewAlertmanager(amcCfg, metrics) + amc, err := remoteClient.NewAlertmanager(amcCfg, metrics, tracer) if err != nil { return nil, err } diff --git a/pkg/services/ngalert/remote/alertmanager_test.go b/pkg/services/ngalert/remote/alertmanager_test.go index dffb38e4644..5016ab49e38 100644 --- a/pkg/services/ngalert/remote/alertmanager_test.go +++ b/pkg/services/ngalert/remote/alertmanager_test.go @@ -23,6 +23,7 @@ import ( "github.com/grafana/alerting/definition" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/metrics" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -105,7 +106,7 @@ func TestNewAlertmanager(t *testing.T) { DefaultConfig: defaultGrafanaConfig, } m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m) + am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) if test.expErr != "" { require.EqualError(tt, err, test.expErr) return @@ -177,7 +178,7 @@ func TestApplyConfig(t *testing.T) { // An error response from the remote Alertmanager should result in the readiness check failing. m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m) + am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) config := &ngmodels.AlertConfiguration{ @@ -305,6 +306,7 @@ func TestCompareAndSendConfiguration(t *testing.T) { decryptFn, test.autogenFn, m, + tracing.InitializeTracerForTest(), ) require.NoError(t, err) @@ -364,7 +366,7 @@ func TestIntegrationRemoteAlertmanagerConfiguration(t *testing.T) { secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m) + am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) encodedFullState, err := am.getFullState(ctx) @@ -521,7 +523,7 @@ func TestIntegrationRemoteAlertmanagerGetStatus(t *testing.T) { secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m) + am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // We should get the default Cloud Alertmanager configuration. @@ -555,7 +557,7 @@ func TestIntegrationRemoteAlertmanagerSilences(t *testing.T) { secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m) + am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // We should have no silences at first. @@ -640,7 +642,7 @@ func TestIntegrationRemoteAlertmanagerAlerts(t *testing.T) { secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m) + am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // Wait until the Alertmanager is ready to send alerts. @@ -709,7 +711,7 @@ func TestIntegrationRemoteAlertmanagerReceivers(t *testing.T) { secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m) + am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // We should start with the default config. diff --git a/pkg/services/ngalert/remote/client/alertmanager.go b/pkg/services/ngalert/remote/client/alertmanager.go index 88a7ca7638b..52ab72b2723 100644 --- a/pkg/services/ngalert/remote/client/alertmanager.go +++ b/pkg/services/ngalert/remote/client/alertmanager.go @@ -9,6 +9,7 @@ import ( httptransport "github.com/go-openapi/runtime/client" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/ngalert/client" "github.com/grafana/grafana/pkg/services/ngalert/metrics" amclient "github.com/prometheus/alertmanager/api/v2/client" @@ -31,7 +32,7 @@ type Alertmanager struct { logger log.Logger } -func NewAlertmanager(cfg *AlertmanagerConfig, metrics *metrics.RemoteAlertmanager) (*Alertmanager, error) { +func NewAlertmanager(cfg *AlertmanagerConfig, metrics *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*Alertmanager, error) { // First, add the authentication middleware. c := &http.Client{Transport: &MimirAuthRoundTripper{ TenantID: cfg.TenantID, @@ -40,14 +41,15 @@ func NewAlertmanager(cfg *AlertmanagerConfig, metrics *metrics.RemoteAlertmanage }} tc := client.NewTimedClient(c, metrics.RequestLatency) + trc := client.NewTracedClient(tc, tracer, "remote.alertmanager.client") apiEndpoint := *cfg.URL // Next, make sure you set the right path. u := apiEndpoint.JoinPath(alertmanagerAPIMountPath, amclient.DefaultBasePath) - // Create an Alertmanager client using the timed client as the transport. + // Create an Alertmanager client using the instrumented client as the transport. r := httptransport.New(u.Host, u.Path, []string{u.Scheme}) - r.Transport = tc + r.Transport = trc return &Alertmanager{ logger: cfg.Logger, diff --git a/pkg/services/ngalert/remote/client/mimir.go b/pkg/services/ngalert/remote/client/mimir.go index b99864af510..cf0a0941652 100644 --- a/pkg/services/ngalert/remote/client/mimir.go +++ b/pkg/services/ngalert/remote/client/mimir.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/client" "github.com/grafana/grafana/pkg/services/ngalert/metrics" @@ -68,7 +69,7 @@ func (e *errorResponse) Error() string { return e.Error2 } -func New(cfg *Config, metrics *metrics.RemoteAlertmanager) (*Mimir, error) { +func New(cfg *Config, metrics *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*Mimir, error) { rt := &MimirAuthRoundTripper{ TenantID: cfg.TenantID, Password: cfg.Password, @@ -78,10 +79,12 @@ func New(cfg *Config, metrics *metrics.RemoteAlertmanager) (*Mimir, error) { c := &http.Client{ Transport: rt, } + tc := client.NewTimedClient(c, metrics.RequestLatency) + trc := client.NewTracedClient(tc, tracer, "remote.alertmanager.client") return &Mimir{ endpoint: cfg.URL, - client: client.NewTimedClient(c, metrics.RequestLatency), + client: trc, logger: cfg.Logger, metrics: metrics, promoteConfig: cfg.PromoteConfig, diff --git a/pkg/services/ngalert/state/historian/loki.go b/pkg/services/ngalert/state/historian/loki.go index 72add63518d..fd163a3a806 100644 --- a/pkg/services/ngalert/state/historian/loki.go +++ b/pkg/services/ngalert/state/historian/loki.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/ngalert/client" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/metrics" @@ -55,9 +56,9 @@ type RemoteLokiBackend struct { log log.Logger } -func NewRemoteLokiBackend(logger log.Logger, cfg LokiConfig, req client.Requester, metrics *metrics.Historian) *RemoteLokiBackend { +func NewRemoteLokiBackend(logger log.Logger, cfg LokiConfig, req client.Requester, metrics *metrics.Historian, tracer tracing.Tracer) *RemoteLokiBackend { return &RemoteLokiBackend{ - client: NewLokiClient(cfg, req, metrics, logger), + client: NewLokiClient(cfg, req, metrics, logger, tracer), externalLabels: cfg.ExternalLabels, clock: clock.New(), metrics: metrics, diff --git a/pkg/services/ngalert/state/historian/loki_http.go b/pkg/services/ngalert/state/historian/loki_http.go index 44a7c0044eb..2d9ca11ba13 100644 --- a/pkg/services/ngalert/state/historian/loki_http.go +++ b/pkg/services/ngalert/state/historian/loki_http.go @@ -12,6 +12,7 @@ import ( "time" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/ngalert/client" "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/setting" @@ -103,10 +104,11 @@ const ( NeqRegEx Operator = "!~" ) -func NewLokiClient(cfg LokiConfig, req client.Requester, metrics *metrics.Historian, logger log.Logger) *HttpLokiClient { +func NewLokiClient(cfg LokiConfig, req client.Requester, metrics *metrics.Historian, logger log.Logger, tracer tracing.Tracer) *HttpLokiClient { tc := client.NewTimedClient(req, metrics.WriteDuration) + trc := client.NewTracedClient(tc, tracer, "ngalert.historian.client") return &HttpLokiClient{ - client: tc, + client: trc, encoder: cfg.Encoder, cfg: cfg, metrics: metrics, diff --git a/pkg/services/ngalert/state/historian/loki_http_test.go b/pkg/services/ngalert/state/historian/loki_http_test.go index 961f3238097..0c81eca1ee4 100644 --- a/pkg/services/ngalert/state/historian/loki_http_test.go +++ b/pkg/services/ngalert/state/historian/loki_http_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" ) func TestLokiConfig(t *testing.T) { @@ -227,7 +228,7 @@ func TestLokiHTTPClient_Manual(t *testing.T) { ReadPathURL: url, WritePathURL: url, Encoder: JsonEncoder{}, - }, NewRequester(), metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem), log.NewNopLogger()) + }, NewRequester(), metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem), log.NewNopLogger(), tracing.InitializeTracerForTest()) // Unauthorized request should fail against Grafana Cloud. err = client.Ping(context.Background()) @@ -255,7 +256,7 @@ func TestLokiHTTPClient_Manual(t *testing.T) { BasicAuthUser: "", BasicAuthPassword: "", Encoder: JsonEncoder{}, - }, NewRequester(), metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem), log.NewNopLogger()) + }, NewRequester(), metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem), log.NewNopLogger(), tracing.InitializeTracerForTest()) // When running on prem, you might need to set the tenant id, // so the x-scope-orgid header is set. @@ -389,7 +390,7 @@ func createTestLokiClient(req client.Requester) *HttpLokiClient { Encoder: JsonEncoder{}, } met := metrics.NewHistorianMetrics(prometheus.NewRegistry(), metrics.Subsystem) - return NewLokiClient(cfg, req, met, log.NewNopLogger()) + return NewLokiClient(cfg, req, met, log.NewNopLogger(), tracing.InitializeTracerForTest()) } func reqBody(t *testing.T, req *http.Request) string { diff --git a/pkg/services/ngalert/state/historian/loki_test.go b/pkg/services/ngalert/state/historian/loki_test.go index dbdefd0c51c..dad1e0503a0 100644 --- a/pkg/services/ngalert/state/historian/loki_test.go +++ b/pkg/services/ngalert/state/historian/loki_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/ngalert/client" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/metrics" @@ -514,7 +515,7 @@ func createTestLokiBackend(req client.Requester, met *metrics.Historian) *Remote ExternalLabels: map[string]string{"externalLabelKey": "externalLabelValue"}, } lokiBackendLogger := log.New("ngalert.state.historian", "backend", "loki") - return NewRemoteLokiBackend(lokiBackendLogger, cfg, req, met) + return NewRemoteLokiBackend(lokiBackendLogger, cfg, req, met, tracing.InitializeTracerForTest()) } func singleFromNormal(st *state.State) []state.StateTransition { diff --git a/pkg/services/publicdashboards/service/common_test.go b/pkg/services/publicdashboards/service/common_test.go index 1124b687288..959ed23849b 100644 --- a/pkg/services/publicdashboards/service/common_test.go +++ b/pkg/services/publicdashboards/service/common_test.go @@ -5,6 +5,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/annotations/annotationsimpl" "github.com/grafana/grafana/pkg/services/dashboards" @@ -29,7 +30,7 @@ func newPublicDashboardServiceImpl( db, cfg := db.InitTestDBWithCfg(t) tagService := tagimpl.ProvideService(db) if annotationsRepo == nil { - annotationsRepo = annotationsimpl.ProvideService(db, cfg, featuremgmt.WithFeatures(), tagService) + annotationsRepo = annotationsimpl.ProvideService(db, cfg, featuremgmt.WithFeatures(), tagService, tracing.InitializeTracerForTest()) } if publicDashboardStore == nil { From b9812a07841dd68c6a316fd0e797f1c0419a7ca5 Mon Sep 17 00:00:00 2001 From: Diego Augusto Molina Date: Sat, 15 Jun 2024 02:46:14 -0300 Subject: [PATCH 04/47] Unified Storage: Fix data races and context usage in broadcaster (#88955) * Fix several broadcaster data races and error handling - Separate concerns between sender and receiver sides in channel usage - broadcaster: Fix data race between Subscribe/Unsubscribe and start - Fix Subscribe error to be io.EOF when broadcaster is terminated - Fix Watch never unsubscribing - General cleanup - fix usage of context - add a huge amount of documentation about channels --- .../store/entity/sqlstash/broadcaster.go | 226 +++++++++++++----- .../entity/sqlstash/sql_storage_server.go | 23 +- 2 files changed, 181 insertions(+), 68 deletions(-) diff --git a/pkg/services/store/entity/sqlstash/broadcaster.go b/pkg/services/store/entity/sqlstash/broadcaster.go index 5c805eb6473..56a010bf2a0 100644 --- a/pkg/services/store/entity/sqlstash/broadcaster.go +++ b/pkg/services/store/entity/sqlstash/broadcaster.go @@ -3,18 +3,97 @@ package sqlstash import ( "context" "fmt" + "io" ) -type ConnectFunc[T any] func(chan T) error +// Please, when reviewing or working on this file have the following cheat-sheet +// in mind: +// 1. A channel type in Go has one of three directions: send-only (chan<- T), +// receive-only (<-chan T) or bidirctional (chan T). Each of them are a +// different type. A bidirectional type can be converted to any of the other +// two types and is automatic, any other conversion attempt results in a +// panic. +// 2. There are three operations you can do on a channel: send, receive and +// close. Availability of operation for each channel direction: +// | Channel direction +// Operation | Receive-only | Send-only | Bidirectional +// ----------+--------------+------------+-------------- +// Receive | Yes | No (panic) | Yes +// Send | No (panic) | Yes | Yes +// Close | No (panic) | Yes | Yes +// 3. A channel of any type also has one of three states: nil (zero value), +// closed, or open (technically called "non-nil, not-closed channel", +// created with the `make` builtin). Nil and closed channels are also +// useful, but you have to know and care for how you use them. Outcome of +// each operation on a channel depending on its state, assuming the +// operation is available to the channel given its direction: +// | Channel state +// Operation | Nil | Closed | Open +// ----------+---------------+---------------+------------------ +// Receive | Block forever | Block forever | Receive/Block until receive +// Send | Block forever | Panic | Send/Block until send +// Close | Panic | Panic | Close the channel +// 4. A `select` statement has zero or more `case` branches, each one of them +// containing either a send or a receive channel operation. A `select` with +// no branches blocks forever. At most one branch will be executed, which +// means it behaves similar to a `switch`. If more than one branch can be +// executed then one of them is picked AT RANDOM (i.e. not the one first in +// the list). A `select` statement can also have a (single and optional) +// `default` branch that is executed if all the other branches are +// operations that are blocked at the time the `select` statement is +// reached. This means that having a `default` branch causes the `select` +// statement to never block. +// 5. A receive operation on a closed channel never blocks (as said before), +// but it will always yield a zero value. As it is also valid to send a zero +// value to the channel, you can receive from channels in two forms: +// v := <-c // get a zero value if closed +// v2, ok := <-c // `ok` is set to false iif the channel is closed +// 6. The `make` builtin is used to create open channels (and is the only way +// to get them). It has an optional second parameter to specify the amount +// of items that can buffered. After that, a send operation will block +// waiting for another goroutine to receive from it (which would make room +// for the new item). When the second argument is not passed to `make`, then +// all operations are fully synchronized, meaning that a send will block +// until a receive in another goroutine is performed, and vice versa. Less +// interestingly, `make` can also create send-only or receive-only channel. +// +// The sources are the Go Specs, Effective Go and Go 101, which are already +// linked in the contributing guide for the backend or elsewhere in Grafana, but +// this file exploits so many of these subtleties that it's worth keeping a +// refresher about them at all times. The above is unlikely to change in the +// foreseeable future, so it's zero maintenance as well. We exclude patterns for +// using channels and other concurrency patterns since that's a way longer +// topic for a refresher. + +// ConnectFunc is used to initialize the watch implementation. It should do very +// basic work and checks and it has the chance to return an error. After that, +// it should fork to a different goroutine with the provided channel and send to +// it all the new events from the backing database. It is also responsible for +// closing the provided channel under all circumstances, included returning an +// error. The caller of this function will only receive from this channel (i.e. +// it is guaranteed to never send to it or close it), hence providing a safe +// separation of concerns and preventing panics. +// +// FIXME: this signature suffers from inversion of control. It would also be +// much simpler if NewBroadcaster receives a context.Context and a <-chan T +// instead. That would also reduce the scope of the broadcaster to only +// broadcast to subscribers what it receives on the provided <-chan T. The +// context.Context is still needed to provide additional values in case we want +// to add observability into the broadcaster, which we want. The broadcaster +// should still terminate on either the context being done or the provided +// channel being closed. +type ConnectFunc[T any] func(chan<- T) error type Broadcaster[T any] interface { Subscribe(context.Context) (<-chan T, error) - Unsubscribe(chan T) + Unsubscribe(<-chan T) } func NewBroadcaster[T any](ctx context.Context, connect ConnectFunc[T]) (Broadcaster[T], error) { - b := &broadcaster[T]{} - err := b.start(ctx, connect) + b := &broadcaster[T]{ + started: make(chan struct{}), + } + err := b.init(ctx, connect) if err != nil { return nil, err } @@ -23,101 +102,132 @@ func NewBroadcaster[T any](ctx context.Context, connect ConnectFunc[T]) (Broadca } type broadcaster[T any] struct { - running bool // FIXME: race condition between `Subscribe`/`Unsubscribe` and `start` - ctx context.Context - subs map[chan T]struct{} + // lifecycle management + + started, terminated chan struct{} + shouldTerminate <-chan struct{} + + // subscription management + cache Cache[T] subscribe chan chan T - unsubscribe chan chan T + unsubscribe chan (<-chan T) + subs map[<-chan T]chan T } func (b *broadcaster[T]) Subscribe(ctx context.Context) (<-chan T, error) { - if !b.running { - return nil, fmt.Errorf("broadcaster not running") + select { + case <-ctx.Done(): // client canceled + return nil, ctx.Err() + case <-b.started: // wait for broadcaster to start } + // create the subscription sub := make(chan T, 100) - b.subscribe <- sub - go func() { - <-ctx.Done() - b.unsubscribe <- sub - }() - return sub, nil -} - -func (b *broadcaster[T]) Unsubscribe(sub chan T) { - b.unsubscribe <- sub -} - -func (b *broadcaster[T]) start(ctx context.Context, connect ConnectFunc[T]) error { - if b.running { - return fmt.Errorf("broadcaster already running") + select { + case <-ctx.Done(): // client canceled + return nil, ctx.Err() + case <-b.terminated: // no more data + return nil, io.EOF + case b.subscribe <- sub: // success submitting subscription + return sub, nil } +} +func (b *broadcaster[T]) Unsubscribe(sub <-chan T) { + // wait for broadcaster to start. In practice, the only way to reach + // Unsubscribe is by first having called Subscribe, which means we have + // already started. But a malfunctioning caller may call Unsubscribe freely, + // which would cause us to block forever the goroutine of the caller when + // trying to send to a nil `b.unsubscribe` or receive from a nil + // `b.terminated` if we haven't yet initialized those values. This would + // mean leaking that malfunctioninig caller's goroutine, so we rather make + // Unsubscribe safe in any possible case + if sub == nil { + return + } + <-b.started // wait for broadcaster to start + + select { + case b.unsubscribe <- sub: // success submitting unsubscription + case <-b.terminated: // broadcaster terminated, nothing to do + } +} + +// init initializes the broadcaster. It should not be run more than once. +func (b *broadcaster[T]) init(ctx context.Context, connect ConnectFunc[T]) error { + // create the stream that will connect us with the watch implementation and + // send it to them so they initialize and start sending data stream := make(chan T, 100) - - err := connect(stream) - if err != nil { + if err := connect(stream); err != nil { return err } - b.ctx = ctx - + // initialize our internal state + b.shouldTerminate = ctx.Done() b.cache = NewCache[T](ctx, 100) b.subscribe = make(chan chan T, 100) - b.unsubscribe = make(chan chan T, 100) - b.subs = make(map[chan T]struct{}) + b.unsubscribe = make(chan (<-chan T), 100) + b.subs = make(map[<-chan T]chan T) + b.terminated = make(chan struct{}) + // start handling incoming data from the watch implementation. If data came + // in until now, it will be buffered in `stream` go b.stream(stream) - b.running = true + // unblock any Subscribe/Unsubscribe calls since we are ready to handle them + close(b.started) + return nil } -func (b *broadcaster[T]) stream(input chan T) { +// stream acts a message broker between the watch implementation that receives a +// raw stream of events and the individual clients watching for those events. +// Thus, we hold the receive side of the watch implementation, and we are +// limited here to receive from it, whereas we are responsible for sending to +// watchers and closing their channels. The responsibility of closing `input` +// (as with any other channel) will always be of the sending side. Hence, the +// watch implementation should do it. +func (b *broadcaster[T]) stream(input <-chan T) { + // make sure we unconditionally cleanup upon return + defer func() { + // prevent new subscriptions and make sure to discard unsubscriptions + close(b.terminated) + // terminate all subscirptions and clean the map + for _, sub := range b.subs { + close(sub) + delete(b.subs, sub) + } + }() + for { select { - // context cancelled - case <-b.ctx.Done(): - close(input) - for sub := range b.subs { - close(sub) - delete(b.subs, sub) - } - b.running = false + case <-b.shouldTerminate: // service context cancelled return - // new subscriber - case sub := <-b.subscribe: + + case sub := <-b.subscribe: // subscribe // send initial batch of cached items err := b.cache.ReadInto(sub) if err != nil { close(sub) continue } + b.subs[sub] = sub - b.subs[sub] = struct{}{} - // unsubscribe - case sub := <-b.unsubscribe: - if _, ok := b.subs[sub]; ok { + case recv := <-b.unsubscribe: // unsubscribe + if sub, ok := b.subs[recv]; ok { close(sub) delete(b.subs, sub) } - // read item from input - case item, ok := <-input: + + case item, ok := <-input: // data arrived, send to subscribers // input closed, drain subscribers and exit if !ok { - for sub := range b.subs { - close(sub) - delete(b.subs, sub) - } - b.running = false return } - b.cache.Add(item) - - for sub := range b.subs { + for _, sub := range b.subs { select { case sub <- item: default: diff --git a/pkg/services/store/entity/sqlstash/sql_storage_server.go b/pkg/services/store/entity/sqlstash/sql_storage_server.go index 76024024dae..0a7d7cce617 100644 --- a/pkg/services/store/entity/sqlstash/sql_storage_server.go +++ b/pkg/services/store/entity/sqlstash/sql_storage_server.go @@ -63,6 +63,9 @@ func ProvideSQLEntityServer(db db.EntityDBInterface, tracer tracing.Tracer /*, c type SqlEntityServer interface { entity.EntityStoreServer + // FIXME: accpet a context.Context in the lifecycle methods, and Stop should + // also return an error. + Init() error Stop() } @@ -75,7 +78,6 @@ type sqlEntityServer struct { broadcaster Broadcaster[*entity.EntityWatchResponse] ctx context.Context // TODO: remove cancel context.CancelFunc - stream chan *entity.EntityWatchResponse tracer trace.Tracer once sync.Once @@ -139,9 +141,7 @@ func (s *sqlEntityServer) init() error { s.dialect = migrator.NewDialect(engine.DriverName()) // set up the broadcaster - s.broadcaster, err = NewBroadcaster(s.ctx, func(stream chan *entity.EntityWatchResponse) error { - s.stream = stream - + s.broadcaster, err = NewBroadcaster(s.ctx, func(stream chan<- *entity.EntityWatchResponse) error { // start the poller go s.poller(stream) @@ -994,13 +994,18 @@ func (s *sqlEntityServer) watchInit(ctx context.Context, r *entity.EntityWatchRe return lastRv, nil } -func (s *sqlEntityServer) poller(stream chan *entity.EntityWatchResponse) { +func (s *sqlEntityServer) poller(stream chan<- *entity.EntityWatchResponse) { var err error + // FIXME: we need a way to state startup of server from a (Group, Resource) + // standpoint, and consider that new (Group, Resource) may be added to + // `kind_version`, so we should probably also poll for changes in there since := int64(0) + interval := 1 * time.Second t := time.NewTicker(interval) + defer close(stream) defer t.Stop() for { @@ -1017,7 +1022,7 @@ func (s *sqlEntityServer) poller(stream chan *entity.EntityWatchResponse) { } } -func (s *sqlEntityServer) poll(since int64, out chan *entity.EntityWatchResponse) (int64, error) { +func (s *sqlEntityServer) poll(since int64, out chan<- *entity.EntityWatchResponse) (int64, error) { ctx, span := s.tracer.Start(s.ctx, "storage_server.poll") defer span.End() ctxLogger := s.log.FromContext(log.WithContextualAttributes(ctx, []any{"method", "poll"})) @@ -1182,26 +1187,25 @@ func (s *sqlEntityServer) watch(r *entity.EntityWatchRequest, w entity.EntitySto if err != nil { return err } + defer s.broadcaster.Unsubscribe(evts) stop := make(chan struct{}) since := r.Since go func() { + defer close(stop) for { r, err := w.Recv() if errors.Is(err, io.EOF) { s.log.Debug("watch client closed stream") - stop <- struct{}{} return } if err != nil { s.log.Error("error receiving message", "err", err) - stop <- struct{}{} return } if r.Action == entity.EntityWatchRequest_STOP { s.log.Debug("watch stop requested") - stop <- struct{}{} return } // handle any other message types @@ -1211,7 +1215,6 @@ func (s *sqlEntityServer) watch(r *entity.EntityWatchRequest, w entity.EntitySto for { select { - // stop signal case <-stop: s.log.Debug("watch stopped") return nil From ae80ed02e413022dd1c658e280c7778e72fc7160 Mon Sep 17 00:00:00 2001 From: Dominik Prokop Date: Mon, 17 Jun 2024 09:14:27 +0200 Subject: [PATCH 05/47] DashboardScene: Emit meta analytic view event (#89094) * DashboardScene: Emit view event * check fix --- .../pages/DashboardScenePageStateManager.ts | 10 ++++++++++ .../app/features/dashboard/state/analyticsProcessor.ts | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts index f0235de5a61..30e7064eb2a 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts +++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts @@ -5,6 +5,7 @@ import { default as localStorageStore } from 'app/core/store'; import { startMeasure, stopMeasure } from 'app/core/utils/metrics'; import { dashboardLoaderSrv } from 'app/features/dashboard/services/DashboardLoaderSrv'; import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; +import { emitDashboardViewEvent } from 'app/features/dashboard/state/analyticsProcessor'; import { DASHBOARD_FROM_LS_KEY, removeDashboardToFetchFromLocalStorage, @@ -185,6 +186,15 @@ export class DashboardScenePageStateManager extends StateManagerBase) { const eventData: DashboardViewEventPayload = { /** @deprecated */ dashboardId: dashboard.id, From 0107754da8b9334d16703d8eee20e410ad9dcb05 Mon Sep 17 00:00:00 2001 From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Date: Mon, 17 Jun 2024 10:39:28 +0200 Subject: [PATCH 06/47] Select: Add orange indicator to selected item (#88695) * Initial experiment * Add pill and underline * Text decoration for hover * Only set underline on the title * Remove underline from hover * Remove underline alltogether --- .../src/components/Select/getSelectStyles.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/grafana-ui/src/components/Select/getSelectStyles.ts b/packages/grafana-ui/src/components/Select/getSelectStyles.ts index 0d89a3fa519..a75b79417a3 100644 --- a/packages/grafana-ui/src/components/Select/getSelectStyles.ts +++ b/packages/grafana-ui/src/components/Select/getSelectStyles.ts @@ -17,6 +17,7 @@ export const getSelectStyles = stylesFactory((theme: GrafanaTheme2) => { option: css({ label: 'grafana-select-option', padding: '8px', + position: 'relative', display: 'flex', alignItems: 'center', flexDirection: 'row', @@ -64,6 +65,17 @@ export const getSelectStyles = stylesFactory((theme: GrafanaTheme2) => { }), optionSelected: css({ background: theme.colors.action.selected, + '&::before': { + backgroundImage: theme.colors.gradients.brandVertical, + borderRadius: theme.shape.radius.default, + content: '" "', + display: 'block', + height: '100%', + position: 'absolute', + transform: 'translateX(-50%)', + width: theme.spacing(0.5), + left: 0, + }, }), optionDisabled: css({ label: 'grafana-select-option-disabled', From d4bba872a10f0b2e121bb6a1b17305299fbb097c Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Mon, 17 Jun 2024 10:19:57 +0100 Subject: [PATCH 07/47] LibraryPanels: Use new folder picker when creating a library panel (#89228) LibraryPanels: Use new folder picker when creating a Library Panel --- .../AddLibraryPanelModal/AddLibraryPanelModal.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx b/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx index d9dfdc14d9a..1b393b3b8f0 100644 --- a/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx +++ b/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx @@ -4,7 +4,7 @@ import { useAsync, useDebounce } from 'react-use'; import { FetchError, isFetchError } from '@grafana/runtime'; import { LibraryPanel } from '@grafana/schema/dist/esm/index.gen'; import { Button, Field, Input, Modal } from '@grafana/ui'; -import { OldFolderPicker } from 'app/core/components/Select/OldFolderPicker'; +import { FolderPicker } from 'app/core/components/Select/FolderPicker'; import { t, Trans } from 'app/core/internationalization'; import { PanelModel } from '../../../dashboard/state'; @@ -36,6 +36,7 @@ export const AddLibraryPanelContents = ({ const onCreate = useCallback(() => { panel.libraryPanel = { uid: '', name: panelName }; + saveLibraryPanel(panel, folderUid!).then((res: LibraryPanel | FetchError) => { if (!isFetchError(res)) { onDismiss?.(); @@ -84,9 +85,9 @@ export const AddLibraryPanelContents = ({ 'Library panel permissions are derived from the folder permissions' )} > - setFolderUid(uid)} - initialFolderUid={initialFolderUid} + setFolderUid(uid)} + value={folderUid} inputId="share-panel-library-panel-folder-picker" /> From ab2af9b8f75cd13595f4d487c1168e849768a518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ida=20=C5=A0tambuk?= Date: Mon, 17 Jun 2024 11:41:50 +0200 Subject: [PATCH 08/47] Feature management: Add openSearchBackendFlowEnabled feature toggle (#89208) --- .../configure-grafana/feature-toggles/index.md | 1 + .../grafana-data/src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 6 ++++++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 ++++ pkg/services/featuremgmt/toggles_gen.json | 12 ++++++++++++ 6 files changed, 25 insertions(+) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 9bb322b34fa..99c345d9eb0 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -101,6 +101,7 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `groupToNestedTableTransformation` | Enables the group to nested table transformation | | `newPDFRendering` | New implementation for the dashboard-to-PDF rendering | | `ssoSettingsSAML` | Use the new SSO Settings API to configure the SAML connector | +| `openSearchBackendFlowEnabled` | Enables the backend query flow for Open Search datasource plugin | ## Experimental feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index fb7399368ca..634257b5886 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -194,4 +194,5 @@ export interface FeatureToggles { azureMonitorPrometheusExemplars?: boolean; pinNavItems?: boolean; authZGRPCServer?: boolean; + openSearchBackendFlowEnabled?: boolean; } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 17485739de4..3626238d106 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1317,6 +1317,12 @@ var ( HideFromAdminPage: true, HideFromDocs: true, }, + { + Name: "openSearchBackendFlowEnabled", + Description: "Enables the backend query flow for Open Search datasource plugin", + Stage: FeatureStagePublicPreview, + Owner: awsDatasourcesSquad, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 242c488b6fb..6e189240da4 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -175,3 +175,4 @@ pluginProxyPreserveTrailingSlash,GA,@grafana/plugins-platform-backend,false,fals azureMonitorPrometheusExemplars,experimental,@grafana/partner-datasources,false,false,false pinNavItems,experimental,@grafana/grafana-frontend-platform,false,false,false authZGRPCServer,experimental,@grafana/identity-access-team,false,false,false +openSearchBackendFlowEnabled,preview,@grafana/aws-datasources,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 9afc6b29e29..420c12da06f 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -710,4 +710,8 @@ const ( // FlagAuthZGRPCServer // Enables the gRPC server for authorization FlagAuthZGRPCServer = "authZGRPCServer" + + // FlagOpenSearchBackendFlowEnabled + // Enables the backend query flow for Open Search datasource plugin + FlagOpenSearchBackendFlowEnabled = "openSearchBackendFlowEnabled" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index de18acfb7be..70cda830cc7 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -1599,6 +1599,18 @@ "codeowner": "@grafana/grafana-operator-experience-squad" } }, + { + "metadata": { + "name": "openSearchBackendFlowEnabled", + "resourceVersion": "1718357852240", + "creationTimestamp": "2024-06-14T09:37:32Z" + }, + "spec": { + "description": "Enables the backend query flow for Open Search datasource plugin", + "stage": "preview", + "codeowner": "@grafana/aws-datasources" + } + }, { "metadata": { "name": "panelFilterVariable", From 67f2d93281c4a9eda59b16b5be726b07b28f8b16 Mon Sep 17 00:00:00 2001 From: Bogdan Matei Date: Mon, 17 Jun 2024 13:00:20 +0300 Subject: [PATCH 09/47] Scopes: QoL UI fixes (#89158) --- .../scene/Scopes/ScopesDashboardsScene.tsx | 2 +- .../scene/Scopes/ScopesFiltersScene.tsx | 95 +++++++------- .../scene/Scopes/ScopesInput.tsx | 119 ++++++++++++++++++ .../scene/Scopes/ScopesScene.test.tsx | 15 ++- .../scene/Scopes/ScopesScene.tsx | 2 +- .../scene/Scopes/ScopesTreeLevel.tsx | 23 +++- .../dashboard-scene/scene/Scopes/api.ts | 16 ++- .../scene/Scopes/testUtils.tsx | 2 +- .../dashboard-scene/scene/Scopes/types.ts | 12 +- public/locales/en-US/grafana.json | 5 +- public/locales/pseudo-LOCALE/grafana.json | 5 +- 11 files changed, 228 insertions(+), 68 deletions(-) create mode 100644 public/app/features/dashboard-scene/scene/Scopes/ScopesInput.tsx diff --git a/public/app/features/dashboard-scene/scene/Scopes/ScopesDashboardsScene.tsx b/public/app/features/dashboard-scene/scene/Scopes/ScopesDashboardsScene.tsx index 72b6c60055a..35ab7ea9be6 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/ScopesDashboardsScene.tsx +++ b/public/app/features/dashboard-scene/scene/Scopes/ScopesDashboardsScene.tsx @@ -74,7 +74,7 @@ export function ScopesDashboardsSceneRenderer({ model }: SceneComponentProps } - placeholder={t('scopes.suggestedDashboards.search', 'Filter')} + placeholder={t('scopes.suggestedDashboards.search', 'Search')} disabled={isLoading} data-testid="scopes-dashboards-search" onChange={(evt) => model.changeSearchQuery(evt.currentTarget.value)} diff --git a/public/app/features/dashboard-scene/scene/Scopes/ScopesFiltersScene.tsx b/public/app/features/dashboard-scene/scene/Scopes/ScopesFiltersScene.tsx index 341abd85e1e..cef966c5944 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/ScopesFiltersScene.tsx +++ b/public/app/features/dashboard-scene/scene/Scopes/ScopesFiltersScene.tsx @@ -13,19 +13,20 @@ import { SceneObjectUrlValues, SceneObjectWithUrlSync, } from '@grafana/scenes'; -import { Button, Drawer, IconButton, Input, Spinner, useStyles2 } from '@grafana/ui'; +import { Button, Drawer, Spinner, useStyles2 } from '@grafana/ui'; import { t, Trans } from 'app/core/internationalization'; +import { ScopesInput } from './ScopesInput'; import { ScopesScene } from './ScopesScene'; import { ScopesTreeLevel } from './ScopesTreeLevel'; -import { fetchNodes, fetchScope, fetchScopes } from './api'; -import { NodesMap } from './types'; +import { fetchNodes, fetchScope, fetchSelectedScopes } from './api'; +import { NodesMap, SelectedScope, TreeScope } from './types'; export interface ScopesFiltersSceneState extends SceneObjectState { nodes: NodesMap; loadingNodeName: string | undefined; - scopes: Scope[]; - dirtyScopeNames: string[]; + scopes: SelectedScope[]; + treeScopes: TreeScope[]; isLoadingScopes: boolean; isOpened: boolean; } @@ -57,7 +58,7 @@ export class ScopesFiltersScene extends SceneObjectBase }, loadingNodeName: undefined, scopes: [], - dirtyScopeNames: [], + treeScopes: [], isLoadingScopes: false, isOpened: false, }); @@ -72,14 +73,16 @@ export class ScopesFiltersScene extends SceneObjectBase } public getUrlState() { - return { scopes: this.getScopeNames() }; + return { + scopes: this.state.scopes.map(({ scope }) => scope.metadata.name), + }; } public updateFromUrl(values: SceneObjectUrlValues) { - let dirtyScopeNames = values.scopes ?? []; - dirtyScopeNames = Array.isArray(dirtyScopeNames) ? dirtyScopeNames : [dirtyScopeNames]; + let scopeNames = values.scopes ?? []; + scopeNames = Array.isArray(scopeNames) ? scopeNames : [scopeNames]; - this.updateScopes(dirtyScopeNames); + this.updateScopes(scopeNames.map((scopeName) => ({ scopeName, path: [] }))); } public fetchBaseNodes() { @@ -126,7 +129,7 @@ export class ScopesFiltersScene extends SceneObjectBase } public toggleNodeSelect(path: string[]) { - let dirtyScopeNames = [...this.state.dirtyScopeNames]; + let treeScopes = [...this.state.treeScopes]; let siblings = this.state.nodes; @@ -134,22 +137,27 @@ export class ScopesFiltersScene extends SceneObjectBase siblings = siblings[path[idx]].nodes; } - const name = path[path.length - 1]; - const { linkId } = siblings[name]; + const nodeName = path[path.length - 1]; + const { linkId } = siblings[nodeName]; - const selectedIdx = dirtyScopeNames.findIndex((scopeName) => scopeName === linkId); + const selectedIdx = treeScopes.findIndex(({ scopeName }) => scopeName === linkId); if (selectedIdx === -1) { fetchScope(linkId!); const selectedFromSameNode = - dirtyScopeNames.length === 0 || Object.values(siblings).some(({ linkId }) => linkId === dirtyScopeNames[0]); + treeScopes.length === 0 || Object.values(siblings).some(({ linkId }) => linkId === treeScopes[0].scopeName); - this.setState({ dirtyScopeNames: !selectedFromSameNode ? [linkId!] : [...dirtyScopeNames, linkId!] }); + const treeScope = { + scopeName: linkId!, + path, + }; + + this.setState({ treeScopes: !selectedFromSameNode ? [treeScope] : [...treeScopes, treeScope] }); } else { - dirtyScopeNames.splice(selectedIdx, 1); + treeScopes.splice(selectedIdx, 1); - this.setState({ dirtyScopeNames }); + this.setState({ treeScopes }); } } @@ -164,62 +172,53 @@ export class ScopesFiltersScene extends SceneObjectBase } public getSelectedScopes(): Scope[] { - return this.state.scopes; + return this.state.scopes.map(({ scope }) => scope); } - public async updateScopes(dirtyScopeNames = this.state.dirtyScopeNames) { - if (isEqual(dirtyScopeNames, this.getScopeNames())) { + public async updateScopes(treeScopes = this.state.treeScopes) { + if (isEqual(treeScopes, this.getTreeScopes())) { return; } - this.setState({ dirtyScopeNames, isLoadingScopes: true }); + this.setState({ treeScopes, isLoadingScopes: true }); - this.setState({ scopes: await fetchScopes(dirtyScopeNames), isLoadingScopes: false }); + this.setState({ scopes: await fetchSelectedScopes(treeScopes), isLoadingScopes: false }); } public resetDirtyScopeNames() { - this.setState({ dirtyScopeNames: this.getScopeNames() }); + this.setState({ treeScopes: this.getTreeScopes() }); } public removeAllScopes() { - this.setState({ scopes: [], dirtyScopeNames: [], isLoadingScopes: false }); + this.setState({ scopes: [], treeScopes: [], isLoadingScopes: false }); } public enterViewMode() { this.setState({ isOpened: false }); } - private getScopeNames(): string[] { - return this.state.scopes.map(({ metadata: { name } }) => name); + private getTreeScopes(): TreeScope[] { + return this.state.scopes.map(({ scope, path }) => ({ + scopeName: scope.metadata.name, + path, + })); } } export function ScopesFiltersSceneRenderer({ model }: SceneComponentProps) { const styles = useStyles2(getStyles); - const { nodes, loadingNodeName, dirtyScopeNames, isLoadingScopes, isOpened, scopes } = model.useState(); + const { nodes, loadingNodeName, treeScopes, isLoadingScopes, isOpened, scopes } = model.useState(); const { isViewing } = model.scopesParent.useState(); - const scopesTitles = scopes.map(({ spec: { title } }) => title).join(', '); - return ( <> - 0 && !isViewing ? ( - model.removeAllScopes()} - /> - ) : undefined - } - onClick={() => model.open()} + model.open()} + onRemoveAllClick={() => model.removeAllScopes()} /> {isOpened && ( @@ -238,7 +237,7 @@ export function ScopesFiltersSceneRenderer({ model }: SceneComponentProps model.updateNode(path, isExpanded, query)} onNodeSelectToggle={(path) => model.toggleNodeSelect(path)} /> diff --git a/public/app/features/dashboard-scene/scene/Scopes/ScopesInput.tsx b/public/app/features/dashboard-scene/scene/Scopes/ScopesInput.tsx new file mode 100644 index 00000000000..b44d0ee4d11 --- /dev/null +++ b/public/app/features/dashboard-scene/scene/Scopes/ScopesInput.tsx @@ -0,0 +1,119 @@ +import { css } from '@emotion/css'; +import { groupBy } from 'lodash'; +import React, { useMemo } from 'react'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { IconButton, Input, Tooltip } from '@grafana/ui'; +import { useStyles2 } from '@grafana/ui/'; +import { t } from 'app/core/internationalization'; + +import { NodesMap, SelectedScope } from './types'; + +export interface ScopesInputProps { + nodes: NodesMap; + scopes: SelectedScope[]; + isDisabled: boolean; + isLoading: boolean; + onInputClick: () => void; + onRemoveAllClick: () => void; +} + +export function ScopesInput({ + nodes, + scopes, + isDisabled, + isLoading, + onInputClick, + onRemoveAllClick, +}: ScopesInputProps) { + const styles = useStyles2(getStyles); + + const scopesPaths = useMemo(() => { + const pathsTitles = scopes.map(({ scope, path }) => { + let currentLevel = nodes; + + let titles: string[]; + + if (path.length > 0) { + titles = path.map((nodeName) => { + const { title, nodes } = currentLevel[nodeName]; + + currentLevel = nodes; + + return title; + }); + + if (titles[0] === '') { + titles.splice(0, 1); + } + } else { + titles = [scope.spec.title]; + } + + const scopeName = titles.pop(); + + return [titles.join(' > '), scopeName]; + }); + + const groupedByPath = groupBy(pathsTitles, ([path]) => path); + + return Object.entries(groupedByPath) + .map(([path, pathScopes]) => { + const scopesTitles = pathScopes.map(([, scopeTitle]) => scopeTitle).join(', '); + + return (path ? [path, scopesTitles] : [scopesTitles]).join(' > '); + }) + .map((path) => ( +

+ {path} +

+ )); + }, [nodes, scopes, styles]); + + const scopesTitles = useMemo(() => scopes.map(({ scope }) => scope.spec.title).join(', '), [scopes]); + + const input = ( + 0 && !isDisabled ? ( + onRemoveAllClick()} + /> + ) : undefined + } + onClick={() => { + if (!isDisabled) { + onInputClick(); + } + }} + /> + ); + + if (scopes.length === 0) { + return input; + } + + return ( + {scopesPaths}} interactive={true}> + {input} + + ); +} + +const getStyles = (theme: GrafanaTheme2) => { + return { + scopePath: css({ + color: theme.colors.text.primary, + fontSize: theme.typography.pxToRem(14), + margin: theme.spacing(1, 0), + }), + }; +}; diff --git a/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.test.tsx b/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.test.tsx index e52e3fa0b24..1d38c5f51c5 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.test.tsx +++ b/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.test.tsx @@ -12,7 +12,7 @@ import { fetchDashboardsSpy, fetchNodesSpy, fetchScopeSpy, - fetchScopesSpy, + fetchSelectedScopesSpy, getApplicationsClustersExpand, getApplicationsClustersSelect, getApplicationsExpand, @@ -104,7 +104,7 @@ describe('ScopesScene', () => { fetchNodesSpy.mockClear(); fetchScopeSpy.mockClear(); - fetchScopesSpy.mockClear(); + fetchSelectedScopesSpy.mockClear(); fetchDashboardsSpy.mockClear(); dashboardScene = buildTestScene(); @@ -134,7 +134,12 @@ describe('ScopesScene', () => { }); it('Selects the proper scopes', async () => { - await act(async () => filtersScene.updateScopes(['slothPictureFactory', 'slothVoteTracker'])); + await act(async () => + filtersScene.updateScopes([ + { scopeName: 'slothPictureFactory', path: [] }, + { scopeName: 'slothVoteTracker', path: [] }, + ]) + ); await userEvents.click(getFiltersInput()); await userEvents.click(getApplicationsExpand()); expect(getApplicationsSlothVoteTrackerSelect()).toBeChecked(); @@ -203,7 +208,7 @@ describe('ScopesScene', () => { await userEvents.click(getFiltersInput()); await userEvents.click(getClustersSelect()); await userEvents.click(getFiltersApply()); - await waitFor(() => expect(fetchScopesSpy).toHaveBeenCalled()); + await waitFor(() => expect(fetchSelectedScopesSpy).toHaveBeenCalled()); expect(filtersScene.getSelectedScopes()).toEqual( mocksScopes.filter(({ metadata: { name } }) => name === 'indexHelperCluster') ); @@ -213,7 +218,7 @@ describe('ScopesScene', () => { await userEvents.click(getFiltersInput()); await userEvents.click(getClustersSelect()); await userEvents.click(getFiltersCancel()); - await waitFor(() => expect(fetchScopesSpy).not.toHaveBeenCalled()); + await waitFor(() => expect(fetchSelectedScopesSpy).not.toHaveBeenCalled()); expect(filtersScene.getSelectedScopes()).toEqual([]); }); diff --git a/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.tsx b/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.tsx index 62e59eb4a3a..a7f991913c5 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.tsx +++ b/public/app/features/dashboard-scene/scene/Scopes/ScopesScene.tsx @@ -32,7 +32,7 @@ export class ScopesScene extends SceneObjectBase { this.state.filters.subscribeToState((newState, prevState) => { if (newState.scopes !== prevState.scopes) { if (this.state.isExpanded) { - this.state.dashboards.fetchDashboards(newState.scopes); + this.state.dashboards.fetchDashboards(this.state.filters.getSelectedScopes()); } sceneGraph.getTimeRange(this.parent!).onRefresh(); diff --git a/public/app/features/dashboard-scene/scene/Scopes/ScopesTreeLevel.tsx b/public/app/features/dashboard-scene/scene/Scopes/ScopesTreeLevel.tsx index 06566d270bc..787e089448c 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/ScopesTreeLevel.tsx +++ b/public/app/features/dashboard-scene/scene/Scopes/ScopesTreeLevel.tsx @@ -5,15 +5,15 @@ import Skeleton from 'react-loading-skeleton'; import { GrafanaTheme2 } from '@grafana/data'; import { Checkbox, Icon, IconButton, Input, useStyles2 } from '@grafana/ui'; -import { t } from 'app/core/internationalization'; +import { t, Trans } from 'app/core/internationalization'; -import { NodesMap } from './types'; +import { NodesMap, TreeScope } from './types'; export interface ScopesTreeLevelProps { nodes: NodesMap; nodePath: string[]; loadingNodeName: string | undefined; - scopeNames: string[]; + scopes: TreeScope[]; onNodeUpdate: (path: string[], isExpanded: boolean, query: string) => void; onNodeSelectToggle: (path: string[]) => void; } @@ -22,7 +22,7 @@ export function ScopesTreeLevel({ nodes, nodePath, loadingNodeName, - scopeNames, + scopes, onNodeUpdate, onNodeSelectToggle, }: ScopesTreeLevelProps) { @@ -34,6 +34,7 @@ export function ScopesTreeLevel({ const childNodesArr = Object.values(childNodes); const isNodeLoading = loadingNodeName === nodeId; + const scopeNames = scopes.map(({ scopeName }) => scopeName); const anyChildExpanded = childNodesArr.some(({ isExpanded }) => isExpanded); const anyChildSelected = childNodesArr.some(({ linkId }) => linkId && scopeNames.includes(linkId!)); @@ -45,13 +46,19 @@ export function ScopesTreeLevel({ } className={styles.searchInput} - placeholder={t('scopes.tree.search', 'Filter')} + placeholder={t('scopes.tree.search', 'Search')} defaultValue={node.query} data-testid={`scopes-tree-${nodeId}-search`} onInput={(evt) => onQueryUpdate(nodePath, true, evt.currentTarget.value)} /> )} + {!anyChildExpanded && !node.query && ( +
+ Recommended +
+ )} +
{isNodeLoading && } @@ -102,7 +109,7 @@ export function ScopesTreeLevel({ nodes={node.nodes} nodePath={childNodePath} loadingNodeName={loadingNodeName} - scopeNames={scopeNames} + scopes={scopes} onNodeUpdate={onNodeUpdate} onNodeSelectToggle={onNodeSelectToggle} /> @@ -121,6 +128,10 @@ const getStyles = (theme: GrafanaTheme2) => { searchInput: css({ margin: theme.spacing(1, 0), }), + headline: css({ + color: theme.colors.text.secondary, + margin: theme.spacing(1, 0), + }), loader: css({ margin: theme.spacing(0.5, 0), }), diff --git a/public/app/features/dashboard-scene/scene/Scopes/api.ts b/public/app/features/dashboard-scene/scene/Scopes/api.ts index 43e1e7931ed..d35b19d277f 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/api.ts +++ b/public/app/features/dashboard-scene/scene/Scopes/api.ts @@ -1,7 +1,8 @@ import { Scope, ScopeSpec, ScopeNode, ScopeDashboardBinding } from '@grafana/data'; import { config, getBackendSrv } from '@grafana/runtime'; import { ScopedResourceClient } from 'app/features/apiserver/client'; -import { NodesMap } from 'app/features/dashboard-scene/scene/Scopes/types'; + +import { NodesMap, SelectedScope, TreeScope } from './types'; const group = 'scope.grafana.app'; const version = 'v0alpha1'; @@ -90,6 +91,19 @@ export async function fetchScopes(names: string[]): Promise { return await Promise.all(names.map(fetchScope)); } +export async function fetchSelectedScopes(treeScopes: TreeScope[]): Promise { + const scopes = await fetchScopes(treeScopes.map(({ scopeName }) => scopeName)); + + return scopes.reduce((acc, scope, idx) => { + acc.push({ + scope, + path: treeScopes[idx].path, + }); + + return acc; + }, []); +} + export async function fetchDashboards(scopes: Scope[]): Promise { try { const response = await getBackendSrv().get<{ items: ScopeDashboardBinding[] }>(dashboardsEndpoint, { diff --git a/public/app/features/dashboard-scene/scene/Scopes/testUtils.tsx b/public/app/features/dashboard-scene/scene/Scopes/testUtils.tsx index f204bc9560f..faf5838bde6 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/testUtils.tsx +++ b/public/app/features/dashboard-scene/scene/Scopes/testUtils.tsx @@ -225,7 +225,7 @@ export const mocksNodes: Array = [ export const fetchNodesSpy = jest.spyOn(api, 'fetchNodes'); export const fetchScopeSpy = jest.spyOn(api, 'fetchScope'); -export const fetchScopesSpy = jest.spyOn(api, 'fetchScopes'); +export const fetchSelectedScopesSpy = jest.spyOn(api, 'fetchSelectedScopes'); export const fetchDashboardsSpy = jest.spyOn(api, 'fetchDashboards'); const selectors = { diff --git a/public/app/features/dashboard-scene/scene/Scopes/types.ts b/public/app/features/dashboard-scene/scene/Scopes/types.ts index ed68e0c8ff6..1a556870ecf 100644 --- a/public/app/features/dashboard-scene/scene/Scopes/types.ts +++ b/public/app/features/dashboard-scene/scene/Scopes/types.ts @@ -1,4 +1,4 @@ -import { ScopeNodeSpec } from '@grafana/data'; +import { Scope, ScopeNodeSpec } from '@grafana/data'; export interface Node extends ScopeNodeSpec { name: string; @@ -10,3 +10,13 @@ export interface Node extends ScopeNodeSpec { } export type NodesMap = Record; + +export interface SelectedScope { + scope: Scope; + path: string[]; +} + +export interface TreeScope { + scopeName: string; + path: string[]; +} diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 16bfdda9b48..c83965e3c5a 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -1606,7 +1606,7 @@ }, "suggestedDashboards": { "loading": "Loading dashboards", - "search": "Filter", + "search": "Search", "toggle": { "collapse": "Collapse scope filters", "expand": "Expand scope filters" @@ -1615,7 +1615,8 @@ "tree": { "collapse": "Collapse", "expand": "Expand", - "search": "Filter" + "headline": "Recommended", + "search": "Search" } }, "search": { diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index 474ee4c3b22..f99ab49211d 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -1606,7 +1606,7 @@ }, "suggestedDashboards": { "loading": "Ŀőäđįʼnģ đäşĥþőäřđş", - "search": "Fįľŧęř", + "search": "Ŝęäřčĥ", "toggle": { "collapse": "Cőľľäpşę şčőpę ƒįľŧęřş", "expand": "Ēχpäʼnđ şčőpę ƒįľŧęřş" @@ -1615,7 +1615,8 @@ "tree": { "collapse": "Cőľľäpşę", "expand": "Ēχpäʼnđ", - "search": "Fįľŧęř" + "headline": "Ŗęčőmmęʼnđęđ", + "search": "Ŝęäřčĥ" } }, "search": { From 8a891dcdc4b68b00110e7877685f710b2bef417e Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Mon, 17 Jun 2024 11:15:37 +0100 Subject: [PATCH 10/47] RestoreDashboards: Clear cached parent folders after restoring a dashboard (#89162) * RestoreDashboards: Refresh parent folders after restoring a dashboard * actually, just clear the cache * less log * refactor --- .../api/browseDashboardsAPI.ts | 5 --- .../components/RecentlyDeletedActions.tsx | 32 ++++++++++++++++--- .../browse-dashboards/state/reducers.ts | 15 +++++++++ .../features/browse-dashboards/state/slice.ts | 3 +- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts b/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts index 9be53c2b10b..49f91d2bcef 100644 --- a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts +++ b/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts @@ -381,11 +381,6 @@ export const browseDashboardsAPI = createApi({ url: `/dashboards/uid/${dashboardUID}/trash`, method: 'PATCH', }), - onQueryStarted: ({ dashboardUID }, { queryFulfilled, dispatch }) => { - queryFulfilled.then(() => { - dispatch(refreshParents([dashboardUID])); - }); - }, }), }), }); diff --git a/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx b/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx index 8657244d90c..c1ceaa07289 100644 --- a/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx +++ b/public/app/features/browse-dashboards/components/RecentlyDeletedActions.tsx @@ -3,16 +3,16 @@ import React, { useMemo } from 'react'; import { GrafanaTheme2 } from '@grafana/data/'; import { Button, useStyles2 } from '@grafana/ui'; +import { GENERAL_FOLDER_UID } from 'app/features/search/constants'; import appEvents from '../../../core/app_events'; import { Trans } from '../../../core/internationalization'; import { useDispatch } from '../../../types'; import { ShowModalReactEvent } from '../../../types/events'; -import { useRestoreDashboardMutation } from '../api/browseDashboardsAPI'; +import { useRestoreDashboardMutation } from '../../browse-dashboards/api/browseDashboardsAPI'; +import { clearFolders, setAllSelection, useActionSelectionState } from '../../browse-dashboards/state'; import { useRecentlyDeletedStateManager } from '../api/useRecentlyDeletedStateManager'; -import { setAllSelection, useActionSelectionState } from '../state'; - -import { RestoreModal } from './RestoreModal'; +import { RestoreModal } from '../components/RestoreModal'; export function RecentlyDeletedActions() { const styles = useStyles2(getStyles); @@ -36,9 +36,31 @@ export function RecentlyDeletedActions() { }; const onRestore = async () => { - const promises = selectedDashboards.map((uid) => restoreDashboard({ dashboardUID: uid })); + const resultsView = stateManager.state.result?.view.toArray(); + if (!resultsView) { + return; + } + + const promises = selectedDashboards.map((uid) => { + return restoreDashboard({ dashboardUID: uid }); + }); await Promise.all(promises); + + const parentUIDs = new Set(); + for (const uid of selectedDashboards) { + const foundItem = resultsView.find((v) => v.uid === uid); + if (!foundItem) { + continue; + } + + // Search API returns items with no parent with a location of 'general', so we + // need to convert that back to undefined + const folderUID = foundItem.location === GENERAL_FOLDER_UID ? undefined : foundItem.location; + parentUIDs.add(folderUID); + } + dispatch(clearFolders(Array.from(parentUIDs))); + onActionComplete(); }; diff --git a/public/app/features/browse-dashboards/state/reducers.ts b/public/app/features/browse-dashboards/state/reducers.ts index e506748dcd6..045be88a490 100644 --- a/public/app/features/browse-dashboards/state/reducers.ts +++ b/public/app/features/browse-dashboards/state/reducers.ts @@ -200,3 +200,18 @@ export function setAllSelection( } } } + +export function clearFolders(state: BrowseDashboardsState, action: PayloadAction>) { + const folderUIDs = Array.isArray(action.payload) ? action.payload : [action.payload]; + + for (const folderUID of folderUIDs) { + if (!folderUID) { + state.rootItems = undefined; + } else { + state.childrenByParentUID[folderUID] = undefined; + + // close the folder to require it to be refetched next time its opened + state.openFolders[folderUID] = false; + } + } +} diff --git a/public/app/features/browse-dashboards/state/slice.ts b/public/app/features/browse-dashboards/state/slice.ts index 55254eb78d1..8c2172e442f 100644 --- a/public/app/features/browse-dashboards/state/slice.ts +++ b/public/app/features/browse-dashboards/state/slice.ts @@ -32,7 +32,8 @@ const browseDashboardsSlice = createSlice({ export const browseDashboardsReducer = browseDashboardsSlice.reducer; -export const { setFolderOpenState, setItemSelectionState, setAllSelection } = browseDashboardsSlice.actions; +export const { setFolderOpenState, setItemSelectionState, setAllSelection, clearFolders } = + browseDashboardsSlice.actions; export default { browseDashboards: browseDashboardsReducer, From a7726ff81315439f63c2ddf0ee23d733c0724e0f Mon Sep 17 00:00:00 2001 From: Darren Janeczek <38694490+darrenjaneczek@users.noreply.github.com> Date: Mon, 17 Jun 2024 06:22:11 -0400 Subject: [PATCH 11/47] fix: "Feature toggles" sentence casing on menu item (#89249) fix: sentence casing on menu item --- pkg/services/navtree/navtreeimpl/admin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/navtree/navtreeimpl/admin.go b/pkg/services/navtree/navtreeimpl/admin.go index a81542fbb18..80b45148c11 100644 --- a/pkg/services/navtree/navtreeimpl/admin.go +++ b/pkg/services/navtree/navtreeimpl/admin.go @@ -43,7 +43,7 @@ func (s *ServiceImpl) getAdminNode(c *contextmodel.ReqContext) (*navtree.NavLink } if s.features.IsEnabled(ctx, featuremgmt.FlagFeatureToggleAdminPage) && hasAccess(ac.EvalPermission(ac.ActionFeatureManagementRead)) { generalNodeLinks = append(generalNodeLinks, &navtree.NavLink{ - Text: "Feature Toggles", + Text: "Feature toggles", SubTitle: "View and edit feature toggles", Id: "feature-toggles", Url: s.cfg.AppSubURL + "/admin/featuretoggles", From 43a246f431fc7ce94ea66f3151ca55801dfcd2fe Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Mon, 17 Jun 2024 12:25:47 +0200 Subject: [PATCH 12/47] Alerting: Improve performance of /api/prometheus for large numbers of alerts. (#89268) * Alerting: Optimize sorting alert instances. * Also change other Labels fields for consistency --- pkg/services/ngalert/api/api_prometheus.go | 12 ++--- .../ngalert/api/tooling/definitions/prom.go | 53 ++++++------------- .../tooling/definitions/prom_bench_test.go | 5 +- .../api/tooling/definitions/prom_test.go | 36 ++++++++----- 4 files changed, 49 insertions(+), 57 deletions(-) diff --git a/pkg/services/ngalert/api/api_prometheus.go b/pkg/services/ngalert/api/api_prometheus.go index ccea63ffbb8..d5c6343f780 100644 --- a/pkg/services/ngalert/api/api_prometheus.go +++ b/pkg/services/ngalert/api/api_prometheus.go @@ -97,8 +97,8 @@ func PrepareAlertStatuses(manager state.AlertInstanceManager, opts AlertStatuses } alertResponse.Data.Alerts = append(alertResponse.Data.Alerts, &apimodels.Alert{ - Labels: alertState.GetLabels(labelOptions...), - Annotations: alertState.Annotations, + Labels: apimodels.LabelsFromMap(alertState.GetLabels(labelOptions...)), + Annotations: apimodels.LabelsFromMap(alertState.Annotations), // TODO: or should we make this two fields? Using one field lets the // frontend use the same logic for parsing text on annotations and this. @@ -444,12 +444,12 @@ func toRuleGroup(log log.Logger, manager state.AlertInstanceManager, groupKey ng Name: rule.Title, Query: ruleToQuery(log, rule), Duration: rule.For.Seconds(), - Annotations: rule.Annotations, + Annotations: apimodels.LabelsFromMap(rule.Annotations), } newRule := apimodels.Rule{ Name: rule.Title, - Labels: rule.GetLabels(labelOptions...), + Labels: apimodels.LabelsFromMap(rule.GetLabels(labelOptions...)), Health: "ok", Type: rule.Type().String(), LastEvaluation: time.Time{}, @@ -471,8 +471,8 @@ func toRuleGroup(log log.Logger, manager state.AlertInstanceManager, groupKey ng totals["error"] += 1 } alert := apimodels.Alert{ - Labels: alertState.GetLabels(labelOptions...), - Annotations: alertState.Annotations, + Labels: apimodels.LabelsFromMap(alertState.GetLabels(labelOptions...)), + Annotations: apimodels.LabelsFromMap(alertState.Annotations), // TODO: or should we make this two fields? Using one field lets the // frontend use the same logic for parsing text on annotations and this. diff --git a/pkg/services/ngalert/api/tooling/definitions/prom.go b/pkg/services/ngalert/api/tooling/definitions/prom.go index cb37533377c..f2ad8c4c110 100644 --- a/pkg/services/ngalert/api/tooling/definitions/prom.go +++ b/pkg/services/ngalert/api/tooling/definitions/prom.go @@ -9,6 +9,7 @@ import ( "time" v1 "github.com/prometheus/client_golang/api/prometheus/v1" + promlabels "github.com/prometheus/prometheus/model/labels" ) // swagger:route GET /prometheus/grafana/api/v1/rules prometheus RouteGetGrafanaRuleStatuses @@ -151,7 +152,7 @@ type AlertingRule struct { Query string `json:"query,omitempty"` Duration float64 `json:"duration,omitempty"` // required: true - Annotations overrideLabels `json:"annotations,omitempty"` + Annotations promlabels.Labels `json:"annotations,omitempty"` // required: true ActiveAt *time.Time `json:"activeAt,omitempty"` Alerts []Alert `json:"alerts,omitempty"` @@ -166,8 +167,8 @@ type Rule struct { // required: true Name string `json:"name"` // required: true - Query string `json:"query"` - Labels overrideLabels `json:"labels,omitempty"` + Query string `json:"query"` + Labels promlabels.Labels `json:"labels,omitempty"` // required: true Health string `json:"health"` LastError string `json:"lastError,omitempty"` @@ -181,9 +182,9 @@ type Rule struct { // swagger:model type Alert struct { // required: true - Labels overrideLabels `json:"labels"` + Labels promlabels.Labels `json:"labels"` // required: true - Annotations overrideLabels `json:"annotations"` + Annotations promlabels.Labels `json:"annotations"` // required: true State string `json:"state"` ActiveAt *time.Time `json:"activeAt"` @@ -300,31 +301,6 @@ func (by AlertsBy) TopK(alerts []Alert, k int) []Alert { // is more important than "normal". If two alerts have the same importance // then the ordering is based on their ActiveAt time and their labels. func AlertsByImportance(a1, a2 *Alert) bool { - // labelsForComparison concatenates each key/value pair into a string and - // sorts them. - labelsForComparison := func(m map[string]string) []string { - s := make([]string, 0, len(m)) - for k, v := range m { - s = append(s, k+v) - } - sort.Strings(s) - return s - } - - // compareLabels returns true if labels1 are less than labels2. This happens - // when labels1 has fewer labels than labels2, or if the next label from - // labels1 is lexicographically less than the next label from labels2. - compareLabels := func(labels1, labels2 []string) bool { - if len(labels1) == len(labels2) { - for i := range labels1 { - if labels1[i] != labels2[i] { - return labels1[i] < labels2[i] - } - } - } - return len(labels1) < len(labels2) - } - // The importance of an alert is first based on the importance of their states. // This ordering is intended to show the most important alerts first when // using pagination. @@ -345,9 +321,7 @@ func AlertsByImportance(a1, a2 *Alert) bool { return true } // Both alerts are active since the same time so compare their labels - labels1 := labelsForComparison(a1.Labels) - labels2 := labelsForComparison(a2.Labels) - return compareLabels(labels1, labels2) + return promlabels.Compare(a1.Labels, a2.Labels) < 0 } return importance1 < importance2 @@ -362,9 +336,16 @@ func (s AlertsSorter) Len() int { return len(s.alerts) } func (s AlertsSorter) Swap(i, j int) { s.alerts[i], s.alerts[j] = s.alerts[j], s.alerts[i] } func (s AlertsSorter) Less(i, j int) bool { return s.by(&s.alerts[i], &s.alerts[j]) } -// override the labels type with a map for generation. -// The custom marshaling for labels.Labels ends up doing this anyways. -type overrideLabels map[string]string +// LabelsFromMap creates Labels from a map. Note the Labels type requires the +// labels be sorted, so we make sure to do that. +func LabelsFromMap(m map[string]string) promlabels.Labels { + sb := promlabels.NewScratchBuilder(len(m)) + for k, v := range m { + sb.Add(k, v) + } + sb.Sort() + return sb.Labels() +} // swagger:parameters RouteGetGrafanaAlertStatuses type GetGrafanaAlertStatusesParams struct { diff --git a/pkg/services/ngalert/api/tooling/definitions/prom_bench_test.go b/pkg/services/ngalert/api/tooling/definitions/prom_bench_test.go index 64a051751cc..65d6fa39754 100644 --- a/pkg/services/ngalert/api/tooling/definitions/prom_bench_test.go +++ b/pkg/services/ngalert/api/tooling/definitions/prom_bench_test.go @@ -21,10 +21,11 @@ func makeAlerts(amount int) []Alert { alerts := make([]Alert, amount) for i := 0; i < len(alerts); i++ { - alerts[i].Labels = make(map[string]string) + lbls := make(map[string]string) for label := 0; label < numLabels; label++ { - alerts[i].Labels[fmt.Sprintf("label_%d", label)] = fmt.Sprintf("label_%d_value_%d", label, i%100) + lbls[fmt.Sprintf("label_%d", label)] = fmt.Sprintf("label_%d_value_%d", label, i%100) } + alerts[i].Labels = LabelsFromMap(lbls) if i%100 < percentAlerting { alerts[i].State = "alerting" diff --git a/pkg/services/ngalert/api/tooling/definitions/prom_test.go b/pkg/services/ngalert/api/tooling/definitions/prom_test.go index a2013541e2b..40c6dc66107 100644 --- a/pkg/services/ngalert/api/tooling/definitions/prom_test.go +++ b/pkg/services/ngalert/api/tooling/definitions/prom_test.go @@ -69,32 +69,42 @@ func TestSortAlertsByImportance(t *testing.T) { }, { name: "inactive alerts with same importance are ordered by labels", input: []Alert{ - {State: "normal", Labels: map[string]string{"c": "d"}}, - {State: "normal", Labels: map[string]string{"a": "b"}}, + {State: "normal", Labels: LabelsFromMap(map[string]string{"c": "d"})}, + {State: "normal", Labels: LabelsFromMap(map[string]string{"a": "b"})}, }, expected: []Alert{ - {State: "normal", Labels: map[string]string{"a": "b"}}, - {State: "normal", Labels: map[string]string{"c": "d"}}, + {State: "normal", Labels: LabelsFromMap(map[string]string{"a": "b"})}, + {State: "normal", Labels: LabelsFromMap(map[string]string{"c": "d"})}, }, }, { - name: "active alerts with same importance and active time are ordered fewest labels first", + name: "active alerts with same importance and active time are ordered by label names", input: []Alert{ - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"a": "b", "c": "d"}}, - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"e": "f"}}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"c": "d", "e": "f"})}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"a": "b"})}, }, expected: []Alert{ - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"e": "f"}}, - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"a": "b", "c": "d"}}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"a": "b"})}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"c": "d", "e": "f"})}, }, }, { name: "active alerts with same importance and active time are ordered by labels", input: []Alert{ - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"c": "d"}}, - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"a": "b"}}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"c": "d"})}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"a": "b"})}, }, expected: []Alert{ - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"a": "b"}}, - {State: "alerting", ActiveAt: &tm1, Labels: map[string]string{"c": "d"}}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"a": "b"})}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"c": "d"})}, + }, + }, { + name: "active alerts with same importance and active time are ordered by label values", + input: []Alert{ + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"x": "b"})}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"x": "a"})}, + }, + expected: []Alert{ + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"x": "a"})}, + {State: "alerting", ActiveAt: &tm1, Labels: LabelsFromMap(map[string]string{"x": "b"})}, }, }} From 212c1477c26bc2533529b417e4c7b30625a8ddeb Mon Sep 17 00:00:00 2001 From: Sergej-Vlasov <37613182+Sergej-Vlasov@users.noreply.github.com> Date: Mon, 17 Jun 2024 12:31:52 +0200 Subject: [PATCH 13/47] DashboardScene: Adjust a11y tests errors (#89275) adjust a11y tests errors --- .../core/components/AccessControl/PermissionListItem.tsx | 2 +- .../features/dashboard-scene/scene/NavToolbarActions.tsx | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/public/app/core/components/AccessControl/PermissionListItem.tsx b/public/app/core/components/AccessControl/PermissionListItem.tsx index 0d6eaf15cf7..1fdf97b7a55 100644 --- a/public/app/core/components/AccessControl/PermissionListItem.tsx +++ b/public/app/core/components/AccessControl/PermissionListItem.tsx @@ -60,7 +60,7 @@ export const PermissionListItem = ({ item, permissionLevels, canSet, onRemove, o /> ) : ( - - )}
diff --git a/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistory.tsx b/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistory.tsx new file mode 100644 index 00000000000..e01bb4f6a02 --- /dev/null +++ b/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistory.tsx @@ -0,0 +1,383 @@ +import { css } from '@emotion/css'; +import React, { useCallback, useState } from 'react'; +import { useForm } from 'react-hook-form'; +import { useMeasure } from 'react-use'; + +import { GrafanaTheme2, TimeRange } from '@grafana/data'; +import { isFetchError } from '@grafana/runtime'; +import { SceneComponentProps, SceneObjectBase, sceneGraph } from '@grafana/scenes'; +import { + Alert, + Button, + Field, + Icon, + Input, + Label, + LoadingBar, + Stack, + Text, + Tooltip, + useStyles2, + withErrorBoundary, +} from '@grafana/ui'; +import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound'; +import { Trans, t } from 'app/core/internationalization'; +import { + GrafanaAlertStateWithReason, + isAlertStateWithReason, + isGrafanaAlertState, + mapStateWithReasonToBaseState, + mapStateWithReasonToReason, +} from 'app/types/unified-alerting-dto'; + +import { stateHistoryApi } from '../../../api/stateHistoryApi'; +import { GRAFANA_RULES_SOURCE_NAME } from '../../../utils/datasource'; +import { stringifyErrorLike } from '../../../utils/misc'; +import { hashLabelsOrAnnotations } from '../../../utils/rule-id'; +import { AlertLabels } from '../../AlertLabels'; +import { CollapseToggle } from '../../CollapseToggle'; +import { LogRecord } from '../state-history/common'; +import { useRuleHistoryRecords } from '../state-history/useRuleHistoryRecords'; + +const LIMIT_EVENTS = 250; + +const HistoryEventsList = ({ timeRange }: { timeRange?: TimeRange }) => { + const styles = useStyles2(getStyles); + + // Filter state + const [eventsFilter, setEventsFilter] = useState(''); + // form for filter fields + const { register, handleSubmit, reset } = useForm({ defaultValues: { query: '' } }); // form for search field + const from = timeRange?.from.unix(); + const to = timeRange?.to.unix(); + const onFilterCleared = useCallback(() => { + setEventsFilter(''); + reset(); + }, [setEventsFilter, reset]); + + const { + data: stateHistory, + isLoading, + isError, + error, + } = stateHistoryApi.endpoints.getRuleHistory.useQuery( + { + from: from, + to: to, + limit: LIMIT_EVENTS, + }, + { + refetchOnFocus: true, + refetchOnReconnect: true, + } + ); + + const { historyRecords } = useRuleHistoryRecords(stateHistory, eventsFilter); + + if (isError) { + return ; + } + + return ( + +
+
setEventsFilter(data.query))}> + + + +
+ + + + ); +}; + +// todo: this function has been copied from RuleList.v2.tsx, should be moved to a shared location +const LoadingIndicator = ({ visible = false }) => { + const [measureRef, { width }] = useMeasure(); + return
{visible && }
; +}; + +interface HistoryLogEventsProps { + logRecords: LogRecord[]; +} +function HistoryLogEvents({ logRecords }: HistoryLogEventsProps) { + // display log records + return ( +
    + {logRecords.map((record) => { + return ; + })} +
+ ); +} + +interface HistoryErrorMessageProps { + error: unknown; +} + +function HistoryErrorMessage({ error }: HistoryErrorMessageProps) { + if (isFetchError(error) && error.status === 404) { + return ; + } + const title = t('central-alert-history.error', 'Something went wrong loading the alert state history'); + + return {stringifyErrorLike(error)}; +} + +interface SearchFieldInputProps { + showClearFilterSuffix: boolean; + onClearFilterClick: () => void; +} +const SearchFieldInput = React.forwardRef( + ({ showClearFilterSuffix, onClearFilterClick, ...rest }: SearchFieldInputProps, ref) => { + const placeholder = t('central-alert-history.filter.placeholder', 'Filter events in the list with labels'); + return ( + + + + Filter events + + + + } + > + } + suffix={ + showClearFilterSuffix && ( + + ) + } + placeholder={placeholder} + ref={ref} + {...rest} + /> + + ); + } +); + +SearchFieldInput.displayName = 'SearchFieldInput'; + +function EventRow({ record }: { record: LogRecord }) { + const styles = useStyles2(getStyles); + const [isCollapsed, setIsCollapsed] = useState(true); + return ( +
+
+ + +
+ +
+
+ +
+
+ {record.line.labels ? : null} +
+
+ +
+ +
+
+ ); +} + +function AlertRuleName({ labels, ruleUID }: { labels: Record; ruleUID?: string }) { + const styles = useStyles2(getStyles); + const alertRuleName = labels['alertname']; + if (!ruleUID) { + return {alertRuleName}; + } + return ( + + + {alertRuleName} + + + ); +} + +interface EventTransitionProps { + previous: GrafanaAlertStateWithReason; + current: GrafanaAlertStateWithReason; +} +function EventTransition({ previous, current }: EventTransitionProps) { + return ( + + + + + + ); +} + +function EventState({ state }: { state: GrafanaAlertStateWithReason }) { + const styles = useStyles2(getStyles); + + if (!isGrafanaAlertState(state) && !isAlertStateWithReason(state)) { + return ( + + + + ); + } + const baseState = mapStateWithReasonToBaseState(state); + const reason = mapStateWithReasonToReason(state); + + switch (baseState) { + case 'Normal': + return ( + + + + ); + case 'Alerting': + return ( + + + + ); + case 'NoData': //todo:change icon + return ( + + + {/* no idea which icon to use */} + + ); + case 'Error': + return ( + + + + ); + + case 'Pending': + return ( + + + + ); + default: + return ; + } +} + +interface TimestampProps { + time: number; // epoch timestamp +} + +const Timestamp = ({ time }: TimestampProps) => { + const dateTime = new Date(time); + const formattedDate = dateTime.toLocaleString('en-US', { + month: 'long', + day: 'numeric', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + hour12: false, + }); + + return ( + + {formattedDate} + + ); +}; + +export default withErrorBoundary(HistoryEventsList, { style: 'page' }); + +export const getStyles = (theme: GrafanaTheme2) => { + return { + header: css({ + display: 'flex', + flexDirection: 'row', + alignItems: 'center', + padding: `${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(1)} 0`, + flexWrap: 'nowrap', + borderBottom: `1px solid ${theme.colors.border.weak}`, + + '&:hover': { + backgroundColor: theme.components.table.rowHoverBackground, + }, + }), + + collapseToggle: css({ + background: 'none', + border: 'none', + marginTop: `-${theme.spacing(1)}`, + marginBottom: `-${theme.spacing(1)}`, + + svg: { + marginBottom: 0, + }, + }), + normalColor: css({ + fill: theme.colors.success.text, + }), + warningColor: css({ + fill: theme.colors.warning.text, + }), + alertingColor: css({ + fill: theme.colors.error.text, + }), + timeCol: css({ + width: '150px', + }), + transitionCol: css({ + width: '80px', + }), + alertNameCol: css({ + width: '300px', + }), + labelsCol: css({ + display: 'flex', + overflow: 'hidden', + alignItems: 'center', + paddingRight: theme.spacing(2), + flex: 1, + }), + alertName: css({ + whiteSpace: 'nowrap', + cursor: 'pointer', + overflow: 'hidden', + textOverflow: 'ellipsis', + display: 'block', + color: theme.colors.text.link, + }), + labelsFilter: css({ + width: '100%', + paddingTop: theme.spacing(4), + }), + }; +}; + +export class HistoryEventsListObject extends SceneObjectBase { + public static Component = HistoryEventsListObjectRenderer; +} + +export function HistoryEventsListObjectRenderer({ model }: SceneComponentProps) { + const { value: timeRange } = sceneGraph.getTimeRange(model).useState(); // get time range from scene graph + + return ; +} diff --git a/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistoryPage.tsx b/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistoryPage.tsx new file mode 100644 index 00000000000..0ab57f508d4 --- /dev/null +++ b/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistoryPage.tsx @@ -0,0 +1,16 @@ +import React from 'react'; + +import { withErrorBoundary } from '@grafana/ui'; + +import { AlertingPageWrapper } from '../../AlertingPageWrapper'; + +import { CentralAlertHistoryScene } from './CentralAlertHistoryScene'; + +const HistoryPage = () => { + return ( + + + + ); +}; +export default withErrorBoundary(HistoryPage, { style: 'page' }); diff --git a/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistoryScene.tsx b/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistoryScene.tsx new file mode 100644 index 00000000000..0cd75c05a4b --- /dev/null +++ b/public/app/features/alerting/unified/components/rules/central-state-history/CentralAlertHistoryScene.tsx @@ -0,0 +1,124 @@ +import React from 'react'; + +import { getDataSourceSrv } from '@grafana/runtime'; +import { + EmbeddedScene, + PanelBuilders, + SceneControlsSpacer, + SceneFlexItem, + SceneFlexLayout, + SceneQueryRunner, + SceneReactObject, + SceneRefreshPicker, + SceneTimePicker, +} from '@grafana/scenes'; +import { + GraphDrawStyle, + GraphGradientMode, + LegendDisplayMode, + LineInterpolation, + ScaleDistribution, + StackingMode, + TooltipDisplayMode, + VisibilityMode, +} from '@grafana/schema/dist/esm/index'; + +import { DataSourceInformation, PANEL_STYLES } from '../../../home/Insights'; +import { SectionSubheader } from '../../../insights/SectionSubheader'; + +import { HistoryEventsListObjectRenderer } from './CentralAlertHistory'; + +export const CentralAlertHistoryScene = () => { + const dataSourceSrv = getDataSourceSrv(); + const alertStateHistoryDatasource: DataSourceInformation = { + type: 'loki', + uid: 'grafanacloud-alert-state-history', + settings: undefined, + }; + + alertStateHistoryDatasource.settings = dataSourceSrv.getInstanceSettings(alertStateHistoryDatasource.uid); + + const scene = new EmbeddedScene({ + controls: [new SceneControlsSpacer(), new SceneTimePicker({}), new SceneRefreshPicker({})], + body: new SceneFlexLayout({ + direction: 'column', + children: [ + new SceneFlexItem({ + ySizing: 'content', + body: getEventsSceneObject(alertStateHistoryDatasource), + }), + new SceneFlexItem({ + body: new SceneReactObject({ + component: HistoryEventsListObjectRenderer, + }), + }), + ], + }), + }); + + return ; +}; + +function getEventsSceneObject(ashDs: DataSourceInformation) { + return new EmbeddedScene({ + controls: [ + new SceneReactObject({ + component: SectionSubheader, + }), + ], + body: new SceneFlexLayout({ + direction: 'column', + children: [ + new SceneFlexItem({ + ySizing: 'content', + body: new SceneFlexLayout({ + children: [getEventsScenesFlexItem(ashDs)], + }), + }), + ], + }), + }); +} + +function getSceneQuery(datasource: DataSourceInformation) { + const query = new SceneQueryRunner({ + datasource, + queries: [ + { + refId: 'A', + expr: 'count_over_time({from="state-history"} |= `` [$__auto])', + queryType: 'range', + step: '10s', + }, + ], + }); + return query; +} + +export function getEventsScenesFlexItem(datasource: DataSourceInformation) { + return new SceneFlexItem({ + ...PANEL_STYLES, + body: PanelBuilders.timeseries() + .setTitle('Events') + .setDescription('Alert events during the period of time.') + .setData(getSceneQuery(datasource)) + .setColor({ mode: 'continuous-BlPu' }) + .setCustomFieldConfig('fillOpacity', 100) + .setCustomFieldConfig('drawStyle', GraphDrawStyle.Bars) + .setCustomFieldConfig('lineInterpolation', LineInterpolation.Linear) + .setCustomFieldConfig('lineWidth', 1) + .setCustomFieldConfig('barAlignment', 0) + .setCustomFieldConfig('spanNulls', false) + .setCustomFieldConfig('insertNulls', false) + .setCustomFieldConfig('showPoints', VisibilityMode.Auto) + .setCustomFieldConfig('pointSize', 5) + .setCustomFieldConfig('stacking', { mode: StackingMode.None, group: 'A' }) + .setCustomFieldConfig('gradientMode', GraphGradientMode.Hue) + .setCustomFieldConfig('scaleDistribution', { type: ScaleDistribution.Linear }) + .setOption('legend', { showLegend: false, displayMode: LegendDisplayMode.Hidden }) + .setOption('tooltip', { mode: TooltipDisplayMode.Single }) + + .setNoValue('No events found') + .build(), + }); +} diff --git a/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.tsx b/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.tsx index 062ff5b1820..9c9ea58eabe 100644 --- a/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.tsx +++ b/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.tsx @@ -4,7 +4,7 @@ import React, { useCallback, useMemo, useRef, useState } from 'react'; import { useForm } from 'react-hook-form'; import { DataFrame, dateTime, GrafanaTheme2, TimeRange } from '@grafana/data'; -import { Alert, Button, Field, Icon, Input, Label, Tooltip, useStyles2, Stack } from '@grafana/ui'; +import { Alert, Button, Field, Icon, Input, Label, Stack, Tooltip, useStyles2 } from '@grafana/ui'; import { stateHistoryApi } from '../../../api/stateHistoryApi'; import { combineMatcherStrings } from '../../../utils/alertmanager'; diff --git a/public/app/features/alerting/unified/components/rules/state-history/common.ts b/public/app/features/alerting/unified/components/rules/state-history/common.ts index f797fa1f8a5..d6d924dccdf 100644 --- a/public/app/features/alerting/unified/components/rules/state-history/common.ts +++ b/public/app/features/alerting/unified/components/rules/state-history/common.ts @@ -7,6 +7,7 @@ export interface Line { current: GrafanaAlertStateWithReason; values?: Record; labels?: Record; + ruleUID?: string; } export interface LogRecord { diff --git a/public/app/features/alerting/unified/utils/rule-id.ts b/public/app/features/alerting/unified/utils/rule-id.ts index 3e6ebad8c64..b3785ffcaf5 100644 --- a/public/app/features/alerting/unified/utils/rule-id.ts +++ b/public/app/features/alerting/unified/utils/rule-id.ts @@ -240,7 +240,7 @@ export function hashRule(rule: Rule): string { throw new Error('only recording and alerting rules can be hashed'); } -function hashLabelsOrAnnotations(item: Labels | Annotations | undefined): string { +export function hashLabelsOrAnnotations(item: Labels | Annotations | undefined): string { return JSON.stringify(Object.entries(item || {}).sort((a, b) => a[0].localeCompare(b[0]))); } diff --git a/public/app/features/connections/__mocks__/store.navIndex.mock.ts b/public/app/features/connections/__mocks__/store.navIndex.mock.ts index 217f9891655..f135bf3cf83 100644 --- a/public/app/features/connections/__mocks__/store.navIndex.mock.ts +++ b/public/app/features/connections/__mocks__/store.navIndex.mock.ts @@ -149,6 +149,13 @@ export const navIndex: NavIndex = { icon: 'layer-group', url: '/alerting/groups', }, + { + id: 'history', + text: 'History', + subTitle: 'Alert state history', + icon: 'history', + url: '/alerting/history', + }, { id: 'alerting-admin', text: 'Settings', diff --git a/public/app/types/unified-alerting-dto.ts b/public/app/types/unified-alerting-dto.ts index b226871d30b..c47780b31c3 100644 --- a/public/app/types/unified-alerting-dto.ts +++ b/public/app/types/unified-alerting-dto.ts @@ -42,6 +42,11 @@ export function isAlertStateWithReason( return state !== null && state !== undefined && !propAlertingRuleStateValues.includes(state); } +export function mapStateWithReasonToReason(state: GrafanaAlertStateWithReason): string { + const match = state.match(/\((.*?)\)/); + return match ? match[1] : ''; +} + export function mapStateWithReasonToBaseState( state: GrafanaAlertStateWithReason | PromAlertingRuleState ): GrafanaAlertState | PromAlertingRuleState { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index c83965e3c5a..36fb784122a 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -25,6 +25,14 @@ "user": "User" } }, + "alert-labels": { + "button": { + "hide": "Hide common labels", + "show": { + "tooltip": "Show common labels" + } + } + }, "alert-rule-form": { "evaluation-behaviour": { "description": { @@ -84,15 +92,15 @@ }, "counts": { "alertRule_one": "{{count}} alert rule", - "alertRule_other": "{{count}} alert rules", + "alertRule_other": "{{count}} alert rule", "dashboard_one": "{{count}} dashboard", - "dashboard_other": "{{count}} dashboards", + "dashboard_other": "{{count}} dashboard", "folder_one": "{{count}} folder", - "folder_other": "{{count}} folders", + "folder_other": "{{count}} folder", "libraryPanel_one": "{{count}} library panel", - "libraryPanel_other": "{{count}} library panels", + "libraryPanel_other": "{{count}} library panel", "total_one": "{{count}} item", - "total_other": "{{count}} items" + "total_other": "{{count}} item" }, "dashboards-tree": { "collapse-folder-button": "Collapse folder {{title}}", @@ -138,6 +146,16 @@ "text": "No results found for your query" } }, + "central-alert-history": { + "error": "Something went wrong loading the alert state history", + "filter": { + "button": { + "clear": "Clear" + }, + "label": "Filter events", + "placeholder": "Filter events in the list with labels" + } + }, "clipboard-button": { "inline-toast": { "success": "Copied" @@ -758,7 +776,7 @@ }, "modal": { "body_one": "This panel is being used in {{count}} dashboard. Please choose which dashboard to view the panel in:", - "body_other": "This panel is being used in {{count}} dashboards. Please choose which dashboard to view the panel in:", + "body_other": "This panel is being used in {{count}} dashboard. Please choose which dashboard to view the panel in:", "button-cancel": "Cancel", "button-view-panel1": "View panel in {{label}}...", "button-view-panel2": "View panel in dashboard...", diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index f99ab49211d..bfe2a139e08 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -25,6 +25,14 @@ "user": "Ůşęř" } }, + "alert-labels": { + "button": { + "hide": "Ħįđę čőmmőʼn ľäþęľş", + "show": { + "tooltip": "Ŝĥőŵ čőmmőʼn ľäþęľş" + } + } + }, "alert-rule-form": { "evaluation-behaviour": { "description": { @@ -84,15 +92,15 @@ }, "counts": { "alertRule_one": "{{count}} äľęřŧ řūľę", - "alertRule_other": "{{count}} äľęřŧ řūľęş", + "alertRule_other": "{{count}} äľęřŧ řūľę", "dashboard_one": "{{count}} đäşĥþőäřđ", - "dashboard_other": "{{count}} đäşĥþőäřđş", + "dashboard_other": "{{count}} đäşĥþőäřđ", "folder_one": "{{count}} ƒőľđęř", - "folder_other": "{{count}} ƒőľđęřş", + "folder_other": "{{count}} ƒőľđęř", "libraryPanel_one": "{{count}} ľįþřäřy päʼnęľ", - "libraryPanel_other": "{{count}} ľįþřäřy päʼnęľş", + "libraryPanel_other": "{{count}} ľįþřäřy päʼnęľ", "total_one": "{{count}} įŧęm", - "total_other": "{{count}} įŧęmş" + "total_other": "{{count}} įŧęm" }, "dashboards-tree": { "collapse-folder-button": "Cőľľäpşę ƒőľđęř {{title}}", @@ -138,6 +146,16 @@ "text": "Ńő řęşūľŧş ƒőūʼnđ ƒőř yőūř qūęřy" } }, + "central-alert-history": { + "error": "Ŝőmęŧĥįʼnģ ŵęʼnŧ ŵřőʼnģ ľőäđįʼnģ ŧĥę äľęřŧ şŧäŧę ĥįşŧőřy", + "filter": { + "button": { + "clear": "Cľęäř" + }, + "label": "Fįľŧęř ęvęʼnŧş", + "placeholder": "Fįľŧęř ęvęʼnŧş įʼn ŧĥę ľįşŧ ŵįŧĥ ľäþęľş" + } + }, "clipboard-button": { "inline-toast": { "success": "Cőpįęđ" @@ -758,7 +776,7 @@ }, "modal": { "body_one": "Ŧĥįş päʼnęľ įş þęįʼnģ ūşęđ įʼn {{count}} đäşĥþőäřđ. Pľęäşę čĥőőşę ŵĥįčĥ đäşĥþőäřđ ŧő vįęŵ ŧĥę päʼnęľ įʼn:", - "body_other": "Ŧĥįş päʼnęľ įş þęįʼnģ ūşęđ įʼn {{count}} đäşĥþőäřđş. Pľęäşę čĥőőşę ŵĥįčĥ đäşĥþőäřđ ŧő vįęŵ ŧĥę päʼnęľ įʼn:", + "body_other": "Ŧĥįş päʼnęľ įş þęįʼnģ ūşęđ įʼn {{count}} đäşĥþőäřđ. Pľęäşę čĥőőşę ŵĥįčĥ đäşĥþőäřđ ŧő vįęŵ ŧĥę päʼnęľ įʼn:", "button-cancel": "Cäʼnčęľ", "button-view-panel1": "Vįęŵ päʼnęľ įʼn {{label}}...", "button-view-panel2": "Vįęŵ päʼnęľ įʼn đäşĥþőäřđ...", From 51c0644e41277dfb908b5666764c2919d9b680cb Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Mon, 17 Jun 2024 14:26:23 +0100 Subject: [PATCH 17/47] RestoreDashboards: add IsDeleted and PermanentlyDeleteDate to deleted search (#89283) RestoreDashboards: add IsDeleted and PermanentlyDeleteDate to deleted items in Search --- .../dashboards/service/dashboard_service.go | 4 ++- pkg/services/search/model/model.go | 34 ++++++++++--------- public/api-enterprise-spec.json | 10 ++++-- public/api-merged.json | 8 +++-- public/openapi3.json | 6 +++- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 7871776d224..c94d8b4df7f 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -741,7 +741,9 @@ func makeQueryResult(query *dashboards.FindPersistedDashboardsQuery, res []dashb hit.Tags = append(hit.Tags, item.Term) } if item.Deleted != nil { - hit.RemainingTrashAtAge = util.RemainingDaysUntil((*item.Deleted).Add(daysInTrash)) + deletedDate := (*item.Deleted).Add(daysInTrash) + hit.IsDeleted = true + hit.PermanentlyDeleteDate = &deletedDate } } return hitList diff --git a/pkg/services/search/model/model.go b/pkg/services/search/model/model.go index 5e3658c35a1..6b95d2a9448 100644 --- a/pkg/services/search/model/model.go +++ b/pkg/services/search/model/model.go @@ -2,6 +2,7 @@ package model import ( "strings" + "time" ) // FilterWhere limits the set of dashboard IDs to the dashboards for @@ -62,22 +63,23 @@ const ( ) type Hit struct { - ID int64 `json:"id"` - UID string `json:"uid"` - Title string `json:"title"` - URI string `json:"uri"` - URL string `json:"url"` - Slug string `json:"slug"` - Type HitType `json:"type"` - Tags []string `json:"tags"` - IsStarred bool `json:"isStarred"` - FolderID int64 `json:"folderId,omitempty"` // Deprecated: use FolderUID instead - FolderUID string `json:"folderUid,omitempty"` - FolderTitle string `json:"folderTitle,omitempty"` - FolderURL string `json:"folderUrl,omitempty"` - SortMeta int64 `json:"sortMeta"` - SortMetaName string `json:"sortMetaName,omitempty"` - RemainingTrashAtAge string `json:"remainingTrashAtAge,omitempty"` + ID int64 `json:"id"` + UID string `json:"uid"` + Title string `json:"title"` + URI string `json:"uri"` + URL string `json:"url"` + Slug string `json:"slug"` + Type HitType `json:"type"` + Tags []string `json:"tags"` + IsStarred bool `json:"isStarred"` + FolderID int64 `json:"folderId,omitempty"` // Deprecated: use FolderUID instead + FolderUID string `json:"folderUid,omitempty"` + FolderTitle string `json:"folderTitle,omitempty"` + FolderURL string `json:"folderUrl,omitempty"` + SortMeta int64 `json:"sortMeta"` + SortMetaName string `json:"sortMetaName,omitempty"` + IsDeleted bool `json:"isDeleted"` + PermanentlyDeleteDate *time.Time `json:"permanentlyDeleteDate,omitempty"` } type HitList []*Hit diff --git a/public/api-enterprise-spec.json b/public/api-enterprise-spec.json index 4dea1f5f8ba..4be1d3a41a6 100644 --- a/public/api-enterprise-spec.json +++ b/public/api-enterprise-spec.json @@ -4703,11 +4703,15 @@ "type": "integer", "format": "int64" }, + "isDeleted": { + "type": "boolean" + }, "isStarred": { "type": "boolean" }, - "remainingTrashAtAge": { - "type": "string" + "permanentlyDeleteDate": { + "type": "string", + "format": "date-time" }, "slug": { "type": "string" @@ -7049,6 +7053,7 @@ } }, "State": { + "description": "+enum", "type": "string" }, "Status": { @@ -7492,6 +7497,7 @@ } }, "Type": { + "description": "+enum", "type": "string" }, "TypeMeta": { diff --git a/public/api-merged.json b/public/api-merged.json index 81135377b88..26cf2ef2017 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -15815,11 +15815,15 @@ "type": "integer", "format": "int64" }, + "isDeleted": { + "type": "boolean" + }, "isStarred": { "type": "boolean" }, - "remainingTrashAtAge": { - "type": "string" + "permanentlyDeleteDate": { + "type": "string", + "format": "date-time" }, "slug": { "type": "string" diff --git a/public/openapi3.json b/public/openapi3.json index 592f747a06c..a00082e3deb 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -6194,10 +6194,14 @@ "format": "int64", "type": "integer" }, + "isDeleted": { + "type": "boolean" + }, "isStarred": { "type": "boolean" }, - "remainingTrashAtAge": { + "permanentlyDeleteDate": { + "format": "date-time", "type": "string" }, "slug": { From 8fddf30621a8994faf72fb1f4ea8e03874d63c91 Mon Sep 17 00:00:00 2001 From: Brendan O'Handley Date: Mon, 17 Jun 2024 09:16:52 -0500 Subject: [PATCH 18/47] InfluxDB: Flight SQL test, add function to search for free port (#89255) * add function to search for free port * Update pkg/tsdb/influxdb/fsql/fsql_test.go Co-authored-by: Dave Henderson * Update pkg/tsdb/influxdb/fsql/fsql_test.go Co-authored-by: Dave Henderson * fix test * fix go linting issue * fix go lint --------- Co-authored-by: Dave Henderson --- pkg/tsdb/influxdb/fsql/fsql_test.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/pkg/tsdb/influxdb/fsql/fsql_test.go b/pkg/tsdb/influxdb/fsql/fsql_test.go index b2eb94f5b0d..30ba7e516be 100644 --- a/pkg/tsdb/influxdb/fsql/fsql_test.go +++ b/pkg/tsdb/influxdb/fsql/fsql_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "net" "testing" "github.com/apache/arrow/go/v15/arrow/flight" @@ -21,9 +22,14 @@ type FSQLTestSuite struct { suite.Suite db *sql.DB server flight.Server + addr string } func (suite *FSQLTestSuite) SetupTest() { + addr, _ := freeport(suite.T()) + + suite.addr = addr + db, err := example.CreateDB() require.NoError(suite.T(), err) @@ -32,7 +38,7 @@ func (suite *FSQLTestSuite) SetupTest() { sqliteServer.Alloc = memory.NewCheckedAllocator(memory.DefaultAllocator) server := flight.NewServerWithMiddleware(nil) server.RegisterFlightService(flightsql.NewFlightServer(sqliteServer)) - err = server.Init("localhost:12345") + err = server.Init(suite.addr) require.NoError(suite.T(), err) go func() { err := server.Serve() @@ -59,7 +65,7 @@ func (suite *FSQLTestSuite) TestIntegration_QueryData() { &models.DatasourceInfo{ HTTPClient: nil, Token: "secret", - URL: "http://localhost:12345", + URL: "http://" + suite.addr, DbName: "influxdb", Version: "test", HTTPMode: "proxy", @@ -109,3 +115,15 @@ func mustQueryJSON(t *testing.T, refID, sql string) []byte { } return b } + +func freeport(t *testing.T) (addr string, err error) { + l, err := net.ListenTCP("tcp", &net.TCPAddr{IP: net.ParseIP("127.0.0.1")}) + if err != nil { + t.Fatal(err) + } + defer func() { + err = l.Close() + }() + a := l.Addr().(*net.TCPAddr) + return a.String(), nil +} From 8c5a92520273535629891a7310826730dbbc993b Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Mon, 17 Jun 2024 16:56:15 +0200 Subject: [PATCH 19/47] LogRows: remove app restriction from popover menu (#89276) * LogRows: remove app restriction from popover menu * chore: update tests * Prettier --- .../features/logs/components/LogRows.test.tsx | 35 +++++++++++++++---- .../app/features/logs/components/LogRows.tsx | 2 +- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/public/app/features/logs/components/LogRows.test.tsx b/public/app/features/logs/components/LogRows.test.tsx index 5bf9fcb53da..0d6dce447d9 100644 --- a/public/app/features/logs/components/LogRows.test.tsx +++ b/public/app/features/logs/components/LogRows.test.tsx @@ -3,9 +3,9 @@ import userEvent from '@testing-library/user-event'; import { range } from 'lodash'; import React from 'react'; -import { CoreApp, LogRowModel, LogsDedupStrategy, LogsSortOrder } from '@grafana/data'; +import { LogRowModel, LogsDedupStrategy, LogsSortOrder } from '@grafana/data'; -import { LogRows, PREVIEW_LIMIT } from './LogRows'; +import { LogRows, PREVIEW_LIMIT, Props } from './LogRows'; import { createLogRow } from './__mocks__/logRow'; jest.mock('@grafana/runtime', () => ({ @@ -207,7 +207,7 @@ describe('LogRows', () => { }); describe('Popover menu', () => { - function setup(app = CoreApp.Explore) { + function setup(overrides: Partial = {}) { const rows: LogRowModel[] = [createLogRow({ uid: '1' })]; return render( { displayedFields={[]} onClickFilterOutString={() => {}} onClickFilterString={() => {}} - app={app} + {...overrides} /> ); } @@ -232,6 +232,8 @@ describe('Popover menu', () => { orgGetSelection = document.getSelection; jest.spyOn(document, 'getSelection').mockReturnValue({ toString: () => 'selected log line', + removeAllRanges: () => {}, + addRange: (range: Range) => {}, } as Selection); }); afterAll(() => { @@ -248,9 +250,30 @@ describe('Popover menu', () => { expect(screen.getByText('Add as line contains filter')).toBeInTheDocument(); expect(screen.getByText('Add as line does not contain filter')).toBeInTheDocument(); }); - it('Does not appear outside Explore', async () => { - setup(CoreApp.Unknown); + it('Does not appear when the props are not defined', async () => { + setup({ + onClickFilterOutString: undefined, + onClickFilterString: undefined, + }); await userEvent.click(screen.getByText('log message 1')); expect(screen.queryByText('Copy selection')).not.toBeInTheDocument(); }); + it('Appears after selecting test', async () => { + const onClickFilterOutString = jest.fn(); + const onClickFilterString = jest.fn(); + setup({ + onClickFilterOutString, + onClickFilterString, + }); + await userEvent.click(screen.getByText('log message 1')); + expect(screen.getByText('Copy selection')).toBeInTheDocument(); + await userEvent.click(screen.getByText('Add as line contains filter')); + + await userEvent.click(screen.getByText('log message 1')); + expect(screen.getByText('Copy selection')).toBeInTheDocument(); + await userEvent.click(screen.getByText('Add as line does not contain filter')); + + expect(onClickFilterOutString).toHaveBeenCalledTimes(1); + expect(onClickFilterString).toHaveBeenCalledTimes(1); + }); }); diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx index 41100a4c634..87306b91e2a 100644 --- a/public/app/features/logs/components/LogRows.tsx +++ b/public/app/features/logs/components/LogRows.tsx @@ -105,7 +105,7 @@ class UnThemedLogRows extends PureComponent { }; popoverMenuSupported() { - if (!config.featureToggles.logRowsPopoverMenu || this.props.app !== CoreApp.Explore) { + if (!config.featureToggles.logRowsPopoverMenu) { return false; } return Boolean(this.props.onClickFilterOutString || this.props.onClickFilterString); From 7bb883e375c52537da33f8c64d0dcd70d3722f82 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Mon, 17 Jun 2024 16:19:12 +0100 Subject: [PATCH 20/47] Analytics: Fix ApplicationInsights integration (#89299) change ApplicationInsights backend to use SystemJS to load --- .betterer.results | 4 ---- .../backends/analytics/ApplicationInsightsBackend.ts | 12 ++++++------ 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/.betterer.results b/.betterer.results index fd64c63eea8..aec2e678222 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1320,10 +1320,6 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], - "public/app/core/services/echo/backends/analytics/ApplicationInsightsBackend.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"] - ], "public/app/core/services/echo/backends/analytics/RudderstackBackend.ts:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/public/app/core/services/echo/backends/analytics/ApplicationInsightsBackend.ts b/public/app/core/services/echo/backends/analytics/ApplicationInsightsBackend.ts index 433d4582aff..78db86015cd 100644 --- a/public/app/core/services/echo/backends/analytics/ApplicationInsightsBackend.ts +++ b/public/app/core/services/echo/backends/analytics/ApplicationInsightsBackend.ts @@ -7,8 +7,6 @@ import { PageviewEchoEvent, } from '@grafana/runtime'; -import { loadScript } from '../../utils'; - interface ApplicationInsights { trackPageView: () => void; trackEvent: (event: { name: string; properties?: Record }) => void; @@ -39,10 +37,12 @@ export class ApplicationInsightsBackend implements EchoBackend { - const init = new (window as any).Microsoft.ApplicationInsights.ApplicationInsights(applicationInsightsOpts); - window.applicationInsights = init.loadAppInsights(); - }); + System.import(url) + .then((m) => (m.default ? m.default : m)) + .then(({ ApplicationInsights }) => { + const init = new ApplicationInsights(applicationInsightsOpts); + window.applicationInsights = init.loadAppInsights(); + }); } addEvent = (e: PageviewEchoEvent | InteractionEchoEvent) => { From 0abe4fc709133521cae92ba044b641a6caebd150 Mon Sep 17 00:00:00 2001 From: Ivan Ortega Alba Date: Mon, 17 Jun 2024 17:58:48 +0200 Subject: [PATCH 21/47] Scenes: Be able to show/hide dashboard controls in Kiosk mode (#88920) --- .betterer.results | 3 + .../scene/DashboardControls.test.tsx | 159 ++++++++++++++++++ .../scene/DashboardControls.tsx | 60 ++++++- .../dashboard-scene/scene/DashboardScene.tsx | 4 +- .../scene/DashboardSceneRenderer.tsx | 16 +- .../scene/DashboardSceneUrlSync.test.ts | 24 +++ .../scene/DashboardSceneUrlSync.ts | 12 +- .../transformSceneToSaveModel.ts | 1 - public/app/features/playlist/PlaylistSrv.ts | 3 + public/app/features/playlist/StartModal.tsx | 60 ++++++- 10 files changed, 319 insertions(+), 23 deletions(-) create mode 100644 public/app/features/dashboard-scene/scene/DashboardControls.test.tsx diff --git a/.betterer.results b/.betterer.results index aec2e678222..d266b08b826 100644 --- a/.betterer.results +++ b/.betterer.results @@ -2871,6 +2871,9 @@ exports[`better eslint`] = { "public/app/features/dashboard-scene/saving/shared.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], + "public/app/features/dashboard-scene/scene/DashboardControls.tsx:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"] + ], "public/app/features/dashboard-scene/scene/NavToolbarActions.test.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], diff --git a/public/app/features/dashboard-scene/scene/DashboardControls.test.tsx b/public/app/features/dashboard-scene/scene/DashboardControls.test.tsx new file mode 100644 index 00000000000..52b46d49c99 --- /dev/null +++ b/public/app/features/dashboard-scene/scene/DashboardControls.test.tsx @@ -0,0 +1,159 @@ +import { render } from '@testing-library/react'; +import React from 'react'; + +import { selectors } from '@grafana/e2e-selectors'; +import { SceneDataLayerControls, SceneVariableSet, TextBoxVariable, VariableValueSelectors } from '@grafana/scenes'; + +import { DashboardControls, DashboardControlsState } from './DashboardControls'; +import { DashboardScene } from './DashboardScene'; + +describe('DashboardControls', () => { + describe('Given a standard scene', () => { + it('should initialize with default values', () => { + const scene = buildTestScene(); + expect(scene.state.variableControls).toEqual([]); + expect(scene.state.timePicker).toBeDefined(); + expect(scene.state.refreshPicker).toBeDefined(); + }); + + it('should return if time controls are hidden', () => { + const scene = buildTestScene({ hideTimeControls: false, hideVariableControls: false, hideLinksControls: false }); + expect(scene.hasControls()).toBeTruthy(); + scene.setState({ hideTimeControls: true }); + expect(scene.hasControls()).toBeTruthy(); + scene.setState({ hideVariableControls: true, hideLinksControls: true }); + expect(scene.hasControls()).toBeFalsy(); + }); + }); + + describe('Component', () => { + it('should render', () => { + const scene = buildTestScene(); + expect(() => { + render(); + }).not.toThrow(); + }); + + it('should render visible controls', async () => { + const scene = buildTestScene({ + variableControls: [new VariableValueSelectors({}), new SceneDataLayerControls()], + }); + const renderer = render(); + + expect(await renderer.findByTestId(selectors.pages.Dashboard.Controls)).toBeInTheDocument(); + expect(await renderer.findByTestId(selectors.components.DashboardLinks.container)).toBeInTheDocument(); + expect(await renderer.findByTestId(selectors.components.TimePicker.openButton)).toBeInTheDocument(); + expect(await renderer.findByTestId(selectors.components.RefreshPicker.runButtonV2)).toBeInTheDocument(); + expect(await renderer.findByTestId(selectors.pages.Dashboard.SubMenu.submenuItem)).toBeInTheDocument(); + }); + + it('should render with hidden controls', async () => { + const scene = buildTestScene({ + hideTimeControls: true, + hideVariableControls: true, + hideLinksControls: true, + variableControls: [new VariableValueSelectors({}), new SceneDataLayerControls()], + }); + const renderer = render(); + + expect(await renderer.queryByTestId(selectors.pages.Dashboard.Controls)).not.toBeInTheDocument(); + }); + }); + + describe('UrlSync', () => { + it('should return keys', () => { + const scene = buildTestScene(); + // @ts-expect-error + expect(scene._urlSync.getKeys()).toEqual(['_dash.hideTimePicker', '_dash.hideVariables', '_dash.hideLinks']); + }); + + it('should return url state', () => { + const scene = buildTestScene(); + expect(scene.getUrlState()).toEqual({ + '_dash.hideTimePicker': undefined, + '_dash.hideVariables': undefined, + '_dash.hideLinks': undefined, + }); + scene.setState({ + hideTimeControls: true, + hideVariableControls: true, + hideLinksControls: true, + }); + expect(scene.getUrlState()).toEqual({ + '_dash.hideTimePicker': 'true', + '_dash.hideVariables': 'true', + '_dash.hideLinks': 'true', + }); + }); + + it('should update from url', () => { + const scene = buildTestScene(); + scene.updateFromUrl({ + '_dash.hideTimePicker': 'true', + '_dash.hideVariables': 'true', + '_dash.hideLinks': 'true', + }); + expect(scene.state.hideTimeControls).toBeTruthy(); + expect(scene.state.hideVariableControls).toBeTruthy(); + expect(scene.state.hideLinksControls).toBeTruthy(); + scene.updateFromUrl({ + '_dash.hideTimePicker': '', + '_dash.hideVariables': '', + '_dash.hideLinks': '', + }); + expect(scene.state.hideTimeControls).toBeTruthy(); + expect(scene.state.hideVariableControls).toBeTruthy(); + expect(scene.state.hideLinksControls).toBeTruthy(); + scene.updateFromUrl({}); + expect(scene.state.hideTimeControls).toBeFalsy(); + expect(scene.state.hideVariableControls).toBeFalsy(); + expect(scene.state.hideLinksControls).toBeFalsy(); + }); + + it('should not call setState if no changes', () => { + const scene = buildTestScene(); + const setState = jest.spyOn(scene, 'setState'); + scene.updateFromUrl({}); + scene.updateFromUrl({}); + expect(setState).toHaveBeenCalledTimes(1); + }); + }); +}); + +function buildTestScene(state?: Partial): DashboardControls { + const variable = new TextBoxVariable({ + name: 'A', + label: 'A', + description: 'A', + type: 'textbox', + value: 'Text', + }); + const dashboard = new DashboardScene({ + uid: 'A', + links: [ + { + title: 'Link', + url: 'http://localhost:3000/$A', + type: 'link', + asDropdown: false, + icon: '', + includeVars: true, + keepTime: true, + tags: [], + targetBlank: false, + tooltip: 'Link', + }, + ], + $variables: new SceneVariableSet({ + variables: [variable], + }), + controls: new DashboardControls({ + ...state, + }), + }); + + dashboard.activate(); + variable.activate(); + + return dashboard.state.controls as DashboardControls; +} diff --git a/public/app/features/dashboard-scene/scene/DashboardControls.tsx b/public/app/features/dashboard-scene/scene/DashboardControls.tsx index fb46c138d4a..39db075bc44 100644 --- a/public/app/features/dashboard-scene/scene/DashboardControls.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardControls.tsx @@ -1,7 +1,7 @@ import { css, cx } from '@emotion/css'; import React from 'react'; -import { GrafanaTheme2 } from '@grafana/data'; +import { GrafanaTheme2, VariableHide } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { SceneObjectState, @@ -12,6 +12,9 @@ import { SceneRefreshPicker, SceneDebugger, VariableDependencyConfig, + sceneGraph, + SceneObjectUrlSyncConfig, + SceneObjectUrlValues, } from '@grafana/scenes'; import { Box, Stack, useStyles2 } from '@grafana/ui'; @@ -20,12 +23,15 @@ import { getDashboardSceneFor } from '../utils/utils'; import { DashboardLinksControls } from './DashboardLinksControls'; -interface DashboardControlsState extends SceneObjectState { +export interface DashboardControlsState extends SceneObjectState { variableControls: SceneObject[]; timePicker: SceneTimePicker; refreshPicker: SceneRefreshPicker; hideTimeControls?: boolean; + hideVariableControls?: boolean; + hideLinksControls?: boolean; } + export class DashboardControls extends SceneObjectBase { static Component = DashboardControlsRenderer; @@ -33,6 +39,30 @@ export class DashboardControls extends SceneObjectBase { onAnyVariableChanged: this._onAnyVariableChanged.bind(this), }); + protected _urlSync = new SceneObjectUrlSyncConfig(this, { + keys: ['_dash.hideTimePicker', '_dash.hideVariables', '_dash.hideLinks'], + }); + + getUrlState() { + return { + '_dash.hideTimePicker': this.state.hideTimeControls ? 'true' : undefined, + '_dash.hideVariables': this.state.hideVariableControls ? 'true' : undefined, + '_dash.hideLinks': this.state.hideLinksControls ? 'true' : undefined, + }; + } + + updateFromUrl(values: SceneObjectUrlValues) { + const update: Partial = {}; + + update.hideTimeControls = values['_dash.hideTimePicker'] === 'true' || values['_dash.hideTimePicker'] === ''; + update.hideVariableControls = values['_dash.hideVariables'] === 'true' || values['_dash.hideVariables'] === ''; + update.hideLinksControls = values['_dash.hideLinks'] === 'true' || values['_dash.hideLinks'] === ''; + + if (Object.entries(update).some(([k, v]) => v !== this.state[k as keyof DashboardControlsState])) { + this.setState(update); + } + } + public constructor(state: Partial) { super({ variableControls: [], @@ -51,26 +81,42 @@ export class DashboardControls extends SceneObjectBase { this.forceRender(); } } + + public hasControls(): boolean { + const hasVariables = sceneGraph + .getVariables(this) + ?.state.variables.some((v) => v.state.hide !== VariableHide.hideVariable); + const hasAnnotations = sceneGraph.getDataLayers(this).some((d) => d.state.isEnabled && !d.state.isHidden); + const hasLinks = getDashboardSceneFor(this).state.links?.length > 0; + const hideLinks = this.state.hideLinksControls || !hasLinks; + const hideVariables = this.state.hideVariableControls || (!hasAnnotations && !hasVariables); + const hideTimePicker = this.state.hideTimeControls; + + return !(hideVariables && hideLinks && hideTimePicker); + } } function DashboardControlsRenderer({ model }: SceneComponentProps) { - const { variableControls, refreshPicker, timePicker, hideTimeControls } = model.useState(); + const { variableControls, refreshPicker, timePicker, hideTimeControls, hideVariableControls, hideLinksControls } = + model.useState(); const dashboard = getDashboardSceneFor(model); const { links, meta, editPanel } = dashboard.useState(); const styles = useStyles2(getStyles); const showDebugger = location.search.includes('scene-debugger'); + if (!model.hasControls()) { + return null; + } + return (
- {variableControls.map((c) => ( - - ))} + {!hideVariableControls && variableControls.map((c) => )} - {!editPanel && } + {!hideLinksControls && !editPanel && } {editPanel && } {!hideTimeControls && ( diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index 13e6ec7dff7..83b0edbc9c3 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.tsx @@ -35,7 +35,7 @@ import { DashboardModel, PanelModel } from 'app/features/dashboard/state'; import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher'; import { deleteDashboard } from 'app/features/manage-dashboards/state/actions'; import { VariablesChanged } from 'app/features/variables/types'; -import { DashboardDTO, DashboardMeta, SaveDashboardResponseDTO } from 'app/types'; +import { DashboardDTO, DashboardMeta, KioskMode, SaveDashboardResponseDTO } from 'app/types'; import { ShowConfirmModalEvent } from 'app/types/events'; import { PanelEditor } from '../panel-edit/PanelEditor'; @@ -125,6 +125,8 @@ export interface DashboardSceneState extends SceneObjectState { isEmpty?: boolean; /** Scene object that handles the scopes selector */ scopes?: ScopesScene; + /** Kiosk mode */ + kioskMode?: KioskMode; } export class DashboardScene extends SceneObjectBase { diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx index 6e9a5d659bb..f8e77d21856 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx @@ -24,6 +24,7 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps; const withPanels = ( -
+
); @@ -49,14 +50,14 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps {scopes && } - {!isHomePage && controls && ( + {!isHomePage && controls && hasControls && (
@@ -119,6 +120,9 @@ function getStyles(theme: GrafanaTheme2) { flexGrow: 0, gridArea: 'controls', padding: theme.spacing(2), + ':empty': { + display: 'none', + }, }), controlsWrapperWithScopes: css({ padding: theme.spacing(2, 2, 2, 0), @@ -139,7 +143,11 @@ function getStyles(theme: GrafanaTheme2) { flexGrow: 1, display: 'flex', gap: '8px', - marginBottom: theme.spacing(2), + paddingBottom: theme.spacing(2), + boxSizing: 'border-box', + }), + bodyWithoutControls: css({ + paddingTop: theme.spacing(2), }), }; } diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts index 984b7c488b0..c1a7172044f 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts +++ b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts @@ -1,6 +1,7 @@ import { AppEvents } from '@grafana/data'; import { SceneGridLayout, SceneQueryRunner, VizPanel } from '@grafana/scenes'; import appEvents from 'app/core/app_events'; +import { KioskMode } from 'app/types'; import { DashboardGridItem } from './DashboardGridItem'; import { DashboardScene } from './DashboardScene'; @@ -39,6 +40,29 @@ describe('DashboardSceneUrlSync', () => { (scene.state.body as SceneGridLayout).setState({ UNSAFE_fitPanels: true }); expect(scene.urlSync?.getUrlState().autofitpanels).toBe('true'); }); + + it('Should set kiosk mode when url has kiosk', () => { + const scene = buildTestScene(); + + scene.urlSync?.updateFromUrl({ kiosk: 'invalid' }); + expect(scene.state.kioskMode).toBe(undefined); + scene.urlSync?.updateFromUrl({ kiosk: '' }); + expect(scene.state.kioskMode).toBe(KioskMode.Full); + scene.urlSync?.updateFromUrl({ kiosk: 'tv' }); + expect(scene.state.kioskMode).toBe(KioskMode.TV); + scene.urlSync?.updateFromUrl({ kiosk: 'true' }); + expect(scene.state.kioskMode).toBe(KioskMode.Full); + }); + + it('Should get the kiosk mode from the scene state', () => { + const scene = buildTestScene(); + + expect(scene.urlSync?.getUrlState().kiosk).toBe(undefined); + scene.setState({ kioskMode: KioskMode.TV }); + expect(scene.urlSync?.getUrlState().kiosk).toBe(KioskMode.TV); + scene.setState({ kioskMode: KioskMode.Full }); + expect(scene.urlSync?.getUrlState().kiosk).toBe(''); + }); }); describe('entering edit mode', () => { diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts index 49fbd51be06..fb891974cfe 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts +++ b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts @@ -11,6 +11,7 @@ import { VizPanel, } from '@grafana/scenes'; import appEvents from 'app/core/app_events'; +import { KioskMode } from 'app/types'; import { PanelInspectDrawer } from '../inspect/PanelInspectDrawer'; import { buildPanelEditScene } from '../panel-edit/PanelEditor'; @@ -28,7 +29,7 @@ export class DashboardSceneUrlSync implements SceneObjectUrlSyncHandler { constructor(private _scene: DashboardScene) {} getKeys(): string[] { - return ['inspect', 'viewPanel', 'editPanel', 'editview', 'autofitpanels']; + return ['inspect', 'viewPanel', 'editPanel', 'editview', 'autofitpanels', 'kiosk']; } getUrlState(): SceneObjectUrlValues { @@ -39,6 +40,7 @@ export class DashboardSceneUrlSync implements SceneObjectUrlSyncHandler { viewPanel: state.viewPanelScene?.getUrlKey(), editview: state.editview?.getUrlKey(), editPanel: state.editPanel?.getUrlKey() || undefined, + kiosk: state.kioskMode === KioskMode.Full ? '' : state.kioskMode === KioskMode.TV ? 'tv' : undefined, }; } @@ -159,6 +161,14 @@ export class DashboardSceneUrlSync implements SceneObjectUrlSyncHandler { } } + if (typeof values.kiosk === 'string') { + if (values.kiosk === 'true' || values.kiosk === '') { + update.kioskMode = KioskMode.Full; + } else if (values.kiosk === 'tv') { + update.kioskMode = KioskMode.TV; + } + } + if (Object.keys(update).length > 0) { this._scene.setState(update); } diff --git a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModel.ts b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModel.ts index c6c0b3941a9..e633b7fb0ce 100644 --- a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModel.ts +++ b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModel.ts @@ -346,7 +346,6 @@ export function panelRepeaterToPanels( return [libraryVizPanelToPanel(repeater.state.body, { x, y, w, h })]; } - // console.log('repeater.state', repeater.state); if (repeater.state.repeatedPanels) { const itemHeight = repeater.state.itemHeight ?? 10; const rowCount = Math.ceil(repeater.state.repeatedPanels!.length / repeater.getMaxPerRow()); diff --git a/public/app/features/playlist/PlaylistSrv.ts b/public/app/features/playlist/PlaylistSrv.ts index 8fd715dd8f4..c193de2a032 100644 --- a/public/app/features/playlist/PlaylistSrv.ts +++ b/public/app/features/playlist/PlaylistSrv.ts @@ -12,6 +12,9 @@ export const queryParamsToPreserve: { [key: string]: boolean } = { kiosk: true, autofitpanels: true, orgId: true, + '_dash.hideTimePicker': true, + '_dash.hideVariables': true, + '_dash.hideLinks': true, }; export interface PlaylistSrvState { diff --git a/public/app/features/playlist/StartModal.tsx b/public/app/features/playlist/StartModal.tsx index 01d63469b58..ba88d771026 100644 --- a/public/app/features/playlist/StartModal.tsx +++ b/public/app/features/playlist/StartModal.tsx @@ -1,8 +1,8 @@ import React, { useState } from 'react'; import { SelectableValue, UrlQueryMap, urlUtil } from '@grafana/data'; -import { locationService } from '@grafana/runtime'; -import { Button, Checkbox, Field, FieldSet, Modal, RadioButtonGroup } from '@grafana/ui'; +import { config, locationService } from '@grafana/runtime'; +import { Box, Button, Checkbox, Field, FieldSet, Modal, RadioButtonGroup, Stack } from '@grafana/ui'; import { Playlist, PlaylistMode } from './types'; @@ -14,6 +14,9 @@ export interface Props { export const StartModal = ({ playlist, onDismiss }: Props) => { const [mode, setMode] = useState(false); const [autoFit, setAutofit] = useState(false); + const [displayTimePicker, setDisplayTimePicker] = useState(true); + const [displayVariables, setDisplayVariables] = useState(true); + const [displayLinks, setDisplayLinks] = useState(true); const modes: Array> = [ { label: 'Normal', value: false }, @@ -29,6 +32,17 @@ export const StartModal = ({ playlist, onDismiss }: Props) => { if (autoFit) { params.autofitpanels = true; } + + if (!displayTimePicker) { + params['_dash.hideTimePicker'] = true; + } + if (!displayVariables) { + params['_dash.hideVariables'] = true; + } + if (!displayLinks) { + params['_dash.hideLinks'] = true; + } + locationService.push(urlUtil.renderUrl(`/playlists/play/${playlist.uid}`, params)); }; @@ -38,13 +52,41 @@ export const StartModal = ({ playlist, onDismiss }: Props) => { - setAutofit(e.currentTarget.checked)} - /> + + setAutofit(e.currentTarget.checked)} + /> + + {config.featureToggles.dashboardScene && ( + + + + setDisplayTimePicker(e.currentTarget.checked)} + /> + setDisplayVariables(e.currentTarget.checked)} + /> + setDisplayLinks(e.currentTarget.checked)} + /> + + + + )}