From d62c490af56154cf8a7e9b6a097d74ec97913d94 Mon Sep 17 00:00:00 2001 From: "Arati R." <33031346+suntala@users.noreply.github.com> Date: Fri, 7 Feb 2025 09:29:25 +0100 Subject: [PATCH 01/11] UniStore Big Objects: Fix spec rebuilding (#100183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix big object spec rebuilding and associated test --------- Co-authored-by: Jean-Philippe Quémémer --- pkg/registry/apis/dashboard/large.go | 11 ++++++++++- pkg/registry/apis/dashboard/large_test.go | 10 ++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/registry/apis/dashboard/large.go b/pkg/registry/apis/dashboard/large.go index 9530b247d2f..e54b4d252cb 100644 --- a/pkg/registry/apis/dashboard/large.go +++ b/pkg/registry/apis/dashboard/large.go @@ -51,7 +51,16 @@ func NewDashboardLargeObjectSupport(scheme *runtime.Scheme) *apistore.BasicLarge if err != nil { return err } - return json.Unmarshal(blob, &dash.Spec) + + if err := json.Unmarshal(blob, &dash.Spec); err != nil { + return fmt.Errorf("failed to unmarshal blob into spec: %w", err) + } + + if err := scheme.Convert(dash, obj, nil); err != nil { + return fmt.Errorf("failed to update original object: %w", err) + } + + return nil }, } } diff --git a/pkg/registry/apis/dashboard/large_test.go b/pkg/registry/apis/dashboard/large_test.go index 45510304a65..c5d65fa8f5f 100644 --- a/pkg/registry/apis/dashboard/large_test.go +++ b/pkg/registry/apis/dashboard/large_test.go @@ -59,11 +59,17 @@ func TestLargeDashboardSupport(t *testing.T) { }`, string(small)) // Now make it big again - err = largeObject.RebuildSpec(dash, f) + rehydratedDash := &dashboardv0alpha1.Dashboard{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + } + err = largeObject.RebuildSpec(rehydratedDash, f) require.NoError(t, err) // check that all panels exist again - panels, found, err = unstructured.NestedSlice(dash.Spec.Object, "panels") + panels, found, err = unstructured.NestedSlice(rehydratedDash.Spec.Object, "panels") require.NoError(t, err) require.True(t, found) require.Len(t, panels, expectedPanelCount) From d196b789e2edd6a5a893363ad9e19c91e14b0710 Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Fri, 7 Feb 2025 04:44:17 -0500 Subject: [PATCH 02/11] SQL Expressions: Add more SQLNodes and funcs to allow list (#100227) sql_expr: Add more Nodes and funcs to allow list --- pkg/expr/sql/parser_allow.go | 17 ++++++++++++++++- pkg/expr/sql/parser_allow_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/pkg/expr/sql/parser_allow.go b/pkg/expr/sql/parser_allow.go index 8a2ad436920..54072049253 100644 --- a/pkg/expr/sql/parser_allow.go +++ b/pkg/expr/sql/parser_allow.go @@ -54,7 +54,13 @@ func allowedNode(node sqlparser.SQLNode) (b bool) { case *sqlparser.AliasedExpr, *sqlparser.AliasedTableExpr: return - case *sqlparser.BinaryExpr: + case *sqlparser.AndExpr, *sqlparser.OrExpr: + return + + case *sqlparser.BinaryExpr, *sqlparser.UnaryExpr: + return + + case sqlparser.BoolVal: return case sqlparser.ColIdent, *sqlparser.ColName, sqlparser.Columns: @@ -87,6 +93,9 @@ func allowedNode(node sqlparser.SQLNode) (b bool) { case *sqlparser.Select, sqlparser.SelectExprs: return + case *sqlparser.SetOp: + return + case *sqlparser.StarExpr: return @@ -102,6 +111,9 @@ func allowedNode(node sqlparser.SQLNode) (b bool) { case *sqlparser.Over: return + case *sqlparser.ParenExpr: + return + case *sqlparser.Subquery: return @@ -124,6 +136,9 @@ func allowedFunction(f *sqlparser.FuncExpr) (b bool) { b = true // so don't have to return true in every case but default switch strings.ToLower(f.Name.String()) { + case "if": + return + case "sum", "avg", "count", "min", "max": return diff --git a/pkg/expr/sql/parser_allow_test.go b/pkg/expr/sql/parser_allow_test.go index aeeb1a40c69..98dd3277173 100644 --- a/pkg/expr/sql/parser_allow_test.go +++ b/pkg/expr/sql/parser_allow_test.go @@ -17,6 +17,11 @@ func TestAllowQuery(t *testing.T) { q: example_metrics_query, err: nil, }, + { + name: "an example from todd", + q: example_argo_commit_example, + err: nil, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -78,3 +83,29 @@ SELECT * FROM usage_by_team CROSS JOIN total_metrics CROSS JOIN total_traces` + +var example_argo_commit_example = `WITH +gh AS + (SELECT Count(*) AS commits + FROM + (SELECT * + FROM oss_repo + UNION ALL SELECT * + FROM ent_repo) AS ent_repos), +argo_success AS + (SELECT IF(argo.status = 'Succeeded', argo.value, 0) AS value FROM argo), +argo_failure AS + (SELECT IF(argo.status = 'Failed', argo.value, 0) AS value FROM argo) +SELECT IF(env.value > 1, TRUE, workflows.runs < 1 OR gh.commits < 1) AS status, + gh.commits AS 'merged commits to main (OSS + enterprise)', + drone.value AS 'enterprise downstream publish', + workflows.runs AS 'github trigger instant workflow runs today', + argo_success.value AS 'argo success', + argo_failure.value AS 'argo failure', + (env.value - 1) AS 'new dev instant deployments' +FROM drone, + env, + gh, + argo_success, + argo_failure, + workflows;` From 9da423045ec58f9e187bc6675e4a2d366feb6105 Mon Sep 17 00:00:00 2001 From: Levente Balogh Date: Fri, 7 Feb 2025 10:49:27 +0100 Subject: [PATCH 03/11] Advisor: Enable frontend code generation for the `checktype` kind (#100208) nit: enabling frontend code generation for the `checktype` --- apps/advisor/kinds/checktype.cue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/advisor/kinds/checktype.cue b/apps/advisor/kinds/checktype.cue index 6a4bfa4a0df..aa31430ad2e 100644 --- a/apps/advisor/kinds/checktype.cue +++ b/apps/advisor/kinds/checktype.cue @@ -7,7 +7,7 @@ checktype: { versions: { "v0alpha1": { codegen: { - frontend: false + frontend: true backend: true } schema: { From 6dc98dbbcc05eb67e18b0a9c398a3f8eaa20b2e0 Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Fri, 7 Feb 2025 04:51:55 -0500 Subject: [PATCH 04/11] SQL Expressions: Add str_to_date function and unskip test (#100226) --- pkg/expr/sql/db_test.go | 14 ++++---------- pkg/expr/sql/parser_allow.go | 3 +++ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/pkg/expr/sql/db_test.go b/pkg/expr/sql/db_test.go index 258507121c2..e648407e41a 100644 --- a/pkg/expr/sql/db_test.go +++ b/pkg/expr/sql/db_test.go @@ -148,27 +148,21 @@ func TestQueryFramesNumericSelect(t *testing.T) { } func TestQueryFramesDateTimeSelect(t *testing.T) { - t.Skip("need a fix in go-mysql-server, and then handle the datetime strings (or figure out why strings and not time.Time)") expectedFrame := &data.Frame{ RefID: "a", Name: "a", Fields: []*data.Field{ - data.NewField("ts", nil, []time.Time{}), + data.NewField("ts", nil, []*time.Time{ + p(time.Date(2025, 2, 3, 3, 0, 0, 0, time.UTC)), + }), }, } db := DB{} - // It doesn't like the T in the time string qry := `SELECT str_to_date('2025-02-03T03:00:00','%Y-%m-%dT%H:%i:%s') as ts` - // This comes back as a string, which needs to be dealt with? - //qry := `SELECT str_to_date('2025-02-03-03:00:00','%Y-%m-%d-%H:%i:%s') as ts` - - // This is a datetime(6), need to deal with that as well - //qry := `SELECT current_timestamp() as ts` - - f, err := db.QueryFrames(context.Background(), "b", qry, []*data.Frame{}) + f, err := db.QueryFrames(context.Background(), "a", qry, nil) require.NoError(t, err) if diff := cmp.Diff(expectedFrame, f, data.FrameTestCompareOptions()...); diff != "" { diff --git a/pkg/expr/sql/parser_allow.go b/pkg/expr/sql/parser_allow.go index 54072049253..3b5d553af08 100644 --- a/pkg/expr/sql/parser_allow.go +++ b/pkg/expr/sql/parser_allow.go @@ -145,6 +145,9 @@ func allowedFunction(f *sqlparser.FuncExpr) (b bool) { case "coalesce": return + case "str_to_date": + return + default: return false } From e291140be3ea25ba862aedb73c68440d2eac8ad3 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Fri, 7 Feb 2025 10:57:26 +0100 Subject: [PATCH 05/11] Advisor: Run check steps in parallel (#100200) --- .../pkg/app/checks/datasourcecheck/check.go | 106 +++++++-------- .../app/checks/datasourcecheck/check_test.go | 30 +++-- apps/advisor/pkg/app/checks/ifaces.go | 4 +- .../pkg/app/checks/plugincheck/check.go | 124 +++++++++--------- .../pkg/app/checks/plugincheck/check_test.go | 10 +- apps/advisor/pkg/app/checks/utils.go | 4 +- apps/advisor/pkg/app/utils.go | 52 ++++++-- apps/advisor/pkg/app/utils_test.go | 50 ++++++- 8 files changed, 225 insertions(+), 155 deletions(-) diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check.go b/apps/advisor/pkg/app/checks/datasourcecheck/check.go index 88e10a72719..e946f2e2819 100644 --- a/apps/advisor/pkg/app/checks/datasourcecheck/check.go +++ b/apps/advisor/pkg/app/checks/datasourcecheck/check.go @@ -12,7 +12,6 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/util" - "k8s.io/klog/v2" ) type check struct { @@ -76,26 +75,23 @@ func (s *uidValidationStep) Description() string { return "Check if the UID of each data source is valid." } -func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) { - dsErrs := []advisor.CheckReportError{} - for _, i := range items { - ds, ok := i.(*datasources.DataSource) - if !ok { - return nil, fmt.Errorf("invalid item type %T", i) - } - // Data source UID validation - err := util.ValidateUID(ds.UID) - if err != nil { - dsErrs = append(dsErrs, checks.NewCheckReportError( - advisor.CheckReportErrorSeverityLow, - fmt.Sprintf("Invalid UID '%s' for data source %s", ds.UID, ds.Name), - "Check the documentation for more information.", - s.ID(), - ds.UID, - )) - } +func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) { + ds, ok := i.(*datasources.DataSource) + if !ok { + return nil, fmt.Errorf("invalid item type %T", i) } - return dsErrs, nil + // Data source UID validation + err := util.ValidateUID(ds.UID) + if err != nil { + return checks.NewCheckReportError( + advisor.CheckReportErrorSeverityLow, + fmt.Sprintf("Invalid UID '%s' for data source %s", ds.UID, ds.Name), + "Check the documentation for more information.", + s.ID(), + ds.UID, + ), nil + } + return nil, nil } type healthCheckStep struct { @@ -115,46 +111,38 @@ func (s *healthCheckStep) ID() string { return "health-check" } -func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) { - dsErrs := []advisor.CheckReportError{} - for _, i := range items { - ds, ok := i.(*datasources.DataSource) - if !ok { - return nil, fmt.Errorf("invalid item type %T", i) - } - - // Health check execution - requester, err := identity.GetRequester(ctx) - if err != nil { - return nil, err - } - pCtx, err := s.PluginContextProvider.GetWithDataSource(ctx, ds.Type, requester, ds) - if err != nil { - klog.ErrorS(err, "Error creating plugin context", "datasource", ds.Name) - continue - } - req := &backend.CheckHealthRequest{ - PluginContext: pCtx, - Headers: map[string]string{}, - } - resp, err := s.PluginClient.CheckHealth(ctx, req) - if err != nil { - fmt.Println("Error checking health", err) - continue - } - if resp.Status != backend.HealthStatusOk { - dsErrs = append(dsErrs, checks.NewCheckReportError( - advisor.CheckReportErrorSeverityHigh, - fmt.Sprintf("Health check failed for %s", ds.Name), - fmt.Sprintf( - "Go to the data source configuration"+ - " and address the issues reported.", ds.UID), - s.ID(), - ds.UID, - )) - } +func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) { + ds, ok := i.(*datasources.DataSource) + if !ok { + return nil, fmt.Errorf("invalid item type %T", i) } - return dsErrs, nil + + // Health check execution + requester, err := identity.GetRequester(ctx) + if err != nil { + return nil, err + } + pCtx, err := s.PluginContextProvider.GetWithDataSource(ctx, ds.Type, requester, ds) + if err != nil { + return nil, fmt.Errorf("failed to get plugin context: %w", err) + } + req := &backend.CheckHealthRequest{ + PluginContext: pCtx, + Headers: map[string]string{}, + } + resp, err := s.PluginClient.CheckHealth(ctx, req) + if err != nil || resp.Status != backend.HealthStatusOk { + return checks.NewCheckReportError( + advisor.CheckReportErrorSeverityHigh, + fmt.Sprintf("Health check failed for %s", ds.Name), + fmt.Sprintf( + "Go to the data source configuration"+ + " and address the issues reported.", ds.UID), + s.ID(), + ds.UID, + ), nil + } + return nil, nil } type pluginContextProvider interface { diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go index c360f96ad14..5cfe595de67 100644 --- a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go +++ b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go @@ -35,9 +35,13 @@ func TestCheck_Run(t *testing.T) { assert.NoError(t, err) errs := []advisor.CheckReportError{} for _, step := range check.Steps() { - stepErrs, err := step.Run(ctx, &advisor.CheckSpec{}, items) - assert.NoError(t, err) - errs = append(errs, stepErrs...) + for _, item := range items { + stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item) + assert.NoError(t, err) + if stepErr != nil { + errs = append(errs, *stepErr) + } + } } assert.NoError(t, err) @@ -65,9 +69,13 @@ func TestCheck_Run(t *testing.T) { assert.NoError(t, err) errs := []advisor.CheckReportError{} for _, step := range check.Steps() { - stepErrs, err := step.Run(ctx, &advisor.CheckSpec{}, items) - assert.NoError(t, err) - errs = append(errs, stepErrs...) + for _, item := range items { + stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item) + assert.NoError(t, err) + if stepErr != nil { + errs = append(errs, *stepErr) + } + } } assert.NoError(t, err) @@ -96,9 +104,13 @@ func TestCheck_Run(t *testing.T) { assert.NoError(t, err) errs := []advisor.CheckReportError{} for _, step := range check.Steps() { - stepErrs, err := step.Run(ctx, &advisor.CheckSpec{}, items) - assert.NoError(t, err) - errs = append(errs, stepErrs...) + for _, item := range items { + stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item) + assert.NoError(t, err) + if stepErr != nil { + errs = append(errs, *stepErr) + } + } } assert.NoError(t, err) diff --git a/apps/advisor/pkg/app/checks/ifaces.go b/apps/advisor/pkg/app/checks/ifaces.go index b630a84871c..915d986a4a2 100644 --- a/apps/advisor/pkg/app/checks/ifaces.go +++ b/apps/advisor/pkg/app/checks/ifaces.go @@ -24,6 +24,6 @@ type Step interface { Title() string // Description returns the description of the step Description() string - // Run executes the step and returns a list of errors - Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, items []any) ([]advisorv0alpha1.CheckReportError, error) + // Run executes the step for an item and returns a report + Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, item any) (*advisorv0alpha1.CheckReportError, error) } diff --git a/apps/advisor/pkg/app/checks/plugincheck/check.go b/apps/advisor/pkg/app/checks/plugincheck/check.go index c024b2608b0..f9dc853d0b8 100644 --- a/apps/advisor/pkg/app/checks/plugincheck/check.go +++ b/apps/advisor/pkg/app/checks/plugincheck/check.go @@ -78,35 +78,33 @@ func (s *deprecationStep) ID() string { return "deprecation" } -func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) { - errs := []advisor.CheckReportError{} - for _, i := range items { - p, ok := i.(pluginstore.Plugin) - if !ok { - return nil, fmt.Errorf("invalid item type %T", i) - } - - // Skip if it's a core plugin - if p.IsCorePlugin() { - continue - } - - // Check if plugin is deprecated - i, err := s.PluginRepo.PluginInfo(ctx, p.ID) - if err != nil { - continue - } - if i.Status == "deprecated" { - errs = append(errs, checks.NewCheckReportError( - advisor.CheckReportErrorSeverityHigh, - fmt.Sprintf("Plugin deprecated: %s", p.ID), - "Check the documentation for recommended steps.", - s.ID(), - p.ID, - )) - } +func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, it any) (*advisor.CheckReportError, error) { + p, ok := it.(pluginstore.Plugin) + if !ok { + return nil, fmt.Errorf("invalid item type %T", it) } - return errs, nil + + // Skip if it's a core plugin + if p.IsCorePlugin() { + return nil, nil + } + + // Check if plugin is deprecated + i, err := s.PluginRepo.PluginInfo(ctx, p.ID) + if err != nil { + // Unable to check deprecation status + return nil, nil + } + if i.Status == "deprecated" { + return checks.NewCheckReportError( + advisor.CheckReportErrorSeverityHigh, + fmt.Sprintf("Plugin deprecated: %s", p.ID), + "Check the documentation for recommended steps.", + s.ID(), + p.ID, + ), nil + } + return nil, nil } type updateStep struct { @@ -127,44 +125,42 @@ func (s *updateStep) ID() string { return "update" } -func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) { - errs := []advisor.CheckReportError{} - for _, i := range items { - p, ok := i.(pluginstore.Plugin) - if !ok { - return nil, fmt.Errorf("invalid item type %T", i) - } - - // Skip if it's a core plugin - if p.IsCorePlugin() { - continue - } - - // Skip if it's managed or pinned - if s.isManaged(ctx, p.ID) || s.PluginPreinstall.IsPinned(p.ID) { - continue - } - - // Check if plugin has a newer version available - compatOpts := repo.NewCompatOpts(services.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH) - info, err := s.PluginRepo.GetPluginArchiveInfo(ctx, p.ID, "", compatOpts) - if err != nil { - continue - } - if hasUpdate(p, info) { - errs = append(errs, checks.NewCheckReportError( - advisor.CheckReportErrorSeverityLow, - fmt.Sprintf("New version available for %s", p.ID), - fmt.Sprintf( - "Go to the plugin admin page"+ - " and upgrade to the latest version.", p.ID), - s.ID(), - p.ID, - )) - } +func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) { + p, ok := i.(pluginstore.Plugin) + if !ok { + return nil, fmt.Errorf("invalid item type %T", i) } - return errs, nil + // Skip if it's a core plugin + if p.IsCorePlugin() { + return nil, nil + } + + // Skip if it's managed or pinned + if s.isManaged(ctx, p.ID) || s.PluginPreinstall.IsPinned(p.ID) { + return nil, nil + } + + // Check if plugin has a newer version available + compatOpts := repo.NewCompatOpts(services.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH) + info, err := s.PluginRepo.GetPluginArchiveInfo(ctx, p.ID, "", compatOpts) + if err != nil { + // Unable to check updates + return nil, nil + } + if hasUpdate(p, info) { + return checks.NewCheckReportError( + advisor.CheckReportErrorSeverityLow, + fmt.Sprintf("New version available for %s", p.ID), + fmt.Sprintf( + "Go to the plugin admin page"+ + " and upgrade to the latest version.", p.ID), + s.ID(), + p.ID, + ), nil + } + + return nil, nil } func hasUpdate(current pluginstore.Plugin, latest *repo.PluginArchiveInfo) bool { diff --git a/apps/advisor/pkg/app/checks/plugincheck/check_test.go b/apps/advisor/pkg/app/checks/plugincheck/check_test.go index 29dc7ccaabd..ff5190278e2 100644 --- a/apps/advisor/pkg/app/checks/plugincheck/check_test.go +++ b/apps/advisor/pkg/app/checks/plugincheck/check_test.go @@ -136,9 +136,13 @@ func TestRun(t *testing.T) { assert.NoError(t, err) errs := []advisor.CheckReportError{} for _, step := range check.Steps() { - stepErrs, err := step.Run(context.Background(), &advisor.CheckSpec{}, items) - assert.NoError(t, err) - errs = append(errs, stepErrs...) + for _, item := range items { + stepErr, err := step.Run(context.Background(), &advisor.CheckSpec{}, item) + assert.NoError(t, err) + if stepErr != nil { + errs = append(errs, *stepErr) + } + } } assert.NoError(t, err) assert.Equal(t, len(tt.plugins), len(items)) diff --git a/apps/advisor/pkg/app/checks/utils.go b/apps/advisor/pkg/app/checks/utils.go index a6c4089da44..053dc591389 100644 --- a/apps/advisor/pkg/app/checks/utils.go +++ b/apps/advisor/pkg/app/checks/utils.go @@ -10,8 +10,8 @@ func NewCheckReportError( action string, stepID string, itemID string, -) advisor.CheckReportError { - return advisor.CheckReportError{ +) *advisor.CheckReportError { + return &advisor.CheckReportError{ Severity: severity, Reason: reason, Action: action, diff --git a/apps/advisor/pkg/app/utils.go b/apps/advisor/pkg/app/utils.go index c59dbaca21e..b1809dd2ee3 100644 --- a/apps/advisor/pkg/app/utils.go +++ b/apps/advisor/pkg/app/utils.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sync" claims "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/resource" @@ -83,20 +84,17 @@ func processCheck(ctx context.Context, client resource.Client, obj resource.Obje } // Run the steps steps := check.Steps() - errs := []advisorv0alpha1.CheckReportError{} - for _, step := range steps { - stepErrs, err := step.Run(ctx, &c.Spec, items) - if err != nil { - setErr := setStatusAnnotation(ctx, client, obj, "error") - if setErr != nil { - return setErr - } - return fmt.Errorf("error running step %s: %w", step.Title(), err) + reportErrors, err := runStepsInParallel(ctx, &c.Spec, steps, items) + if err != nil { + setErr := setStatusAnnotation(ctx, client, obj, "error") + if setErr != nil { + return setErr } - errs = append(errs, stepErrs...) + return fmt.Errorf("error running steps: %w", err) } + report := &advisorv0alpha1.CheckV0alpha1StatusReport{ - Errors: errs, + Errors: reportErrors, Count: int64(len(items)), } err = setStatusAnnotation(ctx, client, obj, "processed") @@ -111,3 +109,35 @@ func processCheck(ctx context.Context, client resource.Client, obj resource.Obje }}, }, resource.PatchOptions{}, obj) } + +func runStepsInParallel(ctx context.Context, spec *advisorv0alpha1.CheckSpec, steps []checks.Step, items []any) ([]advisorv0alpha1.CheckReportError, error) { + reportErrs := []advisorv0alpha1.CheckReportError{} + var internalErr error + var wg sync.WaitGroup + var mu sync.Mutex + // Avoid too many concurrent requests + limit := make(chan struct{}, 10) + + for _, step := range steps { + for _, item := range items { + wg.Add(1) + limit <- struct{}{} + go func(step checks.Step, item any) { + defer wg.Done() + defer func() { <-limit }() + stepErr, err := step.Run(ctx, spec, item) + mu.Lock() + defer mu.Unlock() + if err != nil { + internalErr = fmt.Errorf("error running step %s: %w", step.ID(), err) + return + } + if stepErr != nil { + reportErrs = append(reportErrs, *stepErr) + } + }(step, item) + } + } + wg.Wait() + return reportErrs, internalErr +} diff --git a/apps/advisor/pkg/app/utils_test.go b/apps/advisor/pkg/app/utils_test.go index a13880f8209..5905e2a9c44 100644 --- a/apps/advisor/pkg/app/utils_test.go +++ b/apps/advisor/pkg/app/utils_test.go @@ -3,6 +3,7 @@ package app import ( "context" "errors" + "fmt" "testing" "github.com/grafana/grafana-app-sdk/resource" @@ -68,13 +69,45 @@ func TestProcessCheck(t *testing.T) { meta.SetCreatedBy("user:1") client := &mockClient{} ctx := context.TODO() - check := &mockCheck{} + check := &mockCheck{ + items: []any{"item"}, + } err = processCheck(ctx, client, obj, check) assert.NoError(t, err) assert.Equal(t, "processed", obj.GetAnnotations()[statusAnnotation]) } +func TestProcessMultipleCheckItems(t *testing.T) { + obj := &advisorv0alpha1.Check{} + obj.SetAnnotations(map[string]string{}) + meta, err := utils.MetaAccessor(obj) + if err != nil { + t.Fatal(err) + } + meta.SetCreatedBy("user:1") + client := &mockClient{} + ctx := context.TODO() + items := make([]any, 100) + for i := range items { + if i%2 == 0 { + items[i] = fmt.Sprintf("item-%d", i) + } else { + items[i] = errors.New("error") + } + } + check := &mockCheck{ + items: items, + } + + err = processCheck(ctx, client, obj, check) + assert.NoError(t, err) + assert.Equal(t, "processed", obj.GetAnnotations()[statusAnnotation]) + r := client.lastValue.(advisorv0alpha1.CheckV0alpha1StatusReport) + assert.Equal(t, r.Count, int64(100)) + assert.Len(t, r.Errors, 50) +} + func TestProcessCheck_AlreadyProcessed(t *testing.T) { obj := &advisorv0alpha1.Check{} obj.SetAnnotations(map[string]string{statusAnnotation: "processed"}) @@ -98,7 +131,8 @@ func TestProcessCheck_RunError(t *testing.T) { ctx := context.TODO() check := &mockCheck{ - err: errors.New("run error"), + items: []any{"item"}, + err: errors.New("run error"), } err = processCheck(ctx, client, obj, check) @@ -108,14 +142,17 @@ func TestProcessCheck_RunError(t *testing.T) { type mockClient struct { resource.Client + lastValue any } func (m *mockClient) PatchInto(ctx context.Context, id resource.Identifier, req resource.PatchRequest, opts resource.PatchOptions, obj resource.Object) error { + m.lastValue = req.Operations[0].Value return nil } type mockCheck struct { - err error + err error + items []any } func (m *mockCheck) ID() string { @@ -123,7 +160,7 @@ func (m *mockCheck) ID() string { } func (m *mockCheck) Items(ctx context.Context) ([]any, error) { - return []any{}, nil + return m.items, nil } func (m *mockCheck) Steps() []checks.Step { @@ -136,10 +173,13 @@ type mockStep struct { err error } -func (m *mockStep) Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, items []any) ([]advisorv0alpha1.CheckReportError, error) { +func (m *mockStep) Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, items any) (*advisorv0alpha1.CheckReportError, error) { if m.err != nil { return nil, m.err } + if _, ok := items.(error); ok { + return &advisorv0alpha1.CheckReportError{}, nil + } return nil, nil } From ccb9cab1318ea76ebb6b1005e7a53e113cba0d3b Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Fri, 7 Feb 2025 11:07:08 +0100 Subject: [PATCH 06/11] Plugins: Add synchronous CDN plugin loader (#99096) * WIP * Run plugin validations and validation steps sequentially if feature is off * Remove dependency between sources.Service and pluginscdn.Service * lint * Parallelize validation only if class is CDN * re-generate feature toggles * remove waitgroup usage * PR review: Add loader concurrency limit setting * re-generate feature toggles * pr review feedback * fix const name * Skip module.js validation for cdn plugins * do not run validation steps in parallel * lint * reduce diff * re-generate feature toggles * lint * pr review feedback * remove leftover config.PluginManagementCfg from sources.Service --- .../feature-toggles/index.md | 1 + .../src/types/featureToggles.gen.ts | 1 + pkg/plugins/config/config.go | 7 +-- pkg/plugins/manager/loader/loader.go | 54 ++++++++++++++++--- pkg/plugins/manager/loader/loader_test.go | 10 +++- .../manager/pipeline/validation/steps.go | 5 ++ 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 +++++ .../pluginsintegration/loader/loader.go | 7 ++- .../pluginsintegration/loader/loader_test.go | 4 +- .../pluginsintegration/pluginconfig/config.go | 7 +-- .../pluginsintegration/pluginsintegration.go | 4 +- .../pluginsintegration/renderer/renderer.go | 2 +- .../pluginsintegration/test_helper.go | 2 +- 16 files changed, 105 insertions(+), 22 deletions(-) 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 13646ef8948..73143bc543b 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -233,6 +233,7 @@ Experimental features might be changed or removed without prior notice. | `elasticsearchImprovedParsing` | Enables less memory intensive Elasticsearch result parsing | | `datasourceConnectionsTab` | Shows defined connections for a data source in the plugins detail page | | `newLogsPanel` | Enables the new logs panel in Explore | +| `pluginsCDNSyncLoader` | Load plugins from CDN synchronously | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 3e0490dd97b..cbe09d02910 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -257,4 +257,5 @@ export interface FeatureToggles { alertingAlertmanagerExtraDedupStageStopPipeline?: boolean; newLogsPanel?: boolean; grafanaconThemes?: boolean; + pluginsCDNSyncLoader?: boolean; } diff --git a/pkg/plugins/config/config.go b/pkg/plugins/config/config.go index a26c66176a1..67b5b9c15ee 100644 --- a/pkg/plugins/config/config.go +++ b/pkg/plugins/config/config.go @@ -31,9 +31,10 @@ type PluginManagementCfg struct { // Features contains the feature toggles used for the plugin management system. type Features struct { - ExternalCorePluginsEnabled bool - SkipHostEnvVarsEnabled bool - SriChecksEnabled bool + ExternalCorePluginsEnabled bool + SkipHostEnvVarsEnabled bool + SriChecksEnabled bool + PluginsCDNSyncLoaderEnabled bool } // NewPluginManagementCfg returns a new PluginManagementCfg. diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go index 2abc88ef735..f3c0d47e73b 100644 --- a/pkg/plugins/manager/loader/loader.go +++ b/pkg/plugins/manager/loader/loader.go @@ -8,6 +8,7 @@ import ( "time" "github.com/grafana/grafana/pkg/plugins" + pluginsCfg "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery" @@ -17,7 +18,10 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs" ) +const concurrencyLimit = 32 + type Loader struct { + cfg *pluginsCfg.PluginManagementCfg discovery discovery.Discoverer bootstrap bootstrap.Bootstrapper initializer initialization.Initializer @@ -27,9 +31,13 @@ type Loader struct { log log.Logger } -func New(discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, - initializer initialization.Initializer, termination termination.Terminator, errorTracker pluginerrs.ErrorTracker) *Loader { +func New( + cfg *pluginsCfg.PluginManagementCfg, + discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, + initializer initialization.Initializer, termination termination.Terminator, errorTracker pluginerrs.ErrorTracker, +) *Loader { return &Loader{ + cfg: cfg, discovery: discovery, bootstrap: bootstrap, validation: validation, @@ -55,11 +63,14 @@ func (l *Loader) recordError(ctx context.Context, p *plugins.Plugin, err error) func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { end := l.instrumentLoad(ctx, src) + st := time.Now() discoveredPlugins, err := l.discovery.Discover(ctx, src) if err != nil { return nil, err } + l.log.Debug("Discovered", "class", src.PluginClass(ctx), "duration", time.Since(st)) + st = time.Now() bootstrappedPlugins := []*plugins.Plugin{} for _, foundBundle := range discoveredPlugins { bootstrappedPlugin, err := l.bootstrap.Bootstrap(ctx, src, foundBundle) @@ -72,17 +83,47 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins } bootstrappedPlugins = append(bootstrappedPlugins, bootstrappedPlugin...) } + l.log.Debug("Bootstrapped", "class", src.PluginClass(ctx), "duration", time.Since(st)) + st = time.Now() validatedPlugins := []*plugins.Plugin{} + type validateResult struct { + bootstrappedPlugin *plugins.Plugin + err error + } + validateResults := make(chan validateResult, len(bootstrappedPlugins)) + + // If the PluginsCDNSyncLoaderEnabled feature is enabled, validate plugins in parallel. + // Otherwise, validate plugins sequentially. + var limitSize int + if l.cfg.Features.PluginsCDNSyncLoaderEnabled && src.PluginClass(ctx) == plugins.ClassCDN { + limitSize = min(len(bootstrappedPlugins), concurrencyLimit) + } else { + limitSize = 1 + } + limit := make(chan struct{}, limitSize) for _, bootstrappedPlugin := range bootstrappedPlugins { - err := l.validation.Validate(ctx, bootstrappedPlugin) - if err != nil { - l.recordError(ctx, bootstrappedPlugin, err) + limit <- struct{}{} + go func(p *plugins.Plugin) { + err := l.validation.Validate(ctx, p) + validateResults <- validateResult{ + bootstrappedPlugin: bootstrappedPlugin, + err: err, + } + <-limit + }(bootstrappedPlugin) + } + for i := 0; i < len(bootstrappedPlugins); i++ { + r := <-validateResults + if r.err != nil { + l.recordError(ctx, r.bootstrappedPlugin, r.err) continue } - validatedPlugins = append(validatedPlugins, bootstrappedPlugin) + validatedPlugins = append(validatedPlugins, r.bootstrappedPlugin) } + l.log.Debug("Validated", "class", src.PluginClass(ctx), "duration", time.Since(st), "total", len(validatedPlugins)) + st = time.Now() initializedPlugins := []*plugins.Plugin{} for _, validatedPlugin := range validatedPlugins { initializedPlugin, err := l.initializer.Initialize(ctx, validatedPlugin) @@ -92,6 +133,7 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins } initializedPlugins = append(initializedPlugins, initializedPlugin) } + l.log.Debug("Initialized", "class", src.PluginClass(ctx), "duration", time.Since(st)) // Clean errors from registry for initialized plugins for _, p := range initializedPlugins { diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index 3472727f127..d39c9c21303 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -58,6 +58,7 @@ func TestLoader_Load(t *testing.T) { t.Errorf("could not construct absolute path of current dir") return } + zeroCfg := &config.PluginManagementCfg{} tests := []struct { name string class plugins.Class @@ -420,7 +421,7 @@ func TestLoader_Load(t *testing.T) { require.NoError(t, err) et := pluginerrs.ProvideErrorTracker() - l := New(discovery.New(tt.cfg, discovery.Opts{}), bootstrap.New(tt.cfg, bootstrap.Opts{}), + l := New(zeroCfg, discovery.New(tt.cfg, discovery.Opts{}), bootstrap.New(tt.cfg, bootstrap.Opts{}), validation.New(tt.cfg, validation.Opts{}), initialization.New(tt.cfg, initialization.Opts{}), terminationStage, et) @@ -455,6 +456,7 @@ func TestLoader_Load(t *testing.T) { var steps []string l := New( + zeroCfg, &fakes.FakeDiscoverer{ DiscoverFunc: func(ctx context.Context, s plugins.PluginSource) ([]*plugins.FoundBundle, error) { require.Equal(t, src, s) @@ -512,6 +514,7 @@ func TestLoader_Load(t *testing.T) { var steps []string l := New( + zeroCfg, &fakes.FakeDiscoverer{ DiscoverFunc: func(ctx context.Context, s plugins.PluginSource) ([]*plugins.FoundBundle, error) { require.Equal(t, src, s) @@ -574,6 +577,7 @@ func TestLoader_Load(t *testing.T) { var steps []string l := New( + zeroCfg, &fakes.FakeDiscoverer{ DiscoverFunc: func(ctx context.Context, s plugins.PluginSource) ([]*plugins.FoundBundle, error) { require.Equal(t, src, s) @@ -629,7 +633,9 @@ func TestLoader_Unload(t *testing.T) { } for _, tc := range tcs { - l := New(&fakes.FakeDiscoverer{}, + l := New( + &config.PluginManagementCfg{}, + &fakes.FakeDiscoverer{}, &fakes.FakeBootstrapper{}, &fakes.FakeValidator{}, &fakes.FakeInitializer{}, diff --git a/pkg/plugins/manager/pipeline/validation/steps.go b/pkg/plugins/manager/pipeline/validation/steps.go index 10af0cce5e4..1b41c1b5d6d 100644 --- a/pkg/plugins/manager/pipeline/validation/steps.go +++ b/pkg/plugins/manager/pipeline/validation/steps.go @@ -57,6 +57,11 @@ func newModuleJSValidator() *ModuleJSValidator { } func (v *ModuleJSValidator) Validate(_ context.Context, p *plugins.Plugin) error { + // CDN plugins are ignored because the module.js is guaranteed to exist + if p.Class == plugins.ClassCDN { + return nil + } + if !p.IsRenderer() && !p.IsCorePlugin() { f, err := p.FS.Open("module.js") if err != nil { diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index ac2eaffc940..81dda206467 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1794,6 +1794,12 @@ var ( HideFromDocs: true, RequiresRestart: true, }, + { + Name: "pluginsCDNSyncLoader", + Description: "Load plugins from CDN synchronously", + Stage: FeatureStageExperimental, + Owner: grafanaPluginsPlatformSquad, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index e88c9c3d176..d21b4c4fded 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -238,3 +238,4 @@ alertingAlertmanagerExtraDedupStage,experimental,@grafana/alerting-squad,false,t alertingAlertmanagerExtraDedupStageStopPipeline,experimental,@grafana/alerting-squad,false,true,false newLogsPanel,experimental,@grafana/observability-logs,false,false,true grafanaconThemes,experimental,@grafana/grafana-frontend-platform,false,true,false +pluginsCDNSyncLoader,experimental,@grafana/plugins-platform-backend,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index e910ed15857..bedb497d4f5 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -962,4 +962,8 @@ const ( // FlagGrafanaconThemes // Enables the temporary themes for GrafanaCon FlagGrafanaconThemes = "grafanaconThemes" + + // FlagPluginsCDNSyncLoader + // Load plugins from CDN synchronously + FlagPluginsCDNSyncLoader = "pluginsCDNSyncLoader" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 607d0581270..db65ff4fdb8 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -3034,6 +3034,18 @@ "frontend": true } }, + { + "metadata": { + "name": "pluginsCDNSyncLoader", + "resourceVersion": "1737026684018", + "creationTimestamp": "2025-01-16T11:24:44Z" + }, + "spec": { + "description": "Load plugins from CDN synchronously", + "stage": "experimental", + "codeowner": "@grafana/plugins-platform-backend" + } + }, { "metadata": { "name": "pluginsDetailsRightPanel", diff --git a/pkg/services/pluginsintegration/loader/loader.go b/pkg/services/pluginsintegration/loader/loader.go index 4852e84d1b4..b3a03885752 100644 --- a/pkg/services/pluginsintegration/loader/loader.go +++ b/pkg/services/pluginsintegration/loader/loader.go @@ -4,6 +4,7 @@ import ( "context" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/config" pluginsLoader "github.com/grafana/grafana/pkg/plugins/manager/loader" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery" @@ -19,11 +20,13 @@ type Loader struct { loader *pluginsLoader.Loader } -func ProvideService(discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, +func ProvideService( + cfg *config.PluginManagementCfg, + discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, initializer initialization.Initializer, termination termination.Terminator, errorTracker pluginerrs.ErrorTracker, ) *Loader { return &Loader{ - loader: pluginsLoader.New(discovery, bootstrap, validation, initializer, termination, errorTracker), + loader: pluginsLoader.New(cfg, discovery, bootstrap, validation, initializer, termination, errorTracker), } } diff --git a/pkg/services/pluginsintegration/loader/loader_test.go b/pkg/services/pluginsintegration/loader/loader_test.go index 06466240b9e..ca6fef68b50 100644 --- a/pkg/services/pluginsintegration/loader/loader_test.go +++ b/pkg/services/pluginsintegration/loader/loader_test.go @@ -1575,7 +1575,7 @@ func newLoader(t *testing.T, cfg *config.PluginManagementCfg, reg registry.Servi terminate, err := pipeline.ProvideTerminationStage(cfg, reg, proc) require.NoError(t, err) - return ProvideService(pipeline.ProvideDiscoveryStage(cfg, + return ProvideService(cfg, pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), @@ -1607,7 +1607,7 @@ func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loade backendFactoryProvider = fakes.NewFakeBackendProcessProvider() } - return ProvideService(pipeline.ProvideDiscoveryStage(cfg, + return ProvideService(cfg, pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), diff --git a/pkg/services/pluginsintegration/pluginconfig/config.go b/pkg/services/pluginsintegration/pluginconfig/config.go index 71952c64eb8..47751b07711 100644 --- a/pkg/services/pluginsintegration/pluginconfig/config.go +++ b/pkg/services/pluginsintegration/pluginconfig/config.go @@ -30,9 +30,10 @@ func ProvidePluginManagementConfig(cfg *setting.Cfg, settingProvider setting.Pro cfg.PluginsCDNURLTemplate, cfg.AppURL, config.Features{ - ExternalCorePluginsEnabled: features.IsEnabledGlobally(featuremgmt.FlagExternalCorePlugins), - SkipHostEnvVarsEnabled: features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars), - SriChecksEnabled: features.IsEnabledGlobally(featuremgmt.FlagPluginsSriChecks), + ExternalCorePluginsEnabled: features.IsEnabledGlobally(featuremgmt.FlagExternalCorePlugins), + SkipHostEnvVarsEnabled: features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars), + SriChecksEnabled: features.IsEnabledGlobally(featuremgmt.FlagPluginsSriChecks), + PluginsCDNSyncLoaderEnabled: features.IsEnabledGlobally(featuremgmt.FlagPluginsCDNSyncLoader), }, cfg.AngularSupportEnabled, cfg.GrafanaComAPIURL, diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index 22929782cf8..eca6c3be53f 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -107,8 +107,6 @@ var WireSet = wire.NewSet( wire.Bind(new(repo.Service), new(*repo.Manager)), licensing.ProvideLicensing, wire.Bind(new(plugins.Licensing), new(*licensing.Service)), - wire.Bind(new(sources.Registry), new(*sources.Service)), - sources.ProvideService, pluginSettings.ProvideService, wire.Bind(new(pluginsettings.Service), new(*pluginSettings.Service)), filestore.ProvideService, @@ -146,6 +144,8 @@ var WireExtensionSet = wire.NewSet( wire.Bind(new(plugins.Client), new(*backend.MiddlewareHandler)), managedplugins.NewNoop, wire.Bind(new(managedplugins.Manager), new(*managedplugins.Noop)), + sources.ProvideService, + wire.Bind(new(sources.Registry), new(*sources.Service)), ) func ProvideClientWithMiddlewares( diff --git a/pkg/services/pluginsintegration/renderer/renderer.go b/pkg/services/pluginsintegration/renderer/renderer.go index ff8003b957c..5623b5812f8 100644 --- a/pkg/services/pluginsintegration/renderer/renderer.go +++ b/pkg/services/pluginsintegration/renderer/renderer.go @@ -141,5 +141,5 @@ func createLoader(cfg *config.PluginManagementCfg, pluginEnvProvider envvars.Pro et := pluginerrs.ProvideErrorTracker() - return loader.New(d, b, v, i, t, et), nil + return loader.New(cfg, d, b, v, i, t, et), nil } diff --git a/pkg/services/pluginsintegration/test_helper.go b/pkg/services/pluginsintegration/test_helper.go index d74e18a4bcf..92ce0dab164 100644 --- a/pkg/services/pluginsintegration/test_helper.go +++ b/pkg/services/pluginsintegration/test_helper.go @@ -110,5 +110,5 @@ func CreateTestLoader(t *testing.T, cfg *pluginsCfg.PluginManagementCfg, opts Lo require.NoError(t, err) } - return loader.New(opts.Discoverer, opts.Bootstrapper, opts.Validator, opts.Initializer, opts.Terminator, pluginerrs.ProvideErrorTracker()) + return loader.New(cfg, opts.Discoverer, opts.Bootstrapper, opts.Validator, opts.Initializer, opts.Terminator, pluginerrs.ProvideErrorTracker()) } From 4b9fee61a8eacdcd535ead80a0cdaba6b1a0e5a2 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Fri, 7 Feb 2025 11:09:51 +0100 Subject: [PATCH 07/11] QueryLibrary: Move to enterprise (#100133) --- .betterer.results | 21 -- public/app/AppWrapper.tsx | 8 +- .../PanelDataPane/PanelDataQueriesTab.tsx | 4 +- .../app/features/explore/ExploreToolbar.tsx | 6 +- .../QueriesDrawer/QueriesDrawerDropdown.tsx | 6 +- .../QueryLibrary/AddToQueryLibraryModal.tsx | 36 --- .../explore/QueryLibrary/QueryLibrary.tsx | 36 --- .../QueryLibraryAnalyticsEvents.ts | 56 ----- .../QueryLibrary/QueryLibraryContext.test.tsx | 79 ------- .../QueryLibrary/QueryLibraryContext.tsx | 106 +++------ .../QueryLibrary/QueryLibraryDrawer.tsx | 53 ----- .../QueryLibrary/QueryLibraryExpmInfo.tsx | 23 -- .../QueryLibrary/QueryTemplateForm.tsx | 180 -------------- .../QueryLibrary/QueryTemplatesList.test.tsx | 139 ----------- .../QueryLibrary/QueryTemplatesList.tsx | 222 ------------------ .../QueryTemplatesTable/ActionsCell.tsx | 110 --------- .../QueryTemplatesTable/AddedByCell.tsx | 20 -- .../DatasourceTypeCell.tsx | 13 - .../QueryTemplatesTable/DateAddedCell.tsx | 13 - .../QueryDescriptionCell.tsx | 55 ----- .../QueryTemplatesTable/index.tsx | 81 ------- .../QueryTemplatesTable/styles.tsx | 47 ---- .../QueryLibrary/QueryTemplatesTable/types.ts | 15 -- .../explore/QueryLibrary/SaveQueryButton.tsx | 44 ---- .../QueryLibrary/utils/dataFetching.ts | 106 --------- .../explore/QueryLibrary/utils/search.ts | 24 -- .../QueryLibrary/utils/useDatasource.tsx | 9 - .../RichHistory/RichHistoryAddToLibrary.tsx | 35 +-- .../features/explore/spec/helper/setup.tsx | 17 +- .../explore/spec/queryLibrary.test.tsx | 172 -------------- public/app/features/query-library/index.ts | 6 - .../query/components/QueryEditorRow.tsx | 14 +- public/app/routes/RoutesWrapper.tsx | 14 +- public/locales/en-US/grafana.json | 40 ---- public/locales/pseudo-LOCALE/grafana.json | 40 ---- 35 files changed, 79 insertions(+), 1771 deletions(-) delete mode 100644 public/app/features/explore/QueryLibrary/AddToQueryLibraryModal.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryLibrary.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryLibraryAnalyticsEvents.ts delete mode 100644 public/app/features/explore/QueryLibrary/QueryLibraryContext.test.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryLibraryDrawer.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryLibraryExpmInfo.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplateForm.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesList.test.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/ActionsCell.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/DatasourceTypeCell.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/DateAddedCell.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/QueryDescriptionCell.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/index.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/styles.tsx delete mode 100644 public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts delete mode 100644 public/app/features/explore/QueryLibrary/SaveQueryButton.tsx delete mode 100644 public/app/features/explore/QueryLibrary/utils/dataFetching.ts delete mode 100644 public/app/features/explore/QueryLibrary/utils/search.ts delete mode 100644 public/app/features/explore/QueryLibrary/utils/useDatasource.tsx delete mode 100644 public/app/features/explore/spec/queryLibrary.test.tsx diff --git a/.betterer.results b/.betterer.results index c9ce73693f2..e33c3ac4abd 100644 --- a/.betterer.results +++ b/.betterer.results @@ -4701,27 +4701,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/QueryLibrary/QueryLibraryExpmInfo.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"] - ], - "public/app/features/explore/QueryLibrary/QueryTemplateForm.tsx:5381": [ - [0, 0, 0, "\'@grafana/ui/src/components/Input/Input\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] - ], - "public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "4"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "5"] - ], - "public/app/features/explore/QueryLibrary/QueryTemplatesTable/QueryDescriptionCell.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] - ], "public/app/features/explore/RichHistory/RichHistoryStarredTab.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], diff --git a/public/app/AppWrapper.tsx b/public/app/AppWrapper.tsx index 04c8fb80615..227eaeedeb2 100644 --- a/public/app/AppWrapper.tsx +++ b/public/app/AppWrapper.tsx @@ -1,5 +1,5 @@ import { Action, KBarProvider } from 'kbar'; -import { Component, ComponentType, Fragment } from 'react'; +import { Component, ComponentType, Fragment, ReactNode } from 'react'; import CacheProvider from 'react-inlinesvg/provider'; import { Provider } from 'react-redux'; import { Route, Routes } from 'react-router-dom-v5-compat'; @@ -37,6 +37,11 @@ interface AppWrapperState { /** Used by enterprise */ let bodyRenderHooks: ComponentType[] = []; let pageBanners: ComponentType[] = []; +const enterpriseProviders: Array> = []; + +export function addEnterpriseProviders(provider: ComponentType<{ children: ReactNode }>) { + enterpriseProviders.push(provider); +} export function addBodyRenderHook(fn: ComponentType) { bodyRenderHooks.push(fn); @@ -100,6 +105,7 @@ export class AppWrapper extends Component { routes: ready && this.renderRoutes(), pageBanners, bodyRenderHooks, + providers: enterpriseProviders, }; const MaybeTimeRangeProvider = config.featureToggles.timeRangeProvider ? TimeRangeProvider : Fragment; diff --git a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataQueriesTab.tsx b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataQueriesTab.tsx index cef20ef129f..66f83b97bd5 100644 --- a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataQueriesTab.tsx +++ b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/PanelDataQueriesTab.tsx @@ -309,7 +309,7 @@ export class PanelDataQueriesTab extends SceneObjectBase) { const { datasource, dsSettings } = model.useState(); const { data, queries } = model.queryRunner.useState(); - const { openDrawer: openQueryLibraryDrawer } = useQueryLibraryContext(); + const { openDrawer: openQueryLibraryDrawer, queryLibraryEnabled } = useQueryLibraryContext(); if (!datasource || !dsSettings || !data) { return null; @@ -355,7 +355,7 @@ export function PanelDataQueriesTabRendered({ model }: SceneComponentProps Add query - {config.featureToggles.queryLibrary && ( + {queryLibraryEnabled && ( - - - - ); -}; diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesList.test.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesList.test.tsx deleted file mode 100644 index 15ec78719e5..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesList.test.tsx +++ /dev/null @@ -1,139 +0,0 @@ -import { render, waitFor, screen } from '@testing-library/react'; - -import { AnnoKeyCreatedBy } from '../../apiserver/types'; -import { ListQueryTemplateApiResponse } from '../../query-library/api/endpoints.gen'; - -import { QueryTemplatesList } from './QueryTemplatesList'; -import { QueryActionButtonProps } from './types'; - -let data: ListQueryTemplateApiResponse = { - items: [], -}; - -jest.mock('app/features/query-library', () => { - const actual = jest.requireActual('app/features/query-library'); - return { - ...actual, - useDeleteQueryTemplateMutation: () => [() => {}], - useListQueryTemplateQuery: () => { - return { - data: data, - isLoading: false, - error: null, - }; - }, - }; -}); - -jest.mock('./utils/dataFetching', () => { - return { - __esModule: true, - useLoadQueryMetadata: () => { - return { - loading: false, - value: [ - { - index: '0', - uid: '0', - datasourceName: 'prometheus', - datasourceRef: { type: 'prometheus', uid: 'Prometheus0' }, - datasourceType: 'prometheus', - createdAtTimestamp: 0, - query: { refId: 'A' }, - queryText: 'http_requests_total{job="test"}', - description: 'template0', - user: { - uid: 'viewer:JohnDoe', - displayName: 'John Doe', - avatarUrl: '', - }, - error: undefined, - }, - ], - }; - }, - useLoadUsers: () => { - return { - value: { - display: [ - { - avatarUrl: '', - displayName: 'john doe', - identity: { - name: 'JohnDoe', - type: 'viewer', - }, - }, - ], - }, - loading: false, - error: null, - }; - }, - }; -}); - -describe('QueryTemplatesList', () => { - it('renders empty state', async () => { - data = {}; - render(); - await waitFor(() => { - expect(screen.getByText(/You haven't saved any queries to your library yet/)).toBeInTheDocument(); - }); - }); - - it('renders query', async () => { - data.items = testItems; - render(); - await waitFor(() => { - // We don't really show query template title for some reason so creator name - expect(screen.getByText(/John Doe/)).toBeInTheDocument(); - }); - }); - - it('renders actionButton for query', async () => { - data.items = testItems; - let passedProps: QueryActionButtonProps; - - const queryActionButton = (props: QueryActionButtonProps) => { - passedProps = props; - return ; - }; - - render(); - await waitFor(() => { - // We don't really show query template title for some reason so creator name - expect(screen.getByText(/John Doe/)).toBeInTheDocument(); - expect(screen.getByText(/TEST_ACTION_BUTTON/)).toBeInTheDocument(); - // We didn't put much else into the query object but should be enough to check the prop - expect(passedProps.queries).toMatchObject([{ refId: 'A' }]); - }); - }); -}); - -const testItems = [ - { - metadata: { - name: 'TEST_QUERY', - creationTimestamp: '2025-01-01T11:11:11.00Z', - annotations: { - [AnnoKeyCreatedBy]: 'viewer:JohnDoe', - }, - }, - spec: { - title: 'Test Query title', - targets: [ - { - variables: {}, - properties: { - refId: 'A', - datasource: { - uid: 'Prometheus', - type: 'prometheus', - }, - }, - }, - ], - }, - }, -]; diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx deleted file mode 100644 index 946e3de9eea..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesList.tsx +++ /dev/null @@ -1,222 +0,0 @@ -import { css } from '@emotion/css'; -import { uniqBy } from 'lodash'; -import { useEffect, useMemo, useState } from 'react'; - -import { AppEvents, GrafanaTheme2, SelectableValue } from '@grafana/data'; -import { getAppEvents } from '@grafana/runtime'; -import { EmptyState, FilterInput, InlineLabel, MultiSelect, Spinner, useStyles2, Stack, Badge } from '@grafana/ui'; -import { t, Trans } from 'app/core/internationalization'; -import { useListQueryTemplateQuery } from 'app/features/query-library'; -import { QueryTemplate } from 'app/features/query-library/types'; - -import { convertDataQueryResponseToQueryTemplates } from '../../query-library/api/mappers'; - -import { QueryLibraryProps } from './QueryLibrary'; -import { queryLibraryTrackFilterDatasource } from './QueryLibraryAnalyticsEvents'; -import { QueryLibraryExpmInfo } from './QueryLibraryExpmInfo'; -import QueryTemplatesTable from './QueryTemplatesTable'; -import { useLoadQueryMetadata, useLoadUsers } from './utils/dataFetching'; -import { searchQueryLibrary } from './utils/search'; - -interface QueryTemplatesListProps extends QueryLibraryProps {} - -export function QueryTemplatesList(props: QueryTemplatesListProps) { - const { data: rawData, isLoading, error } = useListQueryTemplateQuery({}); - const data = useMemo(() => (rawData ? convertDataQueryResponseToQueryTemplates(rawData) : undefined), [rawData]); - const [isModalOpen, setIsModalOpen] = useState(false); - const [searchQuery, setSearchQuery] = useState(''); - const [datasourceFilters, setDatasourceFilters] = useState>>( - props.activeDatasources?.map((ds) => ({ value: ds, label: ds })) || [] - ); - const [userFilters, setUserFilters] = useState>>([]); - const styles = useStyles2(getStyles); - - const loadUsersResult = useLoadUsersWithError(data); - const userNames = loadUsersResult.data ? loadUsersResult.data.display.map((user) => user.displayName) : []; - - const loadQueryMetadataResult = useLoadQueryMetadataWithError(data, loadUsersResult.data); - - // Filtering right now is done just on the frontend until there is better backend support for this. - const filteredRows = useMemo( - () => - searchQueryLibrary( - loadQueryMetadataResult.value || [], - searchQuery, - datasourceFilters.map((f) => f.value || ''), - userFilters.map((f) => f.value || '') - ), - [loadQueryMetadataResult.value, searchQuery, datasourceFilters, userFilters] - ); - - const datasourceNames = useMemo(() => { - return uniqBy(loadQueryMetadataResult.value, 'datasourceName').map((row) => row.datasourceName); - }, [loadQueryMetadataResult.value]); - - if (error instanceof Error) { - return ( - - {error.message} - - ); - } - - if (isLoading || loadUsersResult.isLoading || loadQueryMetadataResult.loading) { - return ; - } - - if (!data || data.length === 0) { - return ( - -

- { - "You haven't saved any queries to your library yet. Start adding them from Explore or your Query History tab." - } -

- - ); - } - - return ( - <> - setIsModalOpen(false)} /> - - setSearchQuery(query)} - escapeRegex={false} - /> - - Datasource name(s): - - { - setDatasourceFilters(items); - actionMeta.action === 'select-option' && queryLibraryTrackFilterDatasource(); - }} - value={datasourceFilters} - options={datasourceNames.map((r) => { - return { value: r, label: r }; - })} - placeholder={'Filter queries for data sources(s)'} - aria-label={'Filter queries for data sources(s)'} - /> - - User name(s): - - { - setUserFilters(items); - actionMeta.action === 'select-option' && queryLibraryTrackFilterDatasource(); - }} - value={userFilters} - options={userNames.map((r) => { - return { value: r, label: r }; - })} - placeholder={'Filter queries for user name(s)'} - aria-label={'Filter queries for user name(s)'} - /> - setIsModalOpen(true)} - /> - - - - ); -} - -/** - * Wrap useLoadUsers with error handling. - * @param data - */ -function useLoadUsersWithError(data: QueryTemplate[] | undefined) { - const userUIDs = useMemo(() => data?.map((qt) => qt.user?.uid).filter((uid) => uid !== undefined), [data]); - const loadUsersResult = useLoadUsers(userUIDs); - useEffect(() => { - if (loadUsersResult.error) { - getAppEvents().publish({ - type: AppEvents.alertError.name, - payload: [ - t('query-library.user-info-get-error', 'Error attempting to get user info from the library: {{error}}', { - error: JSON.stringify(loadUsersResult.error), - }), - ], - }); - } - }, [loadUsersResult.error]); - return loadUsersResult; -} - -/** - * Wrap useLoadQueryMetadata with error handling. - * @param queryTemplates - * @param userDataList - */ -function useLoadQueryMetadataWithError( - queryTemplates: QueryTemplate[] | undefined, - userDataList: ReturnType['data'] -) { - const result = useLoadQueryMetadata(queryTemplates, userDataList); - - // useLoadQueryMetadata returns errors in the values so we filter and group them and later alert only one time for - // all the errors. This way we show data that is loaded even if some rows errored out. - // TODO: maybe we could show the rows with incomplete data to see exactly which ones errored out. I assume this - // can happen for example when data source for saved query was deleted. Would be nice if user would still be able - // to delete such row or decide what to do. - const [values, errors] = useMemo(() => { - let errors: Error[] = []; - let values = []; - if (!result.loading) { - for (const value of result.value!) { - if (value.error) { - errors.push(value.error); - } else { - values.push(value); - } - } - } - return [values, errors]; - }, [result]); - - useEffect(() => { - if (errors.length) { - getAppEvents().publish({ - type: AppEvents.alertError.name, - payload: [ - t('query-library.query-template-get-error', 'Error attempting to load query template metadata: {{error}}', { - error: JSON.stringify(errors), - }), - ], - }); - } - }, [errors]); - - return { - loading: result.loading, - value: values, - }; -} - -const getStyles = (theme: GrafanaTheme2) => ({ - searchInput: css({ - maxWidth: theme.spacing(55), - }), - multiSelect: css({ - maxWidth: theme.spacing(65), - }), - label: css({ - marginLeft: theme.spacing(1), - border: `1px solid ${theme.colors.secondary.border}`, - }), -}); diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/ActionsCell.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/ActionsCell.tsx deleted file mode 100644 index 575f10bd063..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/ActionsCell.tsx +++ /dev/null @@ -1,110 +0,0 @@ -import { useState } from 'react'; - -import { getAppEvents } from '@grafana/runtime'; -import { IconButton, Modal } from '@grafana/ui'; -import { notifyApp } from 'app/core/actions'; -import { createSuccessNotification } from 'app/core/copy/appNotification'; -import { t } from 'app/core/internationalization'; -import { useDeleteQueryTemplateMutation } from 'app/features/query-library'; -import { dispatch } from 'app/store/store'; -import { ShowConfirmModalEvent } from 'app/types/events'; - -import { - queryLibaryTrackDeleteQuery, - queryLibraryTrackAddOrEditDescription, - queryLibraryTrackRunQuery, -} from '../QueryLibraryAnalyticsEvents'; -import { QueryTemplateForm } from '../QueryTemplateForm'; -import { QueryActionButton } from '../types'; - -import { useQueryLibraryListStyles } from './styles'; -import { QueryTemplateRow } from './types'; - -interface ActionsCellProps { - queryUid?: string; - queryTemplate: QueryTemplateRow; - rootDatasourceUid?: string; - QueryActionButton?: QueryActionButton; -} - -function ActionsCell({ queryTemplate, rootDatasourceUid, queryUid, QueryActionButton }: ActionsCellProps) { - const [deleteQueryTemplate] = useDeleteQueryTemplateMutation(); - const [editFormOpen, setEditFormOpen] = useState(false); - const styles = useQueryLibraryListStyles(); - - const onDeleteQuery = (queryUid: string) => { - const performDelete = (queryUid: string) => { - deleteQueryTemplate({ - name: queryUid, - deleteOptions: {}, - }); - dispatch(notifyApp(createSuccessNotification(t('explore.query-library.query-deleted', 'Query deleted')))); - queryLibaryTrackDeleteQuery(); - }; - - getAppEvents().publish( - new ShowConfirmModalEvent({ - title: t('explore.query-library.delete-query-title', 'Delete query'), - text: t( - 'explore.query-library.delete-query-text', - "You're about to remove this query from the query library. This action cannot be undone. Do you want to continue?" - ), - yesText: t('query-library.delete-query-button', 'Delete query'), - icon: 'trash-alt', - onConfirm: () => performDelete(queryUid), - }) - ); - }; - - return ( -
- { - if (queryUid) { - onDeleteQuery(queryUid); - } - }} - /> - { - setEditFormOpen(true); - queryLibraryTrackAddOrEditDescription(); - }} - /> - {QueryActionButton && ( - { - queryLibraryTrackRunQuery(queryTemplate.datasourceType || ''); - }} - /> - )} - setEditFormOpen(false)} - > - setEditFormOpen(false)} - templateData={queryTemplate} - onSave={() => { - setEditFormOpen(false); - }} - /> - -
- ); -} - -export default ActionsCell; diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx deleted file mode 100644 index 35793ab7fbd..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/AddedByCell.tsx +++ /dev/null @@ -1,20 +0,0 @@ -import { Avatar } from '@grafana/ui'; -import { User } from 'app/features/query-library/types'; - -import { useQueryLibraryListStyles } from './styles'; - -type AddedByCellProps = { - user?: User; -}; -export function AddedByCell(props: AddedByCellProps) { - const styles = useQueryLibraryListStyles(); - - return ( -
- - - - {props.user?.displayName || 'Unknown'} -
- ); -} diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/DatasourceTypeCell.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/DatasourceTypeCell.tsx deleted file mode 100644 index bdc361b7532..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/DatasourceTypeCell.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import { CellProps } from 'react-table'; - -import { useDatasource } from '../utils/useDatasource'; - -import { useQueryLibraryListStyles } from './styles'; -import { QueryTemplateRow } from './types'; - -export function DatasourceTypeCell(props: CellProps) { - const datasourceApi = useDatasource(props.row.original.datasourceRef); - const styles = useQueryLibraryListStyles(); - - return

{datasourceApi?.meta.name}

; -} diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/DateAddedCell.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/DateAddedCell.tsx deleted file mode 100644 index 29bf6a96b07..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/DateAddedCell.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import { CellProps } from 'react-table'; - -import { dateTime } from '@grafana/data'; - -import { useQueryLibraryListStyles } from './styles'; -import { QueryTemplateRow } from './types'; - -export function DateAddedCell(props: CellProps) { - const styles = useQueryLibraryListStyles(); - const formattedTime = dateTime(props.row.original.createdAtTimestamp).format('YYYY-MM-DD HH:mm:ss'); - - return

{formattedTime}

; -} diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/QueryDescriptionCell.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/QueryDescriptionCell.tsx deleted file mode 100644 index f19e95f14c8..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/QueryDescriptionCell.tsx +++ /dev/null @@ -1,55 +0,0 @@ -import { css, cx } from '@emotion/css'; -import { CellProps } from 'react-table'; - -import { GrafanaTheme2 } from '@grafana/data'; -import { Spinner, Tooltip, useStyles2 } from '@grafana/ui'; - -import { useDatasource } from '../utils/useDatasource'; - -import { useQueryLibraryListStyles } from './styles'; -import { QueryTemplateRow } from './types'; - -export function QueryDescriptionCell(props: CellProps) { - const datasourceApi = useDatasource(props.row.original.datasourceRef); - const queryLibraryListStyles = useQueryLibraryListStyles(); - const styles = useStyles2(getStyles); - - if (!datasourceApi) { - return ; - } - - if (!props.row.original.query) { - return
No queries
; - } - const queryDisplayText = props.row.original.queryText; - const description = props.row.original.description; - const dsName = props.row.original.datasourceName; - - return ( -
-

- {datasourceApi?.meta.info.description} - {dsName} -

- -

- {queryDisplayText} -

- -

{description}

-
- ); -} - -const getStyles = (theme: GrafanaTheme2) => ({ - container: css({ - maxWidth: theme.spacing(60), - }), - queryDisplayText: css({ - backgroundColor: theme.colors.background.canvas, - }), -}); diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/index.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/index.tsx deleted file mode 100644 index bd7b94e39ef..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/index.tsx +++ /dev/null @@ -1,81 +0,0 @@ -import { css } from '@emotion/css'; -import { SortByFn } from 'react-table'; - -import { GrafanaTheme2 } from '@grafana/data'; -import { Column, InteractiveTable, useStyles2 } from '@grafana/ui'; - -import { QueryActionButton } from '../types'; - -import ActionsCell from './ActionsCell'; -import { AddedByCell } from './AddedByCell'; -import { DatasourceTypeCell } from './DatasourceTypeCell'; -import { DateAddedCell } from './DateAddedCell'; -import { QueryDescriptionCell } from './QueryDescriptionCell'; -import { QueryTemplateRow } from './types'; - -const timestampSort: SortByFn = (rowA, rowB, _, desc) => { - const timeA = rowA.original.createdAtTimestamp || 0; - const timeB = rowB.original.createdAtTimestamp || 0; - return desc ? timeA - timeB : timeB - timeA; -}; - -function createColumns(queryActionButton?: QueryActionButton): Array> { - return [ - { id: 'description', header: 'Data source and query', cell: QueryDescriptionCell }, - { id: 'addedBy', header: 'Added by', cell: ({ row: { original } }) => }, - { id: 'datasourceType', header: 'Datasource type', cell: DatasourceTypeCell, sortType: 'string' }, - { id: 'createdAtTimestamp', header: 'Date added', cell: DateAddedCell, sortType: timestampSort }, - { - id: 'actions', - header: '', - cell: ({ row: { original } }) => ( - - ), - }, - ]; -} - -type Props = { - queryTemplateRows: QueryTemplateRow[]; - queryActionButton?: QueryActionButton; -}; - -export default function QueryTemplatesTable({ queryTemplateRows, queryActionButton }: Props) { - const styles = useStyles2(getStyles); - const columns = createColumns(queryActionButton); - - return ( - row.index} - pageSize={20} - className={styles.table} - /> - ); -} - -const getStyles = (theme: GrafanaTheme2) => ({ - table: css({ - 'tbody tr': { - position: 'relative', - backgroundColor: theme.colors.background.secondary, - borderCollapse: 'collapse', - borderBottom: 'unset', - overflow: 'hidden', // Ensure the row doesn't overflow and cause additonal scrollbars - }, - /* Adds the pseudo-element for the lines between table rows */ - 'tbody tr::after': { - content: '""', - position: 'absolute', - inset: 'auto 0 0 0', - height: theme.spacing(0.5), - backgroundColor: theme.colors.background.primary, - }, - }), -}); diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/styles.tsx b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/styles.tsx deleted file mode 100644 index 715cd5d235b..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/styles.tsx +++ /dev/null @@ -1,47 +0,0 @@ -import { css } from '@emotion/css'; - -import { GrafanaTheme2 } from '@grafana/data/'; -import { useStyles2 } from '@grafana/ui/'; - -export const useQueryLibraryListStyles = () => { - return useStyles2(getStyles); -}; - -const getStyles = (theme: GrafanaTheme2) => ({ - logo: css({ - marginRight: theme.spacing(2), - width: '16px', - }), - header: css({ - margin: 0, - fontSize: theme.typography.h5.fontSize, - color: theme.colors.text.secondary, - }), - mainText: css({ - margin: 0, - fontSize: theme.typography.body.fontSize, - textOverflow: 'ellipsis', - }), - otherText: css({ - margin: 0, - fontSize: theme.typography.body.fontSize, - color: theme.colors.text.secondary, - textOverflow: 'ellipsis', - }), - singleLine: css({ - display: '-webkit-box', - WebkitBoxOrient: 'vertical', - WebkitLineClamp: 1, - overflow: 'hidden', - }), - cell: css({ - display: 'flex', - alignItems: 'center', - '&:last-child': { - justifyContent: 'end', - }, - }), - actionButton: css({ - padding: theme.spacing(1), - }), -}); diff --git a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts b/public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts deleted file mode 100644 index 51f5e490953..00000000000 --- a/public/app/features/explore/QueryLibrary/QueryTemplatesTable/types.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { DataQuery, DataSourceRef } from '@grafana/schema'; -import { User } from 'app/features/query-library/types'; - -export type QueryTemplateRow = { - index: string; - datasourceName?: string; - description?: string; - query?: DataQuery; - queryText?: string; - datasourceRef?: DataSourceRef | null; - datasourceType?: string; - createdAtTimestamp?: number; - user?: User; - uid?: string; -}; diff --git a/public/app/features/explore/QueryLibrary/SaveQueryButton.tsx b/public/app/features/explore/QueryLibrary/SaveQueryButton.tsx deleted file mode 100644 index 07c75179ed6..00000000000 --- a/public/app/features/explore/QueryLibrary/SaveQueryButton.tsx +++ /dev/null @@ -1,44 +0,0 @@ -import { useLocalStorage } from 'react-use'; - -import { DataQuery } from '@grafana/schema'; -import { Badge } from '@grafana/ui'; - -import { QueryOperationAction } from '../../../core/components/QueryOperationRow/QueryOperationAction'; -import { t } from '../../../core/internationalization'; - -import { QUERY_LIBRARY_LOCAL_STORAGE_KEYS } from './QueryLibrary'; -import { useQueryLibraryContext } from './QueryLibraryContext'; - -interface Props { - query: DataQuery; -} - -export function SaveQueryButton({ query }: Props) { - const { openAddQueryModal } = useQueryLibraryContext(); - - const [showQueryLibraryBadgeButton, setShowQueryLibraryBadgeButton] = useLocalStorage( - QUERY_LIBRARY_LOCAL_STORAGE_KEYS.explore.newButton, - true - ); - - return showQueryLibraryBadgeButton ? ( - { - openAddQueryModal(query); - setShowQueryLibraryBadgeButton(false); - }} - style={{ cursor: 'pointer' }} - /> - ) : ( - { - openAddQueryModal(query); - }} - /> - ); -} diff --git a/public/app/features/explore/QueryLibrary/utils/dataFetching.ts b/public/app/features/explore/QueryLibrary/utils/dataFetching.ts deleted file mode 100644 index e874d6a331e..00000000000 --- a/public/app/features/explore/QueryLibrary/utils/dataFetching.ts +++ /dev/null @@ -1,106 +0,0 @@ -import { skipToken } from '@reduxjs/toolkit/query'; -import { compact, uniq } from 'lodash'; -import { useAsync } from 'react-use'; -import { AsyncState } from 'react-use/lib/useAsync'; - -import { getDataSourceSrv } from '@grafana/runtime'; -import { DataQuery, DataSourceRef } from '@grafana/schema'; - -import { createQueryText } from '../../../../core/utils/richHistory'; -import { useGetDisplayMappingQuery } from '../../../iam'; -import { getDatasourceSrv } from '../../../plugins/datasource_srv'; -import { QueryTemplate } from '../../../query-library/types'; - -export function useLoadUsers(userUIDs: string[] | undefined) { - const userQtList = uniq(compact(userUIDs)); - return useGetDisplayMappingQuery( - userUIDs - ? { - key: userQtList, - } - : skipToken - ); -} - -// Explicitly type the result so TS knows to discriminate between the error result and good result by the error prop -// value. -type MetadataValue = - | { - index: string; - uid: string; - datasourceName: string; - datasourceRef: DataSourceRef | undefined | null; - datasourceType: string; - createdAtTimestamp: number; - query: DataQuery; - queryText: string; - description: string; - user: { - uid: string; - displayName: string; - avatarUrl: string; - }; - error: undefined; - } - | { - index: string; - error: Error; - }; - -/** - * Map metadata to query templates we get from the DB. - * @param queryTemplates - * @param userDataList - */ -export function useLoadQueryMetadata( - queryTemplates: QueryTemplate[] | undefined, - userDataList: ReturnType['data'] -): AsyncState { - return useAsync(async () => { - if (!(queryTemplates && userDataList)) { - return []; - } - - const rowsPromises = queryTemplates.map( - async (queryTemplate: QueryTemplate, index: number): Promise => { - try { - const datasourceRef = queryTemplate.targets[0]?.datasource; - const datasourceApi = await getDataSourceSrv().get(datasourceRef); - const datasourceType = getDatasourceSrv().getInstanceSettings(datasourceRef)?.meta.name || ''; - const query = queryTemplate.targets[0]; - const queryText = createQueryText(query, datasourceApi); - const datasourceName = datasourceApi?.name || ''; - const extendedUserData = userDataList.display.find( - (user) => `${user?.identity.type}:${user?.identity.name}` === queryTemplate.user?.uid - ); - - return { - index: index.toString(), - uid: queryTemplate.uid, - datasourceName, - datasourceRef, - datasourceType, - createdAtTimestamp: queryTemplate?.createdAtTimestamp || 0, - query, - queryText, - description: queryTemplate.title, - user: { - uid: queryTemplate.user?.uid || '', - displayName: extendedUserData?.displayName || '', - avatarUrl: extendedUserData?.avatarURL || '', - }, - error: undefined, - }; - } catch (error) { - // Instead of throwing we collect the errors in the result so upstream code can decide what to do. - return { - index: index.toString(), - error: error instanceof Error ? error : new Error('unknown error ' + JSON.stringify(error)), - }; - } - } - ); - - return Promise.all(rowsPromises); - }, [queryTemplates, userDataList]); -} diff --git a/public/app/features/explore/QueryLibrary/utils/search.ts b/public/app/features/explore/QueryLibrary/utils/search.ts deleted file mode 100644 index 2c55be7e9d0..00000000000 --- a/public/app/features/explore/QueryLibrary/utils/search.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { QueryTemplateRow } from '../QueryTemplatesTable/types'; - -export const searchQueryLibrary = ( - queryLibrary: QueryTemplateRow[], - query: string, - dsFilters: string[], - userNameFilters: string[] -) => { - const result = queryLibrary.filter((item) => { - const matchesDsFilter = - dsFilters.length === 0 || dsFilters.some((f) => item.datasourceName?.toLowerCase().includes(f.toLowerCase())); - const matchesUserNameFilter = - userNameFilters.length === 0 || userNameFilters.includes(item.user?.displayName || ''); - return ( - (item.datasourceName?.toLowerCase().includes(query.toLowerCase()) || - item.datasourceType?.toLowerCase().includes(query.toLowerCase()) || - item.description?.toLowerCase().includes(query.toLowerCase()) || - item.queryText?.toLowerCase().includes(query.toLowerCase())) && - matchesDsFilter && - matchesUserNameFilter - ); - }); - return result; -}; diff --git a/public/app/features/explore/QueryLibrary/utils/useDatasource.tsx b/public/app/features/explore/QueryLibrary/utils/useDatasource.tsx deleted file mode 100644 index 1ea4fd29b76..00000000000 --- a/public/app/features/explore/QueryLibrary/utils/useDatasource.tsx +++ /dev/null @@ -1,9 +0,0 @@ -import { useAsync } from 'react-use'; - -import { getDataSourceSrv } from '@grafana/runtime'; -import { DataSourceRef } from '@grafana/schema'; - -export function useDatasource(dataSourceRef?: DataSourceRef | null) { - const { value } = useAsync(async () => await getDataSourceSrv().get(dataSourceRef), [dataSourceRef]); - return value; -} diff --git a/public/app/features/explore/RichHistory/RichHistoryAddToLibrary.tsx b/public/app/features/explore/RichHistory/RichHistoryAddToLibrary.tsx index e6aa0e25143..8e7a057a453 100644 --- a/public/app/features/explore/RichHistory/RichHistoryAddToLibrary.tsx +++ b/public/app/features/explore/RichHistory/RichHistoryAddToLibrary.tsx @@ -1,57 +1,32 @@ import { useState } from 'react'; import { DataQuery } from '@grafana/schema'; -import { Button, Modal } from '@grafana/ui'; +import { Button } from '@grafana/ui'; import { t } from 'app/core/internationalization'; -import { isQueryLibraryEnabled, useListQueryTemplateQuery } from 'app/features/query-library'; -import { - queryLibraryTrackAddFromQueryHistory, - queryLibraryTrackAddFromQueryHistoryAddModalShown, -} from '../QueryLibrary/QueryLibraryAnalyticsEvents'; -import { QueryTemplateForm } from '../QueryLibrary/QueryTemplateForm'; +import { useQueryLibraryContext } from '../QueryLibrary/QueryLibraryContext'; type Props = { query: DataQuery; }; export const RichHistoryAddToLibrary = ({ query }: Props) => { - const { refetch } = useListQueryTemplateQuery({}); - const [isOpen, setIsOpen] = useState(false); const [hasBeenSaved, setHasBeenSaved] = useState(false); + const { openAddQueryModal, queryLibraryEnabled } = useQueryLibraryContext(); const buttonLabel = t('explore.rich-history-card.add-to-library', 'Add to library'); - return isQueryLibraryEnabled() && !hasBeenSaved ? ( + return queryLibraryEnabled && !hasBeenSaved ? ( <> - setIsOpen(false)} - > - setIsOpen(() => false)} - queryToAdd={query} - onSave={(isSuccess) => { - if (isSuccess) { - setIsOpen(false); - setHasBeenSaved(true); - refetch(); - queryLibraryTrackAddFromQueryHistory(query.datasource?.type || ''); - } - }} - /> - ) : undefined; }; diff --git a/public/app/features/explore/spec/helper/setup.tsx b/public/app/features/explore/spec/helper/setup.tsx index c57cf52c435..78aa044cab3 100644 --- a/public/app/features/explore/spec/helper/setup.tsx +++ b/public/app/features/explore/spec/helper/setup.tsx @@ -4,6 +4,7 @@ import { createMemoryHistory } from 'history'; import { KBarProvider } from 'kbar'; import { fromPairs } from 'lodash'; import { stringify } from 'querystring'; +import { ComponentType, ReactNode } from 'react'; import { Provider } from 'react-redux'; // eslint-disable-next-line no-restricted-imports import { Route, Router } from 'react-router-dom'; @@ -47,7 +48,6 @@ import { ExploreQueryParams } from '../../../../types'; import { initialUserState } from '../../../profile/state/reducers'; import ExplorePage from '../../ExplorePage'; import { QueriesDrawerContextProvider } from '../../QueriesDrawer/QueriesDrawerContext'; -import { QueryLibraryContextProvider } from '../../QueryLibrary/QueryLibraryContext'; type DatasourceSetup = { settings: DataSourceInstanceSettings; api: DataSourceApi }; @@ -60,6 +60,7 @@ type SetupOptions = { failAddToLibrary?: boolean; // Use AppChrome wrapper around ExplorePage - needed to test query library/history withAppChrome?: boolean; + provider?: ComponentType<{ children: ReactNode }>; }; type TearDownOptions = { @@ -179,12 +180,18 @@ export function setupExplore(options?: SetupOptions): { const contextMock = getGrafanaContextMock({ location }); + const FinalProvider = + options?.provider || + (({ children }) => { + return children; + }); + const { unmount, container } = render( - - + + {options?.withAppChrome ? ( @@ -204,8 +211,8 @@ export function setupExplore(options?: SetupOptions): { render={(props) => } /> )} - - + + diff --git a/public/app/features/explore/spec/queryLibrary.test.tsx b/public/app/features/explore/spec/queryLibrary.test.tsx deleted file mode 100644 index 8de9796fdf0..00000000000 --- a/public/app/features/explore/spec/queryLibrary.test.tsx +++ /dev/null @@ -1,172 +0,0 @@ -import { Props } from 'react-virtualized-auto-sizer'; - -import { EventBusSrv } from '@grafana/data'; -import { config } from '@grafana/runtime'; -import { DataQuery } from '@grafana/schema/dist/esm/veneer/common.types'; - -import { silenceConsoleOutput } from '../../../../test/core/utils/silenceConsoleOutput'; - -import { - assertAddToQueryLibraryButtonExists, - assertQueryHistory, - assertQueryLibraryTemplateExists, -} from './helper/assert'; -import { - addQueryHistoryToQueryLibrary, - openQueryHistory, - openQueryLibrary, - submitAddToQueryLibrary, -} from './helper/interactions'; -import { setupExplore, waitForExplore } from './helper/setup'; - -const reportInteractionMock = jest.fn(); -const testEventBus = new EventBusSrv(); -testEventBus.publish = jest.fn(); - -interface MockQuery extends DataQuery { - expr: string; -} - -jest.mock('../QueryLibrary/utils/dataFetching', () => { - return { - __esModule: true, - ...jest.requireActual('../QueryLibrary/utils/dataFetching'), - useLoadUsers: () => { - return { - data: { - display: [ - { - avatarUrl: '', - displayName: 'john doe', - identity: { - name: 'JohnDoe', - type: 'viewer', - }, - }, - ], - }, - isLoading: false, - error: null, - }; - }, - }; -}); - -jest.mock('@grafana/runtime', () => ({ - ...jest.requireActual('@grafana/runtime'), - reportInteraction: (...args: object[]) => { - reportInteractionMock(...args); - }, - getAppEvents: () => testEventBus, - usePluginLinks: jest.fn().mockReturnValue({ links: [] }), -})); - -jest.mock('app/core/core', () => ({ - contextSrv: { - hasPermission: () => true, - isSignedIn: true, - getValidIntervals: (defaultIntervals: string[]) => defaultIntervals, - user: { - isSignedIn: true, - }, - }, -})); - -jest.mock('app/core/services/PreferencesService', () => ({ - PreferencesService: function () { - return { - patch: jest.fn(), - load: jest.fn().mockResolvedValue({ - queryHistory: { - homeTab: 'query', - }, - }), - }; - }, -})); - -jest.mock('../hooks/useExplorePageTitle', () => ({ - useExplorePageTitle: jest.fn(), -})); - -jest.mock('react-virtualized-auto-sizer', () => { - return { - __esModule: true, - default(props: Props) { - return
{props.children({ height: 1, scaledHeight: 1, scaledWidth: 1000, width: 1000 })}
; - }, - }; -}); - -function setupQueryLibrary() { - const mockQuery: MockQuery = { refId: 'TEST', expr: 'TEST' }; - setupExplore({ - queryHistory: { - queryHistory: [{ datasourceUid: 'loki', queries: [mockQuery] }], - totalCount: 1, - }, - withAppChrome: true, - }); -} - -let previousQueryLibraryEnabled: boolean | undefined; -let previousQueryHistoryEnabled: boolean; - -describe('QueryLibrary', () => { - silenceConsoleOutput(); - - beforeAll(() => { - previousQueryLibraryEnabled = config.featureToggles.queryLibrary; - previousQueryHistoryEnabled = config.queryHistoryEnabled; - - config.featureToggles.queryLibrary = true; - config.queryHistoryEnabled = true; - }); - - afterAll(() => { - config.featureToggles.queryLibrary = previousQueryLibraryEnabled; - config.queryHistoryEnabled = previousQueryHistoryEnabled; - jest.restoreAllMocks(); - }); - - it('Load query templates', async () => { - setupQueryLibrary(); - await waitForExplore(); - await openQueryLibrary(); - await assertQueryLibraryTemplateExists('loki', 'Loki Query Template'); - }); - - it('Shows add to query library button only when the toggle is enabled', async () => { - setupQueryLibrary(); - await waitForExplore(); - await openQueryHistory(); - await assertQueryHistory(['{"expr":"TEST"}']); - await assertAddToQueryLibraryButtonExists(true); - }); - - it('Does not show the query library button when the toggle is disabled', async () => { - config.featureToggles.queryLibrary = false; - setupQueryLibrary(); - await waitForExplore(); - await openQueryHistory(); - await assertQueryHistory(['{"expr":"TEST"}']); - await assertAddToQueryLibraryButtonExists(false); - config.featureToggles.queryLibrary = true; - }); - - it('Shows a notification when a template is added and hides the add button', async () => { - setupQueryLibrary(); - await waitForExplore(); - await openQueryHistory(); - await assertQueryHistory(['{"expr":"TEST"}']); - await addQueryHistoryToQueryLibrary(); - await submitAddToQueryLibrary({ description: 'Test' }); - expect(testEventBus.publish).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'alert-success', - payload: ['Query successfully saved to the library'], - }) - ); - await assertAddToQueryLibraryButtonExists(false); - }); -}); diff --git a/public/app/features/query-library/index.ts b/public/app/features/query-library/index.ts index 246ca735681..7f0c6f1c325 100644 --- a/public/app/features/query-library/index.ts +++ b/public/app/features/query-library/index.ts @@ -7,8 +7,6 @@ * @alpha */ -import { config } from '@grafana/runtime'; - import { QUERY_LIBRARY_GET_LIMIT } from './api/api'; import { generatedQueryLibraryApi } from './api/endpoints.gen'; import { mockData } from './api/mocks'; @@ -46,10 +44,6 @@ export const { }, }); -export function isQueryLibraryEnabled() { - return config.featureToggles.queryLibrary; -} - export const QueryLibraryMocks = { data: mockData.all, }; diff --git a/public/app/features/query/components/QueryEditorRow.tsx b/public/app/features/query/components/QueryEditorRow.tsx index a3dc05930d7..36a3d5818ee 100644 --- a/public/app/features/query/components/QueryEditorRow.tsx +++ b/public/app/features/query/components/QueryEditorRow.tsx @@ -8,7 +8,6 @@ import { PureComponent, ReactNode } from 'react'; // Utils & Services import { CoreApp, - DataQuery, DataSourceApi, DataSourceInstanceSettings, DataSourcePluginContextProvider, @@ -24,7 +23,8 @@ import { toLegacyResponseData, } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { AngularComponent, config, getAngularLoader, getDataSourceSrv, reportInteraction } from '@grafana/runtime'; +import { AngularComponent, getAngularLoader, getDataSourceSrv, reportInteraction } from '@grafana/runtime'; +import { DataQuery } from '@grafana/schema'; import { Badge, ErrorBoundaryAlert } from '@grafana/ui'; import { OperationRowHelp } from 'app/core/components/QueryOperationRow/OperationRowHelp'; import { @@ -40,7 +40,7 @@ import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { DashboardModel } from 'app/features/dashboard/state/DashboardModel'; import { PanelModel } from 'app/features/dashboard/state/PanelModel'; -import { SaveQueryButton as SaveQueryToQueryLibraryButton } from '../../explore/QueryLibrary/SaveQueryButton'; +import { useQueryLibraryContext } from '../../explore/QueryLibrary/QueryLibraryContext'; import { QueryActionComponent, RowActionComponents } from './QueryActionComponent'; import { QueryEditorRowHeader } from './QueryEditorRowHeader'; @@ -489,7 +489,7 @@ export class QueryEditorRow extends PureComponent )} {this.renderExtraActions()} - {config.featureToggles.queryLibrary && } + > }) { + return props.providers.reduce((tree, Provider): ReactNode => { + return {tree}; + }, props.children); +} type RouterWrapperProps = { routes?: JSX.Element | false; bodyRenderHooks: ComponentType[]; pageBanners: ComponentType[]; + providers: Array>; }; export function RouterWrapper(props: RouterWrapperProps) { return ( @@ -31,7 +37,7 @@ export function RouterWrapper(props: RouterWrapperProps) { - + @@ -48,7 +54,7 @@ export function RouterWrapper(props: RouterWrapperProps) { - + diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 66492eac6b1..f4bbcfbcf7c 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -1346,36 +1346,6 @@ "scan-for-older-logs": "Scan for older logs", "stop-scan": "Stop scan" }, - "query-library": { - "add-edit-description": "Add/edit description", - "cancel": "Cancel", - "default-description": "Public", - "delete-query": "Delete query", - "delete-query-text": "You're about to remove this query from the query library. This action cannot be undone. Do you want to continue?", - "delete-query-title": "Delete query", - "private": "Private", - "public": "Public", - "query-deleted": "Query deleted", - "query-template-add-error": "Error attempting to save this query to the library", - "query-template-added": "Query successfully saved to the library", - "query-template-edit-error": "Error attempting to edit this query", - "query-template-edited": "Query template successfully edited", - "save": "Save" - }, - "query-template-modal": { - "add-info": "You're about to save this query. Once saved, you can easily access it in the Query Library tab for future use and reference.", - "add-title": "Add query to Query Library", - "auto-star": "Auto-star this query to add it to your starred list in the Query Library.", - "data-source-name": "Data source name", - "description": "Description", - "edit-info": "You're about to edit this query. Once saved, you can easily access it in the Query Library tab for future use and reference.", - "edit-title": "Edit query", - "query": "Query", - "visibility": "Visibility" - }, - "query-template-modall": { - "data-source-type": "Data source type" - }, "rich-history": { "close-tooltip": "Close query history", "datasource-a-z": "Data source A-Z", @@ -2993,14 +2963,6 @@ "role-label": "Role" } }, - "query-library": { - "datasource-names": "Datasource name(s):", - "delete-query-button": "Delete query", - "query-template-get-error": "Error attempting to load query template metadata: {{error}}", - "search": "Search by data source, query content or description", - "user-info-get-error": "Error attempting to get user info from the library: {{error}}", - "user-names": "User name(s):" - }, "query-operation": { "header": { "collapse-row": "Collapse query row", @@ -3010,8 +2972,6 @@ "expand-row": "Expand query row", "hide-response": "Hide response", "remove-query": "Remove query", - "save-to-query-library": "Save to query library", - "save-to-query-library-new": "New: Save to query library", "show-response": "Show response", "toggle-edit-mode": "Toggle text edit mode" }, diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index 193643a5111..af213f5ae27 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -1346,36 +1346,6 @@ "scan-for-older-logs": "Ŝčäʼn ƒőř őľđęř ľőģş", "stop-scan": "Ŝŧőp şčäʼn" }, - "query-library": { - "add-edit-description": "Åđđ/ęđįŧ đęşčřįpŧįőʼn", - "cancel": "Cäʼnčęľ", - "default-description": "Pūþľįč", - "delete-query": "Đęľęŧę qūęřy", - "delete-query-text": "Ÿőū'řę äþőūŧ ŧő řęmővę ŧĥįş qūęřy ƒřőm ŧĥę qūęřy ľįþřäřy. Ŧĥįş äčŧįőʼn čäʼnʼnőŧ þę ūʼnđőʼnę. Đő yőū ŵäʼnŧ ŧő čőʼnŧįʼnūę?", - "delete-query-title": "Đęľęŧę qūęřy", - "private": "Přįväŧę", - "public": "Pūþľįč", - "query-deleted": "Qūęřy đęľęŧęđ", - "query-template-add-error": "Ēřřőř äŧŧęmpŧįʼnģ ŧő şävę ŧĥįş qūęřy ŧő ŧĥę ľįþřäřy", - "query-template-added": "Qūęřy şūččęşşƒūľľy şävęđ ŧő ŧĥę ľįþřäřy", - "query-template-edit-error": "Ēřřőř äŧŧęmpŧįʼnģ ŧő ęđįŧ ŧĥįş qūęřy", - "query-template-edited": "Qūęřy ŧęmpľäŧę şūččęşşƒūľľy ęđįŧęđ", - "save": "Ŝävę" - }, - "query-template-modal": { - "add-info": "Ÿőū'řę äþőūŧ ŧő şävę ŧĥįş qūęřy. Øʼnčę şävęđ, yőū čäʼn ęäşįľy äččęşş įŧ įʼn ŧĥę Qūęřy Ŀįþřäřy ŧäþ ƒőř ƒūŧūřę ūşę äʼnđ řęƒęřęʼnčę.", - "add-title": "Åđđ qūęřy ŧő Qūęřy Ŀįþřäřy", - "auto-star": "Åūŧő-şŧäř ŧĥįş qūęřy ŧő äđđ įŧ ŧő yőūř şŧäřřęđ ľįşŧ įʼn ŧĥę Qūęřy Ŀįþřäřy.", - "data-source-name": "Đäŧä şőūřčę ʼnämę", - "description": "Đęşčřįpŧįőʼn", - "edit-info": "Ÿőū'řę äþőūŧ ŧő ęđįŧ ŧĥįş qūęřy. Øʼnčę şävęđ, yőū čäʼn ęäşįľy äččęşş įŧ įʼn ŧĥę Qūęřy Ŀįþřäřy ŧäþ ƒőř ƒūŧūřę ūşę äʼnđ řęƒęřęʼnčę.", - "edit-title": "Ēđįŧ qūęřy", - "query": "Qūęřy", - "visibility": "Vįşįþįľįŧy" - }, - "query-template-modall": { - "data-source-type": "Đäŧä şőūřčę ŧypę" - }, "rich-history": { "close-tooltip": "Cľőşę qūęřy ĥįşŧőřy", "datasource-a-z": "Đäŧä şőūřčę Å-Ż", @@ -2993,14 +2963,6 @@ "role-label": "Ŗőľę" } }, - "query-library": { - "datasource-names": "Đäŧäşőūřčę ʼnämę(ş):", - "delete-query-button": "Đęľęŧę qūęřy", - "query-template-get-error": "Ēřřőř äŧŧęmpŧįʼnģ ŧő ľőäđ qūęřy ŧęmpľäŧę męŧäđäŧä: {{error}}", - "search": "Ŝęäřčĥ þy đäŧä şőūřčę, qūęřy čőʼnŧęʼnŧ őř đęşčřįpŧįőʼn", - "user-info-get-error": "Ēřřőř äŧŧęmpŧįʼnģ ŧő ģęŧ ūşęř įʼnƒő ƒřőm ŧĥę ľįþřäřy: {{error}}", - "user-names": "Ůşęř ʼnämę(ş):" - }, "query-operation": { "header": { "collapse-row": "Cőľľäpşę qūęřy řőŵ", @@ -3010,8 +2972,6 @@ "expand-row": "Ēχpäʼnđ qūęřy řőŵ", "hide-response": "Ħįđę řęşpőʼnşę", "remove-query": "Ŗęmővę qūęřy", - "save-to-query-library": "Ŝävę ŧő qūęřy ľįþřäřy", - "save-to-query-library-new": "Ńęŵ: Ŝävę ŧő qūęřy ľįþřäřy", "show-response": "Ŝĥőŵ řęşpőʼnşę", "toggle-edit-mode": "Ŧőģģľę ŧęχŧ ęđįŧ mőđę" }, From a51e785bc17e7f1683ea41187be05a6dbd5d2124 Mon Sep 17 00:00:00 2001 From: Bogdan Matei Date: Fri, 7 Feb 2025 12:57:54 +0200 Subject: [PATCH 08/11] Dashboards: Finalize refactoring for dynamic dashboards (#100198) --- .../edit-pane/ElementSelection.ts | 2 +- .../MultiSelectedObjectsEditableElement.tsx | 30 +- .../MultiSelectedVizPanelsEditableElement.tsx | 45 +-- .../edit-pane/VizPanelEditableElement.tsx | 2 +- .../dashboard-scene/scene/DashboardScene.tsx | 4 +- .../layout-default/DashboardGridItem.tsx | 220 +++-------- .../DashboardGridItemEditor.tsx | 11 +- .../DashboardGridItemRenderer.tsx | 82 ++++ ...hboardGridItemVariableDependencyHandler.ts | 31 ++ .../DefaultGridLayoutManager.tsx | 234 +++++------ .../RowRepeaterBehavior.test.tsx | 12 +- .../layout-default/RowRepeaterBehavior.ts | 4 - .../layout-default/row-actions/RowActions.tsx | 140 +------ .../row-actions/RowActionsRenderer.tsx | 92 +++++ .../row-actions/RowOptionsButton.tsx | 18 +- .../row-actions/RowOptionsForm.test.tsx | 12 +- .../row-actions/RowOptionsForm.tsx | 33 +- .../row-actions/RowOptionsModal.tsx | 14 +- .../ResponsiveGridItem.tsx | 77 +--- .../ResponsiveGridItemEditor.tsx | 29 ++ .../ResponsiveGridItemRenderer.tsx | 27 ++ .../ResponsiveGridLayoutManager.tsx | 134 ++----- .../ResponsiveGridLayoutManagerEditor.tsx | 83 ++++ .../MultiSelectedRowItemsElement.tsx | 89 ----- .../scene/layout-rows/RowItem.tsx | 370 +++--------------- .../scene/layout-rows/RowItemEditor.tsx | 144 +++++++ .../scene/layout-rows/RowItemRenderer.tsx | 122 ++++++ .../RowItemRepeaterBehavior.test.tsx | 6 +- .../layout-rows/RowItemRepeaterBehavior.ts | 4 - .../scene/layout-rows/RowItems.tsx | 39 ++ .../scene/layout-rows/RowItemsEditor.tsx | 64 +++ .../scene/layout-rows/RowsLayoutManager.tsx | 131 ++----- .../layout-rows/RowsLayoutManagerRenderer.tsx | 32 ++ .../scene/types/BulkActionElement.ts | 7 + .../scene/types/DashboardLayoutManager.ts | 4 +- .../scene/types/LayoutParent.ts | 9 + .../MultiSelectedEditableDashboardElement.ts | 5 + .../utils/dashboardSceneGraph.ts | 11 +- public/locales/en-US/grafana.json | 25 +- public/locales/pseudo-LOCALE/grafana.json | 25 +- 40 files changed, 1209 insertions(+), 1214 deletions(-) create mode 100644 public/app/features/dashboard-scene/scene/layout-default/DashboardGridItemRenderer.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-default/DashboardGridItemVariableDependencyHandler.ts create mode 100644 public/app/features/dashboard-scene/scene/layout-default/row-actions/RowActionsRenderer.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-responsive-grid/ResponsiveGridItemEditor.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-responsive-grid/ResponsiveGridItemRenderer.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-responsive-grid/ResponsiveGridLayoutManagerEditor.tsx delete mode 100644 public/app/features/dashboard-scene/scene/layout-rows/MultiSelectedRowItemsElement.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-rows/RowItemEditor.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-rows/RowItemRenderer.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-rows/RowItems.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-rows/RowItemsEditor.tsx create mode 100644 public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManagerRenderer.tsx diff --git a/public/app/features/dashboard-scene/edit-pane/ElementSelection.ts b/public/app/features/dashboard-scene/edit-pane/ElementSelection.ts index 7a5a9ee61af..cecd28020cf 100644 --- a/public/app/features/dashboard-scene/edit-pane/ElementSelection.ts +++ b/public/app/features/dashboard-scene/edit-pane/ElementSelection.ts @@ -152,7 +152,7 @@ export class ElementSelection { const firstObj = this.selectedObjects?.values().next().value?.resolve(); if (firstObj instanceof VizPanel) { - return new MultiSelectedVizPanelsEditableElement(sceneObjects); + return new MultiSelectedVizPanelsEditableElement(sceneObjects.filter((obj) => obj instanceof VizPanel)); } if (isEditableDashboardElement(firstObj!)) { diff --git a/public/app/features/dashboard-scene/edit-pane/MultiSelectedObjectsEditableElement.tsx b/public/app/features/dashboard-scene/edit-pane/MultiSelectedObjectsEditableElement.tsx index 79724e4474e..3278deb3a17 100644 --- a/public/app/features/dashboard-scene/edit-pane/MultiSelectedObjectsEditableElement.tsx +++ b/public/app/features/dashboard-scene/edit-pane/MultiSelectedObjectsEditableElement.tsx @@ -1,4 +1,5 @@ import { ReactNode } from 'react'; +import { v4 as uuidv4 } from 'uuid'; import { Stack, Text, Button } from '@grafana/ui'; import { Trans } from 'app/core/internationalization'; @@ -9,31 +10,32 @@ import { MultiSelectedEditableDashboardElement } from '../scene/types/MultiSelec export class MultiSelectedObjectsEditableElement implements MultiSelectedEditableDashboardElement { public readonly isMultiSelectedEditableDashboardElement = true; public readonly typeName = 'Objects'; + public readonly key: string; - private items?: BulkActionElement[]; - - constructor(items: BulkActionElement[]) { - this.items = items; + constructor(private _elements: BulkActionElement[]) { + this.key = uuidv4(); } - public onDelete = () => { - for (const item of this.items || []) { - item.onDelete(); - } - }; - - renderActions(): ReactNode { + public renderActions(): ReactNode { return ( - No. of objects selected: - {this.items?.length} + + No. of objects selected: {{ length }} + - {!isClone && isEditing && ( - + {!isClone && isEditing && ( +