From 09b51947f3df2a570c2ad88d388a8a6466f715d1 Mon Sep 17 00:00:00 2001 From: "grafana-pr-automation[bot]" <140550294+grafana-pr-automation[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:17:31 +0200 Subject: [PATCH 01/14] I18n: Download translations from Crowdin (#97311) --- public/locales/de-DE/grafana.json | 6 ++++++ public/locales/es-ES/grafana.json | 6 ++++++ public/locales/fr-FR/grafana.json | 6 ++++++ public/locales/pt-BR/grafana.json | 6 ++++++ public/locales/zh-Hans/grafana.json | 6 ++++++ 5 files changed, 30 insertions(+) diff --git a/public/locales/de-DE/grafana.json b/public/locales/de-DE/grafana.json index 0f53f4ac740..8870ce3246e 100644 --- a/public/locales/de-DE/grafana.json +++ b/public/locales/de-DE/grafana.json @@ -3336,6 +3336,12 @@ "info-box-content": "", "info-box-content-2": "", "title": "" + }, + "unknown-table": { + "loading": "", + "no-unknowns": "", + "renamed-or-missing-variables": "", + "variable": "" } } } \ No newline at end of file diff --git a/public/locales/es-ES/grafana.json b/public/locales/es-ES/grafana.json index e8974dc2e7e..dde7f5aff67 100644 --- a/public/locales/es-ES/grafana.json +++ b/public/locales/es-ES/grafana.json @@ -3336,6 +3336,12 @@ "info-box-content": "", "info-box-content-2": "", "title": "" + }, + "unknown-table": { + "loading": "", + "no-unknowns": "", + "renamed-or-missing-variables": "", + "variable": "" } } } \ No newline at end of file diff --git a/public/locales/fr-FR/grafana.json b/public/locales/fr-FR/grafana.json index fc0e768ddad..fd789dec216 100644 --- a/public/locales/fr-FR/grafana.json +++ b/public/locales/fr-FR/grafana.json @@ -3336,6 +3336,12 @@ "info-box-content": "", "info-box-content-2": "", "title": "" + }, + "unknown-table": { + "loading": "", + "no-unknowns": "", + "renamed-or-missing-variables": "", + "variable": "" } } } \ No newline at end of file diff --git a/public/locales/pt-BR/grafana.json b/public/locales/pt-BR/grafana.json index a1a24d462ee..2985b911ce2 100644 --- a/public/locales/pt-BR/grafana.json +++ b/public/locales/pt-BR/grafana.json @@ -3336,6 +3336,12 @@ "info-box-content": "", "info-box-content-2": "", "title": "" + }, + "unknown-table": { + "loading": "", + "no-unknowns": "", + "renamed-or-missing-variables": "", + "variable": "" } } } \ No newline at end of file diff --git a/public/locales/zh-Hans/grafana.json b/public/locales/zh-Hans/grafana.json index 7c003199924..2c9db3e0ec4 100644 --- a/public/locales/zh-Hans/grafana.json +++ b/public/locales/zh-Hans/grafana.json @@ -3322,6 +3322,12 @@ "info-box-content": "", "info-box-content-2": "", "title": "" + }, + "unknown-table": { + "loading": "", + "no-unknowns": "", + "renamed-or-missing-variables": "", + "variable": "" } } } \ No newline at end of file From e8e0649ebd924b2f0753a79454a3e83b093963df Mon Sep 17 00:00:00 2001 From: Joey <90795735+joey-grafana@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:17:47 +0000 Subject: [PATCH 02/14] Tempo: Update AggregateBy notice (#97630) * Update notice * Update link * Remove extra import * Update notice --- .../SearchTraceQLEditor/GroupByField.tsx | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.tsx b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.tsx index de8e1c3133d..d08b30d1f5f 100644 --- a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.tsx +++ b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.tsx @@ -86,7 +86,10 @@ export const GroupByField = (props: Props) => { .map((t) => ({ label: t, value: t })); return ( - + <> {query.groupBy?.map((f, i) => { const tags = tagOptions(f) @@ -152,15 +155,27 @@ export const GroupByField = (props: Props) => { ); })} {query.groupBy && query.groupBy.length > 0 && query.groupBy[0].tag && ( - + + The aggregate by feature is deprecated. We recommend using Explore Traces instead. If you want to write your + own TraceQL queries to replicate this API, please check + + this page + + . + )} ); }; -export const notice = 'The aggregate by feature is deprecated and will be removed in the future.'; - const getStyles = (theme: GrafanaTheme2) => ({ addTag: css({ marginLeft: theme.spacing(1), @@ -169,4 +184,9 @@ const getStyles = (theme: GrafanaTheme2) => ({ width: '500px', marginTop: theme.spacing(0.75), }), + noticeLink: css({ + color: theme.colors.text.link, + textDecoration: 'underline', + marginLeft: '5px', + }), }); From 61d71ec5b18991ffdcd903732dd1ec2eb9b20475 Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Wed, 11 Dec 2024 12:22:42 +0100 Subject: [PATCH 03/14] Zanzana: Handle anonymous users (#97171) * add anonymous users to schema * sync anonymous user role * remove unused * fix linter * only add anonymous reconciler if feature is enabled --- .../accesscontrol/dualwrite/collectors.go | 30 ++++++++++++++++ .../accesscontrol/dualwrite/reconciler.go | 36 ++++++++++++++++++- pkg/services/authz/zanzana/common/tuple.go | 1 + .../authz/zanzana/schema/schema_core.fga | 4 ++- pkg/services/authz/zanzana/zanzana.go | 1 + 5 files changed, 70 insertions(+), 2 deletions(-) diff --git a/pkg/services/accesscontrol/dualwrite/collectors.go b/pkg/services/accesscontrol/dualwrite/collectors.go index 0be8336069d..3d6b9b3e974 100644 --- a/pkg/services/accesscontrol/dualwrite/collectors.go +++ b/pkg/services/accesscontrol/dualwrite/collectors.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" "github.com/grafana/grafana/pkg/services/authz/zanzana" + "github.com/grafana/grafana/pkg/setting" ) func teamMembershipCollector(store db.DB) legacyTupleCollector { @@ -394,6 +395,35 @@ func rolePermissionsCollector(store db.DB) legacyTupleCollector { } } +// basicRoleBindingsCollector collects role bindings for basic roles +func anonymousRoleBindingsCollector(cfg *setting.Cfg, store db.DB) legacyTupleCollector { + return func(ctx context.Context, orgID int64) (map[string]map[string]*openfgav1.TupleKey, error) { + tuples := make(map[string]map[string]*openfgav1.TupleKey) + object := zanzana.NewTupleEntry(zanzana.TypeRole, zanzana.TranslateBasicRole(cfg.AnonymousOrgRole), "") + // Object should be set to delete obsolete permissions + tuples[object] = make(map[string]*openfgav1.TupleKey) + + o, err := getOrgByName(ctx, store, cfg.AnonymousOrgName) + if err != nil { + return tuples, nil + } + + if o.ID != orgID { + return tuples, nil + } + + tuple := &openfgav1.TupleKey{ + User: zanzana.NewTupleEntry(zanzana.TypeAnonymous, "0", ""), + Relation: zanzana.RelationAssignee, + Object: object, + } + + tuples[tuple.Object][tuple.String()] = tuple + + return tuples, nil + } +} + func zanzanaCollector(relations []string) zanzanaTupleCollector { return func(ctx context.Context, client zanzana.Client, object string, namespace string) (map[string]*openfgav1.TupleKey, error) { // list will use continuation token to collect all tuples for object and relation diff --git a/pkg/services/accesscontrol/dualwrite/reconciler.go b/pkg/services/accesscontrol/dualwrite/reconciler.go index 930327340fb..9a72f848cba 100644 --- a/pkg/services/accesscontrol/dualwrite/reconciler.go +++ b/pkg/services/accesscontrol/dualwrite/reconciler.go @@ -2,6 +2,7 @@ package dualwrite import ( "context" + "fmt" "strconv" "time" @@ -12,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/services/authz/zanzana" + "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/setting" ) @@ -33,7 +35,7 @@ type ZanzanaReconciler struct { } func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, lock *serverlock.ServerLockService) *ZanzanaReconciler { - return &ZanzanaReconciler{ + zanzanaReconciler := &ZanzanaReconciler{ cfg: cfg, log: log.New("zanzana.reconciler"), client: client, @@ -90,6 +92,19 @@ func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, ), }, } + + if cfg.AnonymousEnabled { + zanzanaReconciler.reconcilers = append(zanzanaReconciler.reconcilers, + newResourceReconciler( + "anonymous role binding", + anonymousRoleBindingsCollector(cfg, store), + zanzanaCollector([]string{zanzana.RelationAssignee}), + client, + ), + ) + } + + return zanzanaReconciler } // Reconcile schedules as job that will run and reconcile resources between @@ -182,3 +197,22 @@ func (r *ZanzanaReconciler) getOrgs(ctx context.Context) ([]int64, error) { } return orgs, nil } + +func getOrgByName(ctx context.Context, store db.DB, name string) (*org.Org, error) { + var orga org.Org + err := store.WithDbSession(ctx, func(dbSession *db.Session) error { + exists, err := dbSession.Where("name=?", name).Get(&orga) + if err != nil { + return err + } + if !exists { + return fmt.Errorf("org does not exist: %s", name) + } + return nil + }) + + if err != nil { + return nil, err + } + return &orga, nil +} diff --git a/pkg/services/authz/zanzana/common/tuple.go b/pkg/services/authz/zanzana/common/tuple.go index cc94052438c..bc4839c0128 100644 --- a/pkg/services/authz/zanzana/common/tuple.go +++ b/pkg/services/authz/zanzana/common/tuple.go @@ -14,6 +14,7 @@ const ( TypeUser string = "user" TypeServiceAccount string = "service-account" TypeRenderService string = "render" + TypeAnonymous string = "anonymous" TypeTeam string = "team" TypeRole string = "role" ) diff --git a/pkg/services/authz/zanzana/schema/schema_core.fga b/pkg/services/authz/zanzana/schema/schema_core.fga index 8dd5f382279..80375ad2d4a 100644 --- a/pkg/services/authz/zanzana/schema/schema_core.fga +++ b/pkg/services/authz/zanzana/schema/schema_core.fga @@ -6,9 +6,11 @@ type service-account type render +type anonymous + type role relations - define assignee: [user, service-account, team#member, role#assignee] + define assignee: [user, service-account, anonymous, team#member, role#assignee] type team relations diff --git a/pkg/services/authz/zanzana/zanzana.go b/pkg/services/authz/zanzana/zanzana.go index 5160a3f83d9..cf7d74ddaf3 100644 --- a/pkg/services/authz/zanzana/zanzana.go +++ b/pkg/services/authz/zanzana/zanzana.go @@ -14,6 +14,7 @@ const ( TypeUser = common.TypeUser TypeServiceAccount = common.TypeServiceAccount TypeRenderService = common.TypeRenderService + TypeAnonymous = common.TypeAnonymous TypeTeam = common.TypeTeam TypeRole = common.TypeRole TypeFolder = common.TypeFolder From f51b58488c70bb1d98a0020e5808b52e362912b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Wed, 11 Dec 2024 12:24:19 +0100 Subject: [PATCH 04/14] EmptyState: Add event tracking to `CommandPalette` and `BrowseDashboards` (#97780) --- .../app/features/browse-dashboards/BrowseDashboardsPage.tsx | 4 +++- public/app/features/commandPalette/CommandPalette.tsx | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx b/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx index 7e8e3d4135c..12b31ff4974 100644 --- a/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx +++ b/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx @@ -60,6 +60,9 @@ const BrowseDashboardsPage = memo(() => { if (!isSearching && searchState.result) { stateManager.setState({ result: undefined, includePanels: undefined }); } + if (isSearching && searchState.result?.totalRows === 0) { + reportInteraction('grafana_empty_state_shown', { source: 'browse_dashboards' }); + } }, [isSearching, searchState.result, stateManager]); const { data: folderDTO } = useGetFolderQuery(folderUID ?? skipToken); @@ -114,7 +117,6 @@ const BrowseDashboardsPage = memo(() => { origin: window.location.pathname === getConfig().appSubUrl + '/dashboards' ? 'Dashboards' : 'Folder view', }); }; - return ( { + showEmptyState && reportInteraction('grafana_empty_state_shown', { source: 'command_palette' }); + }, [showEmptyState]); return showEmptyState ? ( Date: Wed, 11 Dec 2024 14:46:31 +0300 Subject: [PATCH 05/14] Unified: Add client-side stats federation to support folders (#97778) --- pkg/storage/unified/client.go | 24 ++- pkg/storage/unified/federated/client.go | 45 ++++++ pkg/storage/unified/federated/stats.go | 70 ++++++++ pkg/storage/unified/federated/stats_test.go | 167 ++++++++++++++++++++ 4 files changed, 304 insertions(+), 2 deletions(-) create mode 100644 pkg/storage/unified/federated/client.go create mode 100644 pkg/storage/unified/federated/stats.go create mode 100644 pkg/storage/unified/federated/stats_test.go diff --git a/pkg/storage/unified/client.go b/pkg/storage/unified/client.go index e3c8a13ddbd..3c45d2135e4 100644 --- a/pkg/storage/unified/client.go +++ b/pkg/storage/unified/client.go @@ -20,6 +20,8 @@ import ( "github.com/grafana/grafana/pkg/services/authz" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/storage/legacysql" + "github.com/grafana/grafana/pkg/storage/unified/federated" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/sql" ) @@ -38,14 +40,32 @@ func ProvideUnifiedStorageClient( ) (resource.ResourceClient, error) { // See: apiserver.ApplyGrafanaConfig(cfg, features, o) apiserverCfg := cfg.SectionWithEnvOverrides("grafana-apiserver") - opts := options.StorageOptions{ + client, err := newClient(options.StorageOptions{ StorageType: options.StorageType(apiserverCfg.Key("storage_type").MustString(string(options.StorageTypeLegacy))), DataPath: apiserverCfg.Key("storage_path").MustString(filepath.Join(cfg.DataPath, "grafana-apiserver")), Address: apiserverCfg.Key("address").MustString(""), // client address BlobStoreURL: apiserverCfg.Key("blob_url").MustString(""), + }, cfg, features, db, tracer, reg, authzc, docs) + if err == nil { + // Used to get the folder stats + client = federated.NewFederatedClient( + client, // The original + legacysql.NewDatabaseProvider(db), + ) } - ctx := context.Background() + return client, err +} +func newClient(opts options.StorageOptions, + cfg *setting.Cfg, + features featuremgmt.FeatureToggles, + db infraDB.DB, + tracer tracing.Tracer, + reg prometheus.Registerer, + authzc authz.Client, + docs resource.DocumentBuilderSupplier, +) (resource.ResourceClient, error) { + ctx := context.Background() switch opts.StorageType { case options.StorageTypeFile: if opts.DataPath == "" { diff --git a/pkg/storage/unified/federated/client.go b/pkg/storage/unified/federated/client.go new file mode 100644 index 00000000000..666ef7d3625 --- /dev/null +++ b/pkg/storage/unified/federated/client.go @@ -0,0 +1,45 @@ +package federated + +import ( + "context" + + "google.golang.org/grpc" + + "github.com/grafana/grafana/pkg/storage/legacysql" + "github.com/grafana/grafana/pkg/storage/unified/resource" +) + +func NewFederatedClient(base resource.ResourceClient, sql legacysql.LegacyDatabaseProvider) resource.ResourceClient { + return &federatedClient{ + ResourceClient: base, + stats: &LegacyStatsGetter{ + SQL: sql, + }, + } +} + +type federatedClient struct { + resource.ResourceClient + + // Local DB for folder stats query + stats *LegacyStatsGetter +} + +// Get the resource stats +func (s *federatedClient) GetStats(ctx context.Context, in *resource.ResourceStatsRequest, opts ...grpc.CallOption) (*resource.ResourceStatsResponse, error) { + rsp, err := s.ResourceClient.GetStats(ctx, in, opts...) + if err != nil { + return nil, err + } + + // When folder stats are requested -- join in the legacy values + if in.Folder != "" { + more, err := s.stats.GetStats(ctx, in) + if err != nil { + return rsp, err + } + rsp.Stats = append(rsp.Stats, more.Stats...) + } + + return rsp, err +} diff --git a/pkg/storage/unified/federated/stats.go b/pkg/storage/unified/federated/stats.go new file mode 100644 index 00000000000..8e5f7c0033c --- /dev/null +++ b/pkg/storage/unified/federated/stats.go @@ -0,0 +1,70 @@ +package federated + +import ( + "context" + "fmt" + + "github.com/grafana/authlib/claims" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/storage/legacysql" + "github.com/grafana/grafana/pkg/storage/unified/resource" +) + +// Read stats from legacy SQL +type LegacyStatsGetter struct { + SQL legacysql.LegacyDatabaseProvider +} + +func (s *LegacyStatsGetter) GetStats(ctx context.Context, in *resource.ResourceStatsRequest) (*resource.ResourceStatsResponse, error) { + info, err := claims.ParseNamespace(in.Namespace) + if err != nil { + return nil, fmt.Errorf("unable to read namespace") + } + if info.OrgID == 0 { + return nil, fmt.Errorf("invalid OrgID found in namespace") + } + + helper, err := s.SQL(ctx) + if err != nil { + return nil, err + } + + rsp := &resource.ResourceStatsResponse{} + err = helper.DB.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + fn := func(table, where, g, r string) error { + count, err := sess.Table(helper.Table(table)).Where(where, info.OrgID, in.Folder).Count() + if err != nil { + return err + } + rsp.Stats = append(rsp.Stats, &resource.ResourceStatsResponse_Stats{ + Group: g, // all legacy for now + Resource: r, + Count: count, + }) + return nil + } + // Indicate that this came from the SQL tables + group := "sql-fallback" + + // Legacy alert rule table + err = fn("alert_rule", "org_id=? AND dashboard_uid=?", group, "alertrules") + if err != nil { + return err + } + + // Legacy dashboard table + err = fn("dashboard", "org_id=? AND folder_uid=?", group, "dashboards") + if err != nil { + return err + } + + // Legacy folder table + err = fn("folder", "org_id=? AND parent_uid=?", group, "folders") + if err != nil { + return err + } + return nil + }) + + return rsp, err +} diff --git a/pkg/storage/unified/federated/stats_test.go b/pkg/storage/unified/federated/stats_test.go new file mode 100644 index 00000000000..56181303632 --- /dev/null +++ b/pkg/storage/unified/federated/stats_test.go @@ -0,0 +1,167 @@ +package federated + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/require" + "k8s.io/apiserver/pkg/endpoints/request" + + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/expr" + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" + "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol/testutil" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/dashboards/database" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/folder/folderimpl" + "github.com/grafana/grafana/pkg/services/guardian" + ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" + ngalertstore "github.com/grafana/grafana/pkg/services/ngalert/store" + "github.com/grafana/grafana/pkg/services/quota/quotatest" + "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" + "github.com/grafana/grafana/pkg/services/tag/tagimpl" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/storage/legacysql" + "github.com/grafana/grafana/pkg/storage/unified/resource" + "github.com/grafana/grafana/pkg/tests/testsuite" +) + +func TestMain(m *testing.M) { + testsuite.Run(m) +} + +func TestDirectSQLStats(t *testing.T) { + db, cfg := db.InitTestDBWithCfg(t) + ctx := context.Background() + + dashStore, err := database.ProvideDashboardStore(db, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(db), quotatest.New(false, nil)) + require.NoError(t, err) + fakeGuardian := &guardian.FakeDashboardGuardian{ + CanSaveValue: true, + CanEditUIDs: []string{}, + CanViewUIDs: []string{}, + } + guardian.MockDashboardGuardian(fakeGuardian) + folderPermissions, err := testutil.ProvideFolderPermissions(featuremgmt.WithFeatures(), cfg, db) + require.NoError(t, err) + fStore := folderimpl.ProvideStore(db) + folderSvc := folderimpl.ProvideService(fStore, actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, + folderimpl.ProvideDashboardFolderStore(db), db, featuremgmt.WithFeatures(), cfg, folderPermissions, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest()) + + // create parent folder + + tempUser := &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{}} + + // create folders - test2 is nested in test1 + folder1UID := "test1" + now := time.Now() + _, err = folderSvc.Create(ctx, &folder.CreateFolderCommand{Title: "test1", UID: folder1UID, OrgID: 1, SignedInUser: tempUser}) + require.NoError(t, err) + folder2UID := "test2" + _, err = folderSvc.Create(ctx, &folder.CreateFolderCommand{Title: "test2", UID: folder2UID, OrgID: 1, ParentUID: folder1UID, SignedInUser: tempUser}) + require.NoError(t, err) + + // create an alert rule inside of folder test2 + ruleStore := ngalertstore.SetupStoreForTesting(t, db) + _, err = ruleStore.InsertAlertRules(context.Background(), []ngmodels.AlertRule{ + { + DashboardUID: &folder2UID, + UID: "test", + Title: "test", + OrgID: 1, + Data: []ngmodels.AlertQuery{ + { + RefID: "A", + Model: json.RawMessage("{}"), + DatasourceUID: expr.DatasourceUID, + RelativeTimeRange: ngmodels.RelativeTimeRange{ + From: ngmodels.Duration(60), + To: ngmodels.Duration(0), + }, + }, + }, + Condition: "ok", + Updated: now, + NamespaceUID: "test", + ExecErrState: ngmodels.ExecutionErrorState(ngmodels.Alerting), + NoDataState: ngmodels.Alerting, + IntervalSeconds: 60, + }}) + require.NoError(t, err) + + // finally, create dashboard inside of test1 + _, err = dashStore.SaveDashboard(ctx, dashboards.SaveDashboardCommand{ + Dashboard: simplejson.New(), + FolderUID: folder1UID, + OrgID: 1, + }) + require.NoError(t, err) + + store := &LegacyStatsGetter{ + SQL: legacysql.NewDatabaseProvider(db), + } + + t.Run("GetStatsForFolder1", func(t *testing.T) { + ctx := context.Background() + ctx = request.WithNamespace(ctx, "default") + + stats, err := store.GetStats(ctx, &resource.ResourceStatsRequest{ + Namespace: "default", + Folder: folder1UID, + }) + require.NoError(t, err) + + jj, _ := json.MarshalIndent(stats.Stats, "", " ") + require.JSONEq(t, `[ + { + "group": "sql-fallback", + "resource": "alertrules" + }, + { + "group": "sql-fallback", + "resource": "dashboards", + "count": 1 + }, + { + "group": "sql-fallback", + "resource": "folders", + "count": 1 + } + ]`, string(jj)) + }) + + t.Run("GetStatsForFolder2", func(t *testing.T) { + ctx := context.Background() + ctx = request.WithNamespace(ctx, "default") + + stats, err := store.GetStats(ctx, &resource.ResourceStatsRequest{ + Namespace: "default", + Folder: folder2UID, + }) + require.NoError(t, err) + + jj, _ := json.MarshalIndent(stats.Stats, "", " ") + require.JSONEq(t, `[ + { + "group": "sql-fallback", + "resource": "alertrules", + "count": 1 + }, + { + "group": "sql-fallback", + "resource": "dashboards" + }, + { + "group": "sql-fallback", + "resource": "folders" + } + ]`, string(jj)) + }) +} From bb673fc8ed9c7488263011cad938f4f6d4b07a54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Wed, 11 Dec 2024 13:53:28 +0100 Subject: [PATCH 06/14] Explore: Show links to queryless apps (#96625) * Extract basic extensions to a separate files * Add simple queryless apps links * Move links for queryless apps next to the datasource picker * Update tests * Add translations * Add tracking * Update translations * Fix tests and betterer * Fix the mock for the test (the hook may be called twice now) * Add a todo --- .betterer.results | 3 - public/app/features/explore/Explore.test.tsx | 2 +- .../app/features/explore/ExploreToolbar.tsx | 13 +- .../extensions/ToolbarExtensionPoint.test.tsx | 144 ++++++++++++++++-- .../extensions/ToolbarExtensionPoint.tsx | 71 ++++----- .../extensions/toolbar/BasicExtensions.tsx | 47 ++++++ .../toolbar/QuerylessAppsExtensions.tsx | 42 +++++ .../explore/extensions/toolbar/types.ts | 10 ++ public/locales/en-US/grafana.json | 2 + public/locales/pseudo-LOCALE/grafana.json | 2 + 10 files changed, 286 insertions(+), 50 deletions(-) create mode 100644 public/app/features/explore/extensions/toolbar/BasicExtensions.tsx create mode 100644 public/app/features/explore/extensions/toolbar/QuerylessAppsExtensions.tsx create mode 100644 public/app/features/explore/extensions/toolbar/types.ts diff --git a/.betterer.results b/.betterer.results index cc2d71fb109..510154a2371 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3148,9 +3148,6 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] ], - "public/app/features/explore/extensions/ToolbarExtensionPoint.tsx:5381": [ - [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] - ], "public/app/features/explore/hooks/useStateSync/index.ts:5381": [ [0, 0, 0, "Do not re-export imported variable (\`./external.utils\`)", "0"] ], diff --git a/public/app/features/explore/Explore.test.tsx b/public/app/features/explore/Explore.test.tsx index 91dc7f35ec7..92e5204d742 100644 --- a/public/app/features/explore/Explore.test.tsx +++ b/public/app/features/explore/Explore.test.tsx @@ -180,7 +180,7 @@ describe('Explore', () => { }); it('should render toolbar extension point if extensions is available', async () => { - usePluginLinksMock.mockReturnValueOnce({ + usePluginLinksMock.mockReturnValue({ links: [ { id: '1', diff --git a/public/app/features/explore/ExploreToolbar.tsx b/public/app/features/explore/ExploreToolbar.tsx index 9345d58b2c1..2cfd2cb39ce 100644 --- a/public/app/features/explore/ExploreToolbar.tsx +++ b/public/app/features/explore/ExploreToolbar.tsx @@ -255,6 +255,12 @@ export function ExploreToolbar({ exploreId, onChangeTime, onContentOutlineToogle hideTextValue={showSmallDataSourcePicker} width={showSmallDataSourcePicker ? 8 : undefined} />, + , ].filter(Boolean)} forceShowLeftItems > @@ -295,7 +301,12 @@ export function ExploreToolbar({ exploreId, onChangeTime, onContentOutlineToogle ), - , + , !isLive && ( {children}, {}); } +function setupToolbarExtensionPoint( + { noTimezone, showQuerylessApps }: { noTimezone?: boolean; showQuerylessApps?: boolean } = { noTimezone: false } +) { + return ( + + ); +} + describe('ToolbarExtensionPoint', () => { describe('with extension points', () => { beforeAll(() => { @@ -79,13 +91,13 @@ describe('ToolbarExtensionPoint', () => { }); it('should render "Add" extension point menu button', () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); expect(screen.getByRole('button', { name: 'Add' })).toBeVisible(); }); it('should render menu with extensions when "Add" is clicked', async () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); await userEvent.click(screen.getByRole('button', { name: 'Add' })); @@ -95,7 +107,7 @@ describe('ToolbarExtensionPoint', () => { }); it('should call onClick from extension when menu item is clicked', async () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); await userEvent.click(screen.getByRole('button', { name: 'Add' })); await userEvent.click(screen.getByRole('menuitem', { name: 'Add to dashboard' })); @@ -109,7 +121,7 @@ describe('ToolbarExtensionPoint', () => { }); it('should render confirm navigation modal when extension with path is clicked', async () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); await userEvent.click(screen.getByRole('button', { name: 'Add' })); await userEvent.click(screen.getByRole('menuitem', { name: 'ML: Forecast' })); @@ -123,7 +135,7 @@ describe('ToolbarExtensionPoint', () => { const targets = [{ refId: 'A' }]; const data = createEmptyQueryResponse(); - renderWithExploreStore(, { + renderWithExploreStore(setupToolbarExtensionPoint(), { targets, data, }); @@ -148,7 +160,7 @@ describe('ToolbarExtensionPoint', () => { const targets = [{ refId: 'A' }]; const data = createEmptyQueryResponse(); - renderWithExploreStore(, { + renderWithExploreStore(setupToolbarExtensionPoint({ noTimezone: true }), { targets, data, }); @@ -160,7 +172,7 @@ describe('ToolbarExtensionPoint', () => { }); it('should correct extension point id when fetching extensions', async () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); const [options] = usePluginLinksMock.mock.calls[0]; const { extensionPointId } = options; @@ -195,13 +207,13 @@ describe('ToolbarExtensionPoint', () => { }); it('should render "Add" extension point menu button', () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); expect(screen.getByRole('button', { name: 'Add' })).toBeVisible(); }); it('should render menu with extensions when "Add" is clicked', async () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); await userEvent.click(screen.getByRole('button', { name: 'Add' })); @@ -219,7 +231,7 @@ describe('ToolbarExtensionPoint', () => { }); it('should render "add to dashboard" action button if one pane is visible', async () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); await waitFor(() => { const button = screen.getByRole('button', { name: /add to dashboard/i }); @@ -237,9 +249,119 @@ describe('ToolbarExtensionPoint', () => { }); it('should not render "add to dashboard" action button', async () => { - renderWithExploreStore(); + renderWithExploreStore(setupToolbarExtensionPoint()); expect(screen.queryByRole('button', { name: /add to dashboard/i })).not.toBeInTheDocument(); }); }); + + describe('with multiple queryless apps links', () => { + beforeAll(() => { + usePluginLinksMock.mockReturnValue({ + links: [ + { + pluginId: 'grafana', + id: '1', + type: PluginExtensionTypes.link, + title: 'Add to dashboard', + category: 'Dashboards', + description: 'Add the current query as a panel to a dashboard', + onClick: jest.fn(), + }, + { + pluginId: 'grafana-ml-app', + id: '2', + type: PluginExtensionTypes.link, + title: 'ML: Forecast', + description: 'Add the query as a ML forecast', + path: '/a/grafana-ml-ap/forecast', + }, + { + pluginId: 'grafana-pyroscope-app', + id: '3', + type: PluginExtensionTypes.link, + title: 'Explore Profiles', + description: 'Explore Profiles', + path: '/a/grafana-pyroscope-app', + }, + { + pluginId: 'grafana-lokiexplore-app', + id: '4', + type: PluginExtensionTypes.link, + title: 'Explore Logs', + description: 'Explore Logs', + path: '/a/grafana-lokiexplore-app', + }, + ], + isLoading: false, + }); + }); + + it('should render menu with extensions without queryless apps when "Add" is clicked', async () => { + renderWithExploreStore(setupToolbarExtensionPoint()); + + await userEvent.click(screen.getByRole('button', { name: 'Add' })); + + expect(screen.getByRole('group', { name: 'Dashboards' })).toBeVisible(); + expect(screen.getByRole('menuitem', { name: 'Add to dashboard' })).toBeVisible(); + expect(screen.getByRole('menuitem', { name: 'ML: Forecast' })).toBeVisible(); + expect(screen.queryByRole('menuitem', { name: 'Explore Profiles' })).not.toBeInTheDocument(); + }); + + it('should render queryless apps links', async () => { + renderWithExploreStore(setupToolbarExtensionPoint({ showQuerylessApps: true })); + + await userEvent.click(screen.getByRole('button', { name: /go queryless/i })); + + expect(screen.queryByRole('group', { name: 'Dashboards' })).not.toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: 'Add to dashboard' })).not.toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: 'ML: Forecast' })).not.toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: 'Explore Profiles' })).toBeVisible(); + expect(screen.getByRole('menuitem', { name: 'Explore Logs' })).toBeVisible(); + }); + }); + + describe('with single queryless apps link', () => { + beforeAll(() => { + usePluginLinksMock.mockReturnValue({ + links: [ + { + pluginId: 'grafana', + id: '1', + type: PluginExtensionTypes.link, + title: 'Add to dashboard', + category: 'Dashboards', + description: 'Add the current query as a panel to a dashboard', + onClick: jest.fn(), + }, + { + pluginId: 'grafana-ml-app', + id: '2', + type: PluginExtensionTypes.link, + title: 'ML: Forecast', + description: 'Add the query as a ML forecast', + path: '/a/grafana-ml-ap/forecast', + }, + { + pluginId: 'grafana-pyroscope-app', + id: '3', + type: PluginExtensionTypes.link, + title: 'Explore Profiles', + description: 'Explore Profiles', + path: '/a/grafana-pyroscope-app', + }, + ], + isLoading: false, + }); + }); + + it('should render single queryless app link', async () => { + renderWithExploreStore(setupToolbarExtensionPoint({ showQuerylessApps: true })); + + await userEvent.click(screen.getByRole('button', { name: /go queryless/i })); + + expect(screen.queryByRole('menuitem', { name: 'Explore Profiles' })).not.toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: 'Explore Logs' })).not.toBeInTheDocument(); + }); + }); }); diff --git a/public/app/features/explore/extensions/ToolbarExtensionPoint.tsx b/public/app/features/explore/extensions/ToolbarExtensionPoint.tsx index 399ba36ee9a..35d027b2076 100644 --- a/public/app/features/explore/extensions/ToolbarExtensionPoint.tsx +++ b/public/app/features/explore/extensions/ToolbarExtensionPoint.tsx @@ -1,66 +1,69 @@ -import { lazy, ReactElement, Suspense, useMemo, useState } from 'react'; +import { ReactElement, useMemo, useState } from 'react'; import { type PluginExtensionLink, PluginExtensionPoints, RawTimeRange, getTimeZone } from '@grafana/data'; -import { config, usePluginLinks } from '@grafana/runtime'; +import { config, reportInteraction, usePluginLinks } from '@grafana/runtime'; import { DataQuery, TimeZone } from '@grafana/schema'; -import { Dropdown, ToolbarButton } from '@grafana/ui'; import { contextSrv } from 'app/core/services/context_srv'; import { AccessControlAction, ExplorePanelData, useSelector } from 'app/types'; import { getExploreItemSelector, isLeftPaneSelector, selectCorrelationDetails } from '../state/selectors'; import { ConfirmNavigationModal } from './ConfirmNavigationModal'; -import { ToolbarExtensionPointMenu } from './ToolbarExtensionPointMenu'; - -const AddToDashboard = lazy(() => - import('./AddToDashboard').then(({ AddToDashboard }) => ({ default: AddToDashboard })) -); +import { BasicExtensions } from './toolbar/BasicExtensions'; +import { QuerylessAppsExtensions } from './toolbar/QuerylessAppsExtensions'; type Props = { exploreId: string; timeZone: TimeZone; + extensionsToShow: 'queryless' | 'basic'; }; +const QUERYLESS_APPS = ['grafana-pyroscope-app', 'grafana-lokiexplore-app', 'grafana-exploretraces-app']; + export function ToolbarExtensionPoint(props: Props): ReactElement | null { - const { exploreId } = props; + const { exploreId, extensionsToShow } = props; const [selectedExtension, setSelectedExtension] = useState(); const [isOpen, setIsOpen] = useState(false); const context = useExtensionPointContext(props); + // TODO: Pull it up to avoid calling it twice const { links } = usePluginLinks({ extensionPointId: PluginExtensionPoints.ExploreToolbarAction, context: context, limitPerPlugin: 3, }); const selectExploreItem = getExploreItemSelector(exploreId); - const noQueriesInPane = useSelector(selectExploreItem)?.queries?.length; + const noQueriesInPane = Boolean(useSelector(selectExploreItem)?.queries?.length); - // If we only have the explore core extension point registered we show the old way of - // adding a query to a dashboard. - if (links.length <= 1) { - const canAddPanelToDashboard = - contextSrv.hasPermission(AccessControlAction.DashboardsCreate) || - contextSrv.hasPermission(AccessControlAction.DashboardsWrite); - - if (!canAddPanelToDashboard) { - return null; - } - - return ( - - - - ); - } - - const menu = ; + const querylessLinks = links.filter((link) => QUERYLESS_APPS.includes(link.pluginId)); + const commonLinks = links.filter((link) => !QUERYLESS_APPS.includes(link.pluginId)); return ( <> - - - Add - - + {extensionsToShow === 'queryless' && ( + { + setSelectedExtension(extension); + reportInteraction('grafana_explore_queryless_app_link_clicked', { + pluginId: extension.pluginId, + }); + }} + setIsModalOpen={setIsOpen} + isModalOpen={isOpen} + /> + )} + {extensionsToShow === 'basic' && ( + + )} {!!selectedExtension && !!selectedExtension.path && ( + import('./../AddToDashboard').then(({ AddToDashboard }) => ({ default: AddToDashboard })) +); + +export function BasicExtensions(props: ExtensionDropdownProps) { + const { exploreId, links, setSelectedExtension, setIsModalOpen, isModalOpen, noQueriesInPane } = props; + // If we only have the explore core extension point registered we show the old way of + // adding a query to a dashboard. + if (links.length <= 1) { + const canAddPanelToDashboard = + contextSrv.hasPermission(AccessControlAction.DashboardsCreate) || + contextSrv.hasPermission(AccessControlAction.DashboardsWrite); + + if (!canAddPanelToDashboard) { + return null; + } + + return ( + + + + ); + } + + const menu = ; + + return ( + <> + + + Add + + + + ); +} diff --git a/public/app/features/explore/extensions/toolbar/QuerylessAppsExtensions.tsx b/public/app/features/explore/extensions/toolbar/QuerylessAppsExtensions.tsx new file mode 100644 index 00000000000..7433c74393c --- /dev/null +++ b/public/app/features/explore/extensions/toolbar/QuerylessAppsExtensions.tsx @@ -0,0 +1,42 @@ +import { first } from 'lodash'; + +import { Dropdown, ToolbarButton } from '@grafana/ui'; + +import { Trans } from '../../../../core/internationalization'; +import { ToolbarExtensionPointMenu } from '../ToolbarExtensionPointMenu'; + +import { ExtensionDropdownProps } from './types'; + +export function QuerylessAppsExtensions(props: ExtensionDropdownProps) { + const { links, setSelectedExtension, setIsModalOpen, isModalOpen, noQueriesInPane } = props; + + if (links.length === 0) { + return undefined; + } + + const menu = ; + + if (links.length === 1) { + const link = first(links)!; + return ( + setSelectedExtension(link)}> + Go queryless + + ); + } + + return ( + <> + + + Go queryless + + + + ); +} diff --git a/public/app/features/explore/extensions/toolbar/types.ts b/public/app/features/explore/extensions/toolbar/types.ts new file mode 100644 index 00000000000..a23b83b51bd --- /dev/null +++ b/public/app/features/explore/extensions/toolbar/types.ts @@ -0,0 +1,10 @@ +import { PluginExtensionLink } from '@grafana/data'; + +export type ExtensionDropdownProps = { + links: PluginExtensionLink[]; + exploreId: string; + setSelectedExtension: (extension: PluginExtensionLink) => void; + setIsModalOpen: (value: boolean) => void; + isModalOpen: boolean; + noQueriesInPane: boolean; +}; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index f430b69d65d..f747547a8a0 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -1261,6 +1261,8 @@ "title-with-name": "Table - {{name}}" }, "toolbar": { + "add-to-extensions": "Add", + "add-to-queryless-extensions": "Go queryless", "aria-label": "Explore toolbar", "copy-link": "Copy URL", "copy-link-abs-time": "Copy absolute URL", diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index fe6defcf56c..e73d0cf088e 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -1261,6 +1261,8 @@ "title-with-name": "Ŧäþľę - {{name}}" }, "toolbar": { + "add-to-extensions": "Åđđ", + "add-to-queryless-extensions": "Ğő qūęřyľęşş", "aria-label": "Ēχpľőřę ŧőőľþäř", "copy-link": "Cőpy ŮŖĿ", "copy-link-abs-time": "Cőpy äþşőľūŧę ŮŖĿ", From d1760760780b99e5bd0ba57e38a52ad28c3eb5e4 Mon Sep 17 00:00:00 2001 From: Adam Yeats <16296989+adamyeats@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:02:51 +0000 Subject: [PATCH 07/14] Azure Monitor: Add safety around usage of frame.Meta.Custom struct (#97766) --- .../azure-log-analytics-datasource.go | 53 ++++++++++++------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/pkg/tsdb/azuremonitor/loganalytics/azure-log-analytics-datasource.go b/pkg/tsdb/azuremonitor/loganalytics/azure-log-analytics-datasource.go index dee69d9fe5c..93db9a8ebbd 100644 --- a/pkg/tsdb/azuremonitor/loganalytics/azure-log-analytics-datasource.go +++ b/pkg/tsdb/azuremonitor/loganalytics/azure-log-analytics-datasource.go @@ -320,14 +320,22 @@ func (e *AzureLogAnalyticsDatasource) executeQuery(ctx context.Context, query *A } logLimitDisabled := backend.GrafanaConfigFromContext(ctx).FeatureToggles().IsEnabled("azureMonitorDisableLogLimit") + frame, err := ResponseTableToFrame(t, query.RefID, query.Query, query.QueryType, query.ResultFormat, logLimitDisabled) if err != nil { return nil, err } + frame = appendErrorNotice(frame, logResponse.Error) if frame == nil { - dataResponse := backend.DataResponse{} - return &dataResponse, nil + return &backend.DataResponse{}, nil + } + + // Ensure Meta.Custom is initialized + if frame.Meta.Custom == nil { + frame.Meta.Custom = &LogAnalyticsMeta{ + ColumnTypes: make([]string, 0), + } } queryUrl, err := getQueryUrl(query.Query, query.Resources, dsInfo.Routes["Azure Portal"].URL, query.TimeRange) @@ -335,30 +343,37 @@ func (e *AzureLogAnalyticsDatasource) executeQuery(ctx context.Context, query *A return nil, err } - if (query.QueryType == dataquery.AzureQueryTypeAzureTraces || query.QueryType == dataquery.AzureQueryTypeTraceql) && query.ResultFormat == dataquery.ResultFormatTrace { - frame.Meta.PreferredVisualization = data.VisTypeTrace - } - - if query.ResultFormat == dataquery.ResultFormatTable { - frame.Meta.PreferredVisualization = data.VisTypeTable - } - - if query.ResultFormat == dataquery.ResultFormatLogs { - frame.Meta.PreferredVisualization = data.VisTypeLogs - frame.Meta.Custom = &LogAnalyticsMeta{ - ColumnTypes: frame.Meta.Custom.(*LogAnalyticsMeta).ColumnTypes, - AzurePortalLink: queryUrl, + // Set the preferred visualization + switch query.ResultFormat { + case dataquery.ResultFormatTrace: + if query.QueryType == dataquery.AzureQueryTypeAzureTraces || query.QueryType == dataquery.AzureQueryTypeTraceql { + frame.Meta.PreferredVisualization = data.VisTypeTrace } - } - - if query.ResultFormat == dataquery.ResultFormatTimeSeries { + case dataquery.ResultFormatTable: + frame.Meta.PreferredVisualization = data.VisTypeTable + case dataquery.ResultFormatLogs: + frame.Meta.PreferredVisualization = data.VisTypeLogs + if logMeta, ok := frame.Meta.Custom.(*LogAnalyticsMeta); ok { + frame.Meta.Custom = &LogAnalyticsMeta{ + ColumnTypes: logMeta.ColumnTypes, + AzurePortalLink: queryUrl, + } + } else { + frame.Meta.Custom = &LogAnalyticsMeta{ + AzurePortalLink: queryUrl, + } + } + case dataquery.ResultFormatTimeSeries: tsSchema := frame.TimeSeriesSchema() if tsSchema.Type == data.TimeSeriesTypeLong { wideFrame, err := data.LongToWide(frame, nil) if err == nil { frame = wideFrame } else { - frame.AppendNotices(data.Notice{Severity: data.NoticeSeverityWarning, Text: "could not convert frame to time series, returning raw table: " + err.Error()}) + frame.AppendNotices(data.Notice{ + Severity: data.NoticeSeverityWarning, + Text: "could not convert frame to time series, returning raw table: " + err.Error(), + }) } } } From d48e9afd5dbd528cea146e0c954d12eb98274708 Mon Sep 17 00:00:00 2001 From: Sriram <153843+yesoreyeram@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:05:06 +0000 Subject: [PATCH 08/14] Datasources: Improved network health check error messages (sql datasources) (#97783) remove unique identifiers from common network error messages --- .../sqleng/handler_checkhealth.go | 18 +++++++++++++++++- pkg/tsdb/mssql/sqleng/handler_checkhealth.go | 18 +++++++++++++++++- pkg/tsdb/mysql/sqleng/handler_checkhealth.go | 18 +++++++++++++++++- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go index a8584dfc59a..abe98fff713 100644 --- a/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go +++ b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go @@ -44,7 +44,23 @@ func ErrToHealthCheckResult(err error) (*backend.CheckHealthResult, error) { if errors.As(err, &opErr) { res.Message = "Network error: Failed to connect to the server" if opErr != nil && opErr.Err != nil { - res.Message += fmt.Sprintf(". Error message: %s", opErr.Err.Error()) + errMessage := opErr.Err.Error() + if strings.HasSuffix(opErr.Err.Error(), "no such host") { + errMessage = "no such host" + } + if strings.HasSuffix(opErr.Err.Error(), "unknown port") { + errMessage = "unknown port" + } + if strings.HasSuffix(opErr.Err.Error(), "invalid port") { + errMessage = "invalid port" + } + if strings.HasSuffix(opErr.Err.Error(), "missing port in address") { + errMessage = "missing port in address" + } + if strings.HasSuffix(opErr.Err.Error(), "invalid syntax") { + errMessage = "invalid syntax found in the address" + } + res.Message += fmt.Sprintf(". Error message: %s", errMessage) } } if errors.Is(err, pq.ErrSSLNotSupported) { diff --git a/pkg/tsdb/mssql/sqleng/handler_checkhealth.go b/pkg/tsdb/mssql/sqleng/handler_checkhealth.go index ade94fe7349..133be66a70a 100644 --- a/pkg/tsdb/mssql/sqleng/handler_checkhealth.go +++ b/pkg/tsdb/mssql/sqleng/handler_checkhealth.go @@ -38,7 +38,23 @@ func ErrToHealthCheckResult(err error) (*backend.CheckHealthResult, error) { if errors.As(err, &opErr) { res.Message = "Network error: Failed to connect to the server" if opErr != nil && opErr.Err != nil { - res.Message += fmt.Sprintf(". Error message: %s", opErr.Err.Error()) + errMessage := opErr.Err.Error() + if strings.HasSuffix(opErr.Err.Error(), "no such host") { + errMessage = "no such host" + } + if strings.HasSuffix(opErr.Err.Error(), "unknown port") { + errMessage = "unknown port" + } + if strings.HasSuffix(opErr.Err.Error(), "invalid port") { + errMessage = "invalid port" + } + if strings.HasSuffix(opErr.Err.Error(), "missing port in address") { + errMessage = "missing port in address" + } + if strings.HasSuffix(opErr.Err.Error(), "invalid syntax") { + errMessage = "invalid syntax found in the address" + } + res.Message += fmt.Sprintf(". Error message: %s", errMessage) } } if strings.HasPrefix(err.Error(), "mssql: ") { diff --git a/pkg/tsdb/mysql/sqleng/handler_checkhealth.go b/pkg/tsdb/mysql/sqleng/handler_checkhealth.go index fea2b0b8f8f..4cc7ed6abf9 100644 --- a/pkg/tsdb/mysql/sqleng/handler_checkhealth.go +++ b/pkg/tsdb/mysql/sqleng/handler_checkhealth.go @@ -41,7 +41,23 @@ func ErrToHealthCheckResult(err error) (*backend.CheckHealthResult, error) { if errors.As(err, &opErr) { res.Message = "Network error: Failed to connect to the server" if opErr != nil && opErr.Err != nil { - res.Message += fmt.Sprintf(". Error message: %s", opErr.Err.Error()) + errMessage := opErr.Err.Error() + if strings.HasSuffix(opErr.Err.Error(), "no such host") { + errMessage = "no such host" + } + if strings.HasSuffix(opErr.Err.Error(), "unknown port") { + errMessage = "unknown port" + } + if strings.HasSuffix(opErr.Err.Error(), "invalid port") { + errMessage = "invalid port" + } + if strings.HasSuffix(opErr.Err.Error(), "missing port in address") { + errMessage = "missing port in address" + } + if strings.HasSuffix(opErr.Err.Error(), "invalid syntax") { + errMessage = "invalid syntax found in the address" + } + res.Message += fmt.Sprintf(". Error message: %s", errMessage) } details["verboseMessage"] = err.Error() details["errorDetailsLink"] = "https://grafana.com/docs/grafana/latest/datasources/mysql/#configure-the-data-source" From 32155b22d6e786cf9ce660a22b48ac0e0f13020a Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Wed, 11 Dec 2024 13:27:54 +0000 Subject: [PATCH 09/14] Theme: increase debug theme gridSize and fix some obvious mistakes (#97661) * increase debug theme gridSize and fix some obvious mistakes * better switch styling * fix checkbox * fix tabs * also fix editable title height --- .../grafana-data/src/themes/createComponents.ts | 2 +- packages/grafana-data/src/themes/registry.ts | 3 +++ .../grafana-ui/src/components/Forms/Checkbox.tsx | 6 +++--- .../grafana-ui/src/components/Menu/MenuItem.tsx | 1 + .../src/components/Select/SelectContainer.tsx | 2 +- .../grafana-ui/src/components/Switch/Switch.tsx | 13 +++++++------ .../components/TabbedContainer/TabbedContainer.tsx | 2 +- packages/grafana-ui/src/components/Tabs/TabsBar.tsx | 4 ++-- public/app/core/components/Page/EditableTitle.tsx | 1 + 9 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/grafana-data/src/themes/createComponents.ts b/packages/grafana-data/src/themes/createComponents.ts index 13e7fd46f5a..e19a16b327c 100644 --- a/packages/grafana-data/src/themes/createComponents.ts +++ b/packages/grafana-data/src/themes/createComponents.ts @@ -97,7 +97,7 @@ export function createComponents(colors: ThemeColors, shadows: ThemeShadows): Th width: 57, }, menuTabs: { - height: 42, + height: 5, }, textHighlight: { text: colors.warning.contrastText, diff --git a/packages/grafana-data/src/themes/registry.ts b/packages/grafana-data/src/themes/registry.ts index f05ea638735..2bf6e07aff1 100644 --- a/packages/grafana-data/src/themes/registry.ts +++ b/packages/grafana-data/src/themes/registry.ts @@ -119,5 +119,8 @@ function createDebug(): GrafanaTheme2 { shape: { borderRadius: 8, }, + spacing: { + gridSize: 10, + }, }); } diff --git a/packages/grafana-ui/src/components/Forms/Checkbox.tsx b/packages/grafana-ui/src/components/Forms/Checkbox.tsx index f5250844fff..3e5e2693f80 100644 --- a/packages/grafana-ui/src/components/Forms/Checkbox.tsx +++ b/packages/grafana-ui/src/components/Forms/Checkbox.tsx @@ -113,10 +113,10 @@ export const getCheckboxStyles = (theme: GrafanaTheme2, invalid = false) => { content: '""', position: 'absolute', zIndex: 2, - left: '4px', + left: theme.spacing(0.5), top: 0, - width: '6px', - height: '12px', + width: theme.spacing(0.75), + height: theme.spacing(1.5), border: `solid ${theme.colors.primary.contrastText}`, borderWidth: '0 3px 3px 0', transform: 'rotate(45deg)', diff --git a/packages/grafana-ui/src/components/Menu/MenuItem.tsx b/packages/grafana-ui/src/components/Menu/MenuItem.tsx index 5bdfb00a682..31e46fd5580 100644 --- a/packages/grafana-ui/src/components/Menu/MenuItem.tsx +++ b/packages/grafana-ui/src/components/Menu/MenuItem.tsx @@ -221,6 +221,7 @@ const getStyles = (theme: GrafanaTheme2) => { display: 'flex', flexDirection: 'column', alignItems: 'stretch', + justifyContent: 'center', padding: theme.spacing(0.5, 2), minHeight: theme.spacing(4), margin: 0, diff --git a/packages/grafana-ui/src/components/Select/SelectContainer.tsx b/packages/grafana-ui/src/components/Select/SelectContainer.tsx index cdac2582193..2d1eb47820c 100644 --- a/packages/grafana-ui/src/components/Select/SelectContainer.tsx +++ b/packages/grafana-ui/src/components/Select/SelectContainer.tsx @@ -50,7 +50,7 @@ const getSelectContainerStyles = (theme: GrafanaTheme2, focused: boolean, disabl flexWrap: 'wrap', alignItems: 'stretch', justifyContent: 'space-between', - minHeight: '32px', + minHeight: theme.spacing(theme.components.height.md), height: 'auto', maxWidth: '100%', diff --git a/packages/grafana-ui/src/components/Switch/Switch.tsx b/packages/grafana-ui/src/components/Switch/Switch.tsx index 351daf346a7..00ed1edc7bd 100644 --- a/packages/grafana-ui/src/components/Switch/Switch.tsx +++ b/packages/grafana-ui/src/components/Switch/Switch.tsx @@ -80,8 +80,8 @@ InlineSwitch.displayName = 'Switch'; const getSwitchStyles = (theme: GrafanaTheme2, transparent?: boolean) => ({ switch: css({ - width: '32px', - height: '16px', + width: theme.spacing(4), + height: theme.spacing(2), position: 'relative', lineHeight: 1, @@ -101,7 +101,7 @@ const getSwitchStyles = (theme: GrafanaTheme2, transparent?: boolean) => ({ }, svg: { - transform: 'translate3d(17px, -50%, 0)', + transform: `translate3d(${theme.spacing(2.25)}, -50%, 0)`, background: theme.colors.primary.contrastText, color: theme.colors.primary.main, }, @@ -147,13 +147,14 @@ const getSwitchStyles = (theme: GrafanaTheme2, transparent?: boolean) => ({ position: 'absolute', display: 'block', color: 'transparent', - width: '12px', - height: '12px', + width: theme.spacing(1.5), + height: theme.spacing(1.5), borderRadius: theme.shape.radius.circle, background: theme.colors.text.secondary, boxShadow: theme.shadows.z1, + left: 0, top: '50%', - transform: 'translate3d(1px, -50%, 0)', + transform: `translate3d(${theme.spacing(0.25)}, -50%, 0)`, transition: 'transform 0.2s cubic-bezier(0.19, 1, 0.22, 1)', '@media (forced-colors: active)': { diff --git a/packages/grafana-ui/src/components/TabbedContainer/TabbedContainer.tsx b/packages/grafana-ui/src/components/TabbedContainer/TabbedContainer.tsx index 07fb0ad039d..4bc11a81e5c 100644 --- a/packages/grafana-ui/src/components/TabbedContainer/TabbedContainer.tsx +++ b/packages/grafana-ui/src/components/TabbedContainer/TabbedContainer.tsx @@ -34,7 +34,7 @@ export function TabbedContainer({ tabs, defaultTab, closeIconTooltip, onClose, t setActiveTab(item.value!); }; - const autoHeight = `calc(100% - (${theme.components.menuTabs.height}px + ${theme.spacing(1)}))`; + const autoHeight = `calc(100% - (${theme.spacing(theme.components.menuTabs.height)} + ${theme.spacing(1)}))`; return (
diff --git a/packages/grafana-ui/src/components/Tabs/TabsBar.tsx b/packages/grafana-ui/src/components/Tabs/TabsBar.tsx index 99d2e8c3b18..d5d43c51cbd 100644 --- a/packages/grafana-ui/src/components/Tabs/TabsBar.tsx +++ b/packages/grafana-ui/src/components/Tabs/TabsBar.tsx @@ -36,8 +36,8 @@ const getStyles = (theme: GrafanaTheme2) => ({ tabs: css({ position: 'relative', display: 'flex', - height: `${theme.components.menuTabs.height}px`, - alignItems: 'center', + height: theme.spacing(theme.components.menuTabs.height), + alignItems: 'stretch', }), }); diff --git a/public/app/core/components/Page/EditableTitle.tsx b/public/app/core/components/Page/EditableTitle.tsx index 79c46f368e9..4e333288ec9 100644 --- a/public/app/core/components/Page/EditableTitle.tsx +++ b/public/app/core/components/Page/EditableTitle.tsx @@ -118,6 +118,7 @@ const getStyles = (theme: GrafanaTheme2) => { alignItems: 'center', display: 'flex', gap: theme.spacing(1), + height: theme.spacing(theme.components.height.md), }), }; }; From 3c2c5610bdfde1a912cd9badc2a3b912115128ad Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Wed, 11 Dec 2024 14:31:58 +0100 Subject: [PATCH 10/14] Alerting: Fix updating condition when refId changes (#97753) fix updating condition when refid changes --- .../query-and-alert-condition/QueryAndExpressionsStep.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx index a54aac610c8..9406c7fe509 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx @@ -269,10 +269,10 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P // update condition too if refId was updated if (condition === oldRefId) { - handleSetCondition(newRefId); + setValue('condition', newRefId); } }, - [condition, queries, handleSetCondition] + [condition, queries, setValue] ); const updateExpressionAndDatasource = useSetExpressionAndDataSource(); From 9c396b74f934434ab9ccd01dde58f438e1e7039c Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Wed, 11 Dec 2024 15:00:11 +0100 Subject: [PATCH 11/14] Alerting: AlertingQueryRunner should skip descendant nodes of invalid queries (#97528) --- .../as-admin-user/alerting.spec.ts | 13 ----- .../components/rule-editor/dag.test.ts | 15 +++++ .../unified/components/rule-editor/dag.ts | 31 ++++++++-- .../unified/state/AlertingQueryRunner.test.ts | 19 +++++-- .../unified/state/AlertingQueryRunner.ts | 57 +++++++++++-------- 5 files changed, 91 insertions(+), 44 deletions(-) diff --git a/e2e/plugin-e2e/plugin-e2e-api-tests/as-admin-user/alerting.spec.ts b/e2e/plugin-e2e/plugin-e2e-api-tests/as-admin-user/alerting.spec.ts index 531cb85e091..ac18e6458da 100644 --- a/e2e/plugin-e2e/plugin-e2e-api-tests/as-admin-user/alerting.spec.ts +++ b/e2e/plugin-e2e/plugin-e2e-api-tests/as-admin-user/alerting.spec.ts @@ -1,19 +1,6 @@ import * as e2e from '@grafana/e2e-selectors'; import { expect, test } from '@grafana/plugin-e2e'; -test('should evaluate to false if entire request returns 500', async ({ page, alertRuleEditPage, selectors }) => { - await alertRuleEditPage.alertRuleNameField.fill('Test Alert Rule'); - - // remove the default query - const queryA = alertRuleEditPage.getAlertRuleQueryRow('A'); - await alertRuleEditPage - .getByGrafanaSelector(selectors.components.QueryEditorRow.actionButton('Remove query'), { - root: queryA.locator, - }) - .click(); - await expect(alertRuleEditPage.evaluate()).not.toBeOK(); -}); - test('should evaluate to false if entire request returns 200 but partial query result is invalid', async ({ page, alertRuleEditPage, diff --git a/public/app/features/alerting/unified/components/rule-editor/dag.test.ts b/public/app/features/alerting/unified/components/rule-editor/dag.test.ts index 7da5c42871f..eecc88977ff 100644 --- a/public/app/features/alerting/unified/components/rule-editor/dag.test.ts +++ b/public/app/features/alerting/unified/components/rule-editor/dag.test.ts @@ -3,6 +3,7 @@ import { AlertQuery } from 'app/types/unified-alerting-dto'; import { _createDagFromQueries, + _getDescendants, _getOriginsOfRefId, fingerPrintQueries, fingerprintGraph, @@ -84,6 +85,20 @@ describe('getOriginsOfRefId', () => { }); }); +describe('getDescendants', () => { + test('with multiple generations', () => { + const graph = new Graph(); + graph.createNodes(['A', 'B', 'C', 'D', 'E', 'F', 'G']); + graph.link('A', 'B'); + graph.link('B', 'G'); + graph.link('A', 'C'); + graph.link('C', 'D'); + graph.link('E', 'F'); + + expect(_getDescendants('A', graph)).toEqual(['B', 'G', 'C', 'D']); + }); +}); + describe('parseRefsFromMathExpression', () => { const cases: Array<[string, string[]]> = [ ['$A', ['A']], diff --git a/public/app/features/alerting/unified/components/rule-editor/dag.ts b/public/app/features/alerting/unified/components/rule-editor/dag.ts index 2971dd66a1e..cd4583fe6e2 100644 --- a/public/app/features/alerting/unified/components/rule-editor/dag.ts +++ b/public/app/features/alerting/unified/components/rule-editor/dag.ts @@ -59,6 +59,7 @@ export function parseRefsFromMathExpression(input: string): string[] { } export const getOriginOfRefId = memoize(_getOriginsOfRefId, (refId, graph) => refId + fingerprintGraph(graph)); +export const getDescendants = memoize(_getDescendants, (refId, graph) => refId + fingerprintGraph(graph)); export function _getOriginsOfRefId(refId: string, graph: Graph): string[] { const node = graph.getNode(refId); @@ -66,23 +67,45 @@ export function _getOriginsOfRefId(refId: string, graph: Graph): string[] { const origins: Node[] = []; // recurse through "node > inputEdges > inputNode" - function findChildNode(node: Node) { + function findParentNode(node: Node) { const inputEdges = node.inputEdges; if (inputEdges.length > 0) { inputEdges.forEach((edge) => { if (edge.inputNode) { - findChildNode(edge.inputNode); + findParentNode(edge.inputNode); } }); } else { - origins?.push(node); + origins.push(node); } } + findParentNode(node); + + return origins.map((origin) => origin.name); +} + +// get all children (and children's children etc) from a given node +export function _getDescendants(refId: string, graph: Graph): string[] { + const node = graph.getNode(refId); + const descendants: Node[] = []; + + // recurse through "node > outputEdges > outputNode" + function findChildNode(node: Node) { + const outputEdges = node.outputEdges; + + outputEdges.forEach((edge) => { + if (edge.outputNode) { + descendants.push(edge.outputNode); + findChildNode(edge.outputNode); + } + }); + } + findChildNode(node); - return origins.map((origin) => origin.name); + return descendants.map((descendant) => descendant.name); } // create a unique fingerprint of the DAG diff --git a/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts b/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts index a4dc6e5f576..e9935f7bc91 100644 --- a/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts +++ b/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts @@ -216,10 +216,10 @@ describe('AlertingQueryRunner', () => { }); }); - it('should skip hidden queries', async () => { + it('should skip hidden queries and descendant nodes', async () => { const results = createFetchResponse({ results: { - B: { frames: [createDataFrameJSON([1, 2, 3])] }, + C: { frames: [createDataFrameJSON([1, 2, 3])] }, }, }); @@ -239,7 +239,17 @@ describe('AlertingQueryRunner', () => { hide: true, }, }), - createQuery('B'), + createQuery('B', { + model: { + expression: 'A', // depends on A + refId: 'B', + }, + }), + createQuery('C', { + model: { + refId: 'C', + }, + }), ], 'B' ); @@ -248,7 +258,8 @@ describe('AlertingQueryRunner', () => { const [loading, _data] = values; expect(loading.A).toBeUndefined(); - expect(loading.B.state).toEqual(LoadingState.Done); + expect(loading.B).toBeUndefined(); + expect(loading.C.state).toEqual(LoadingState.Done); }); }); }); diff --git a/public/app/features/alerting/unified/state/AlertingQueryRunner.ts b/public/app/features/alerting/unified/state/AlertingQueryRunner.ts index c3f56b9b2d2..fcba1587acc 100644 --- a/public/app/features/alerting/unified/state/AlertingQueryRunner.ts +++ b/public/app/features/alerting/unified/state/AlertingQueryRunner.ts @@ -21,6 +21,7 @@ import { cancelNetworkRequestsOnUnsubscribe } from 'app/features/query/state/pro import { setStructureRevision } from 'app/features/query/state/processing/revision'; import { AlertQuery } from 'app/types/unified-alerting-dto'; +import { createDagFromQueries, getDescendants } from '../components/rule-editor/dag'; import { getTimeRangeForExpression } from '../utils/timeRange'; export interface AlertingQueryResult { @@ -51,29 +52,7 @@ export class AlertingQueryRunner { async run(queries: AlertQuery[], condition: string) { const empty = initialState(queries, LoadingState.Done); - const queriesToExclude: string[] = []; - - // do not execute if one more of the queries are not runnable, - // for example not completely configured - for (const query of queries) { - const refId = query.model.refId; - - if (isExpressionQuery(query.model)) { - continue; - } - - const dataSourceInstance = await this.dataSourceSrv.get(query.datasourceUid); - const skipRunningQuery = - dataSourceInstance instanceof DataSourceWithBackend && - dataSourceInstance.filterQuery && - !dataSourceInstance.filterQuery(query.model); - - if (skipRunningQuery) { - queriesToExclude.push(refId); - } - } - - const queriesToRun = reject(queries, (q) => queriesToExclude.includes(q.model.refId)); + const queriesToRun = await this.prepareQueries(queries); if (queriesToRun.length === 0) { return this.subject.next(empty); @@ -98,6 +77,38 @@ export class AlertingQueryRunner { }); } + // this function will omit any invalid queries and all of its descendants from the list of queries + // to do this we will convert the list of queries into a DAG and walk the invalid node's output edges recursively + async prepareQueries(queries: AlertQuery[]) { + const queriesToExclude: string[] = []; + + // convert our list of queries to a graph + const queriesGraph = createDagFromQueries(queries); + + // find all invalid nodes and omit those and their child nodes from the final queries array + // ⚠️ also make sure all dependent nodes are omitted, otherwise we will be evaluating a broken graph with missing references + for (const query of queries) { + const refId = query.model.refId; + + if (isExpressionQuery(query.model)) { + continue; + } + + const dataSourceInstance = await this.dataSourceSrv.get(query.datasourceUid); + const skipRunningQuery = + dataSourceInstance instanceof DataSourceWithBackend && + dataSourceInstance.filterQuery && + !dataSourceInstance.filterQuery(query.model); + + if (skipRunningQuery) { + const descendants = getDescendants(refId, queriesGraph); + queriesToExclude.push(refId, ...descendants); + } + } + + return reject(queries, (q) => queriesToExclude.includes(q.model.refId)); + } + cancel() { if (!this.subscription) { return; From def3632a3ae4827c4f81c4fc7de950f2fba92c7e Mon Sep 17 00:00:00 2001 From: Bruno Abrantes Date: Wed, 11 Dec 2024 15:17:36 +0100 Subject: [PATCH 12/14] Clarifies documentation around what the annotations.tags_length setting does (#97790) Signed-off-by: Bruno Abrantes --- docs/sources/setup-grafana/configure-grafana/_index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index cbd476fef97..238da2179f5 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -1769,7 +1769,7 @@ Configures the batch size for the annotation clean-up job. This setting is used ### tags_length -Enforces the maximum allowed length of the tags for any newly introduced annotations. It can be between 500 and 4096 (inclusive). Default value is 500. Setting it to a higher value would impact performance therefore is not recommended. +Enforces the maximum allowed amount of tags for any newly introduced annotations. This value can be between 500 and 4096 (inclusive). The default value is 500. Setting it to a higher value would impact performance and is therefore not recommended. ## [annotations.dashboard] From 1a225756133f24a6a4aeb490ee0f2c5ca944841d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Wed, 11 Dec 2024 15:45:20 +0100 Subject: [PATCH 13/14] MetricCombobox: Fix overflowing parents (#97049) --- .../components/MetricCombobox.tsx | 65 +++++++++++++++---- .../components/PromQueryBuilder.tsx | 18 +++-- .../src/components/Forms/InlineField.tsx | 1 + .../src/components/Forms/InlineFieldRow.tsx | 1 + 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/packages/grafana-prometheus/src/querybuilder/components/MetricCombobox.tsx b/packages/grafana-prometheus/src/querybuilder/components/MetricCombobox.tsx index 3ce12337f37..80c2be2c262 100644 --- a/packages/grafana-prometheus/src/querybuilder/components/MetricCombobox.tsx +++ b/packages/grafana-prometheus/src/querybuilder/components/MetricCombobox.tsx @@ -1,10 +1,12 @@ +import { css } from '@emotion/css'; import { useCallback, useState } from 'react'; -import { SelectableValue } from '@grafana/data'; +import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { EditorField, EditorFieldGroup, InputGroup } from '@grafana/experimental'; import { config } from '@grafana/runtime'; -import { Button, InlineField, InlineFieldRow } from '@grafana/ui'; +import { Button, ComponentSize, InlineField, InlineFieldRow, useStyles2 } from '@grafana/ui'; import { Combobox, ComboboxOption } from '@grafana/ui/src/components/Combobox/Combobox'; +import { getPropertiesForButtonSize } from '@grafana/ui/src/components/Forms/commonStyles'; import { PrometheusDatasource } from '../../datasource'; import { regexifyLabelValuesQueryString } from '../parsingUtils'; @@ -25,6 +27,9 @@ export interface MetricComboboxProps { variableEditor?: boolean; } +const INLINE_FIELD_WIDTH = 20; +const BUTTON_SIZE: ComponentSize = 'md'; + export function MetricCombobox({ datasource, query, @@ -86,6 +91,8 @@ export function MetricCombobox({ const metricsExplorerEnabled = config.featureToggles.prometheusMetricEncyclopedia; + const styles = useStyles2(getMectricComboboxStyles); + const asyncSelect = () => { return ( @@ -101,6 +108,7 @@ export function MetricCombobox({ {metricsExplorerEnabled ? (
} - > - {asyncSelect()} - - + + + Optional: returns a list of label values for the label name in the specified metric.} + > + {asyncSelect()} + + + ) : ( - - {asyncSelect()} - + + + {asyncSelect()} + + )} ); @@ -171,3 +183,28 @@ const formatKeyValueStringsForLabelValuesQuery = (query: string, labelsFilters?: return formatPrometheusLabelFiltersToString(queryString, labelsFilters); }; + +const getMectricComboboxStyles = (theme: GrafanaTheme2) => { + const { height } = getPropertiesForButtonSize(BUTTON_SIZE, theme); + const buttonSpace = parseInt(theme.spacing(height), 10); + const inlineFieldSpace = parseInt(theme.spacing(INLINE_FIELD_WIDTH), 10); + const widthToSubstract = inlineFieldSpace + buttonSpace; + return { + adaptToParentQueryEditor: css({ + // Take metrics explorer button into account + maxWidth: `calc(100% - ${buttonSpace}px)`, + }), + adaptToParentVariableEditor: css({ + maxWidth: '100%', + display: 'flex', + '[class*="InlineFieldRow"]': { + '> div': { + 'label + div': { + // Take label and the metrics explorer button into account + maxWidth: `calc(100% - ${widthToSubstract}px)`, + }, + }, + }, + }), + }; +}; diff --git a/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.tsx b/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.tsx index a1d450b2452..8b1b1774426 100644 --- a/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.tsx +++ b/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.tsx @@ -6,7 +6,7 @@ import { DataSourceApi, PanelData } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { EditorRow } from '@grafana/experimental'; import { config } from '@grafana/runtime'; -import { Drawer } from '@grafana/ui'; +import { Drawer, useStyles2 } from '@grafana/ui'; import { PrometheusDatasource } from '../../datasource'; import promqlGrammar from '../../promql'; @@ -58,6 +58,7 @@ export const PromQueryBuilder = memo((props) => { checkLlms(); } }, [prometheusPromQAIL]); + const styles = useStyles2(getPromQueryBuilderStyles); return ( <> @@ -71,9 +72,12 @@ export const PromQueryBuilder = memo((props) => { /> )} - - - + + + + + + {initHints.length ? (
((props) => { ); }); - +const getPromQueryBuilderStyles = () => ({ + addaptToParent: css({ + maxWidth: '100%', + }), +}); PromQueryBuilder.displayName = 'PromQueryBuilder'; diff --git a/packages/grafana-ui/src/components/Forms/InlineField.tsx b/packages/grafana-ui/src/components/Forms/InlineField.tsx index 2eb4b5c1f25..c3c95331563 100644 --- a/packages/grafana-ui/src/components/Forms/InlineField.tsx +++ b/packages/grafana-ui/src/components/Forms/InlineField.tsx @@ -98,6 +98,7 @@ const getStyles = (theme: GrafanaTheme2, grow?: boolean, shrink?: boolean) => { position: 'relative', flex: `${grow ? 1 : 0} ${shrink ? 1 : 0} auto`, margin: `0 ${theme.spacing(0.5)} ${theme.spacing(0.5)} 0`, + maxWidth: '100%', }), childContainer: css({ flex: `${grow ? 1 : 0} ${shrink ? 1 : 0} auto`, diff --git a/packages/grafana-ui/src/components/Forms/InlineFieldRow.tsx b/packages/grafana-ui/src/components/Forms/InlineFieldRow.tsx index f77222a04d3..eab09f2480d 100644 --- a/packages/grafana-ui/src/components/Forms/InlineFieldRow.tsx +++ b/packages/grafana-ui/src/components/Forms/InlineFieldRow.tsx @@ -27,6 +27,7 @@ const getStyles = (theme: GrafanaTheme2) => { flexWrap: 'wrap', alignContent: 'flex-start', rowGap: theme.spacing(0.5), + maxWidth: '100%', }), }; }; From 4e10507c847c74da8b08ff224220e99a066d74f2 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Wed, 11 Dec 2024 14:50:49 +0000 Subject: [PATCH 14/14] Chore: Remove global mock of `plugin_loader` (#97351) * Remove global mock of `plugin_loader` Removing this means we can get more accurate datasources behaviour in tests * Fix circular dependency/undefined method when plugin_loader is unmocked * Use optional chaining for trusted policies to stop tests failing when `bootData` is partially set * Add plugin_loader mock back into single test that is still broken * Revert trusted type policies changes * Fix tests that break with trusted type policies --- .../angular/panel/specs/metrics_panel_ctrl.test.ts | 6 ------ .../app/core/history/RichHistoryRemoteStorage.test.ts | 3 +++ public/app/core/reducers/root.test.ts | 11 ----------- public/app/core/utils/explore.test.ts | 2 +- .../serialization/buildNewDashboardSaveModel.test.ts | 1 + .../explore/hooks/useStateSync/internal.utils.ts | 2 +- .../explore/hooks/useStateSync/migrators/v0.test.ts | 2 +- .../explore/hooks/useStateSync/migrators/v0.ts | 2 +- .../explore/hooks/useStateSync/migrators/v1.test.ts | 2 +- .../explore/hooks/useStateSync/migrators/v1.ts | 2 +- public/app/features/explore/state/constants.ts | 6 ++++++ public/app/features/explore/state/main.ts | 9 +++++---- public/app/features/explore/state/utils.ts | 9 +++------ .../features/panel/state/getAllSuggestions.test.ts | 3 --- .../app/features/plugins/tests/plugin_loader.test.ts | 3 --- public/app/features/teams/TeamPages.test.tsx | 1 - public/test/jest-setup.ts | 1 - 17 files changed, 24 insertions(+), 41 deletions(-) create mode 100644 public/app/features/explore/state/constants.ts diff --git a/public/app/angular/panel/specs/metrics_panel_ctrl.test.ts b/public/app/angular/panel/specs/metrics_panel_ctrl.test.ts index 04f72d46fe4..da8129dde1f 100644 --- a/public/app/angular/panel/specs/metrics_panel_ctrl.test.ts +++ b/public/app/angular/panel/specs/metrics_panel_ctrl.test.ts @@ -2,18 +2,12 @@ jest.mock('app/core/core', () => ({})); jest.mock('app/core/config', () => { return { ...jest.requireActual('app/core/config'), - bootData: { - user: {}, - }, panels: { test: { id: 'test', name: 'test', }, }, - config: { - appSubUrl: 'test', - }, }; }); diff --git a/public/app/core/history/RichHistoryRemoteStorage.test.ts b/public/app/core/history/RichHistoryRemoteStorage.test.ts index 31d5c726834..766712d1c0a 100644 --- a/public/app/core/history/RichHistoryRemoteStorage.test.ts +++ b/public/app/core/history/RichHistoryRemoteStorage.test.ts @@ -44,6 +44,9 @@ jest.mock('../services/PreferencesService', () => ({ }, })); +// FIXME: Tests break unless plugin loader is mocked. This is likely due to a circular dependency +jest.mock('app/features/plugins/plugin_loader', () => ({})); + describe('RichHistoryRemoteStorage', () => { let storage: RichHistoryRemoteStorage; diff --git a/public/app/core/reducers/root.test.ts b/public/app/core/reducers/root.test.ts index 3396fc0d18c..6b584776233 100644 --- a/public/app/core/reducers/root.test.ts +++ b/public/app/core/reducers/root.test.ts @@ -6,17 +6,6 @@ import { cleanUpAction } from '../actions/cleanUp'; import { createRootReducer } from './root'; -jest.mock('@grafana/runtime', () => ({ - ...jest.requireActual('@grafana/runtime'), - config: { - ...jest.requireActual('@grafana/runtime').config, - bootData: { - navTree: [], - user: {}, - }, - }, -})); - describe('rootReducer', () => { const rootReducer = createRootReducer(); diff --git a/public/app/core/utils/explore.test.ts b/public/app/core/utils/explore.test.ts index 75da07f4577..76da92421a9 100644 --- a/public/app/core/utils/explore.test.ts +++ b/public/app/core/utils/explore.test.ts @@ -4,7 +4,7 @@ import { config } from '@grafana/runtime'; import { DataQuery } from '@grafana/schema'; import { RefreshPicker } from '@grafana/ui'; import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; -import { DEFAULT_RANGE } from 'app/features/explore/state/utils'; +import { DEFAULT_RANGE } from 'app/features/explore/state/constants'; import { getVariablesUrlParams } from 'app/features/variables/getAllVariableValuesForUrl'; import { DatasourceSrvMock, MockDataSourceApi } from '../../../test/mocks/datasource_srv'; diff --git a/public/app/features/dashboard-scene/serialization/buildNewDashboardSaveModel.test.ts b/public/app/features/dashboard-scene/serialization/buildNewDashboardSaveModel.test.ts index 520cc777009..bfe7a4b658d 100644 --- a/public/app/features/dashboard-scene/serialization/buildNewDashboardSaveModel.test.ts +++ b/public/app/features/dashboard-scene/serialization/buildNewDashboardSaveModel.test.ts @@ -45,6 +45,7 @@ jest.mock('@grafana/runtime', () => ({ newDashboardWithFiltersAndGroupBy: false, }, bootData: { + ...jest.requireActual('@grafana/runtime').config.bootData, user: { timezone: 'Africa/Abidjan', }, diff --git a/public/app/features/explore/hooks/useStateSync/internal.utils.ts b/public/app/features/explore/hooks/useStateSync/internal.utils.ts index e77445939a8..1a12484d533 100644 --- a/public/app/features/explore/hooks/useStateSync/internal.utils.ts +++ b/public/app/features/explore/hooks/useStateSync/internal.utils.ts @@ -3,10 +3,10 @@ import { isEqual } from 'lodash'; import { CoreApp, DataSourceApi, ExploreUrlState, isTruthy } from '@grafana/data'; import { DataQuery, DataSourceRef } from '@grafana/schema'; import { getLastUsedDatasourceUID } from 'app/core/utils/explore'; -import { DEFAULT_RANGE } from 'app/features/explore/state/utils'; import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource'; +import { DEFAULT_RANGE } from '../../state/constants'; import { isFulfilled } from '../utils'; export type InitState = 'pending' | 'done' | 'notstarted'; diff --git a/public/app/features/explore/hooks/useStateSync/migrators/v0.test.ts b/public/app/features/explore/hooks/useStateSync/migrators/v0.test.ts index bde44b7c2e3..84544301aa3 100644 --- a/public/app/features/explore/hooks/useStateSync/migrators/v0.test.ts +++ b/public/app/features/explore/hooks/useStateSync/migrators/v0.test.ts @@ -1,4 +1,4 @@ -import { DEFAULT_RANGE } from 'app/features/explore/state/utils'; +import { DEFAULT_RANGE } from 'app/features/explore/state/constants'; import { v0Migrator } from './v0'; diff --git a/public/app/features/explore/hooks/useStateSync/migrators/v0.ts b/public/app/features/explore/hooks/useStateSync/migrators/v0.ts index a2948163f00..0fdf4b9f548 100644 --- a/public/app/features/explore/hooks/useStateSync/migrators/v0.ts +++ b/public/app/features/explore/hooks/useStateSync/migrators/v0.ts @@ -1,5 +1,5 @@ import { ExploreUrlState } from '@grafana/data'; -import { DEFAULT_RANGE } from 'app/features/explore/state/utils'; +import { DEFAULT_RANGE } from 'app/features/explore/state/constants'; import { BaseExploreURL, MigrationHandler } from './types'; diff --git a/public/app/features/explore/hooks/useStateSync/migrators/v1.test.ts b/public/app/features/explore/hooks/useStateSync/migrators/v1.test.ts index f436ecc5f00..810d8800d65 100644 --- a/public/app/features/explore/hooks/useStateSync/migrators/v1.test.ts +++ b/public/app/features/explore/hooks/useStateSync/migrators/v1.test.ts @@ -1,4 +1,4 @@ -import { DEFAULT_RANGE } from 'app/features/explore/state/utils'; +import { DEFAULT_RANGE } from 'app/features/explore/state/constants'; import { v1Migrator } from './v1'; diff --git a/public/app/features/explore/hooks/useStateSync/migrators/v1.ts b/public/app/features/explore/hooks/useStateSync/migrators/v1.ts index 57b23ce4076..58fe595e49a 100644 --- a/public/app/features/explore/hooks/useStateSync/migrators/v1.ts +++ b/public/app/features/explore/hooks/useStateSync/migrators/v1.ts @@ -1,6 +1,6 @@ import { ExploreUrlState } from '@grafana/data'; import { ID_ALPHABET, generateExploreId } from 'app/core/utils/explore'; -import { DEFAULT_RANGE } from 'app/features/explore/state/utils'; +import { DEFAULT_RANGE } from 'app/features/explore/state/constants'; import { hasKey } from '../../utils'; diff --git a/public/app/features/explore/state/constants.ts b/public/app/features/explore/state/constants.ts new file mode 100644 index 00000000000..a4c20f15e0b --- /dev/null +++ b/public/app/features/explore/state/constants.ts @@ -0,0 +1,6 @@ +import { config } from '@grafana/runtime'; + +export const DEFAULT_RANGE = { + from: `now-${config.exploreDefaultTimeOffset}`, + to: 'now', +}; diff --git a/public/app/features/explore/state/main.ts b/public/app/features/explore/state/main.ts index 3e1375434c9..ed3e789cecc 100644 --- a/public/app/features/explore/state/main.ts +++ b/public/app/features/explore/state/main.ts @@ -14,8 +14,9 @@ import { RichHistorySearchFilters, RichHistorySettings } from '../../../core/uti import { createAsyncThunk, ThunkResult } from '../../../types'; import { withUniqueRefIds } from '../utils/queries'; +import { DEFAULT_RANGE } from './constants'; import { initializeExplore, InitializeExploreOptions, paneReducer } from './explorePane'; -import { DEFAULT_RANGE, makeExplorePaneState } from './utils'; +import { makeExplorePaneState } from './utils'; // // Actions and Payloads @@ -157,7 +158,7 @@ export const navigateToExplore = ( /** * Global Explore state that handles multiple Explore areas and the split state */ -const initialExploreItemState = makeExplorePaneState(); +const initialExploreItemState = () => makeExplorePaneState(); export const initialExploreState: ExploreState = { syncedTimes: false, panes: {}, @@ -265,7 +266,7 @@ export const exploreReducer = (state = initialExploreState, action: AnyAction): ...state, panes: { ...state.panes, - [action.meta.arg.exploreId]: initialExploreItemState, + [action.meta.arg.exploreId]: initialExploreItemState(), }, }; } @@ -274,7 +275,7 @@ export const exploreReducer = (state = initialExploreState, action: AnyAction): const initialPanes = Object.entries(state.panes); const before = initialPanes.slice(0, action.meta.arg.position); const after = initialPanes.slice(before.length); - const panes = [...before, [action.meta.arg.exploreId, initialExploreItemState] as const, ...after].reduce( + const panes = [...before, [action.meta.arg.exploreId, initialExploreItemState()] as const, ...after].reduce( (acc, [id, pane]) => ({ ...acc, [id]: pane }), {} ); diff --git a/public/app/features/explore/state/utils.ts b/public/app/features/explore/state/utils.ts index 8b2c6888c3f..45b8b39e0e4 100644 --- a/public/app/features/explore/state/utils.ts +++ b/public/app/features/explore/state/utils.ts @@ -20,7 +20,7 @@ import { URLRange, URLRangeValue, } from '@grafana/data'; -import { config, getDataSourceSrv } from '@grafana/runtime'; +import { getDataSourceSrv } from '@grafana/runtime'; import { DataQuery, DataSourceJsonData, DataSourceRef, TimeZone } from '@grafana/schema'; import { getLocalRichHistoryStorage } from 'app/core/history/richHistoryStorageProvider'; import { SortOrder } from 'app/core/utils/richHistoryTypes'; @@ -33,12 +33,9 @@ import { setLastUsedDatasourceUID } from '../../../core/utils/explore'; import { getDatasourceSrv } from '../../plugins/datasource_srv'; import { loadSupplementaryQueries } from '../utils/supplementaryQueries'; -export const MAX_HISTORY_AUTOCOMPLETE_ITEMS = 100; +import { DEFAULT_RANGE } from './constants'; -export const DEFAULT_RANGE = { - from: `now-${config.exploreDefaultTimeOffset}`, - to: 'now', -}; +export const MAX_HISTORY_AUTOCOMPLETE_ITEMS = 100; const GRAPH_STYLE_KEY = 'grafana.explore.style.graph'; export const storeGraphStyle = (graphStyle: string): void => { diff --git a/public/app/features/panel/state/getAllSuggestions.test.ts b/public/app/features/panel/state/getAllSuggestions.test.ts index 17b236ed1ec..552c80a74b5 100644 --- a/public/app/features/panel/state/getAllSuggestions.test.ts +++ b/public/app/features/panel/state/getAllSuggestions.test.ts @@ -13,9 +13,6 @@ import { SuggestionName } from 'app/types/suggestions'; import { getAllSuggestions, panelsToCheckFirst } from './getAllSuggestions'; -jest.unmock('app/core/core'); -jest.unmock('app/features/plugins/plugin_loader'); - for (const pluginId of panelsToCheckFirst) { config.panels[pluginId] = { module: `core:plugin/${pluginId}`, diff --git a/public/app/features/plugins/tests/plugin_loader.test.ts b/public/app/features/plugins/tests/plugin_loader.test.ts index d41caa1dbe1..9f6c2e3739b 100644 --- a/public/app/features/plugins/tests/plugin_loader.test.ts +++ b/public/app/features/plugins/tests/plugin_loader.test.ts @@ -1,6 +1,3 @@ -// Use the real plugin_loader (stubbed by default) -jest.unmock('app/features/plugins/plugin_loader'); - jest.mock('app/core/core', () => { return { coreModule: { diff --git a/public/app/features/teams/TeamPages.test.tsx b/public/app/features/teams/TeamPages.test.tsx index f3a83495df5..f2d5c4674a9 100644 --- a/public/app/features/teams/TeamPages.test.tsx +++ b/public/app/features/teams/TeamPages.test.tsx @@ -30,7 +30,6 @@ jest.mock('@grafana/runtime', () => ({ licenseUrl: '', }, featureToggles: { accesscontrol: true }, - bootData: { navTree: [], user: {} }, buildInfo: { edition: 'Open Source', version: '7.5.0', diff --git a/public/test/jest-setup.ts b/public/test/jest-setup.ts index f625756bc8b..9511a3e6a97 100644 --- a/public/test/jest-setup.ts +++ b/public/test/jest-setup.ts @@ -78,7 +78,6 @@ jest.mock('../app/core/core', () => ({ appEvents: testAppEvents, })); jest.mock('../app/angular/partials', () => ({})); -jest.mock('../app/features/plugins/plugin_loader', () => ({})); const throwUnhandledRejections = () => { process.on('unhandledRejection', (err) => {