diff --git a/.betterer.results b/.betterer.results index 9106693440a..1772909d8f6 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1327,8 +1327,10 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Do not use any type assertions.", "1"], [0, 0, 0, "Do not use any type assertions.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"] + [0, 0, 0, "Do not use any type assertions.", "3"], + [0, 0, 0, "Unexpected any. Specify a different type.", "4"], + [0, 0, 0, "Unexpected any. Specify a different type.", "5"], + [0, 0, 0, "Unexpected any. Specify a different type.", "6"] ], "public/app/core/utils/richHistory.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] diff --git a/docs/sources/introduction/grafana-enterprise.md b/docs/sources/introduction/grafana-enterprise.md index 28a647254ef..e9317e632a4 100644 --- a/docs/sources/introduction/grafana-enterprise.md +++ b/docs/sources/introduction/grafana-enterprise.md @@ -33,7 +33,7 @@ Grafana Enterprise includes integrations with more ways to authenticate your use Supported auth providers: -- [Auth Proxy]({{< relref "../setup-grafana/configure-security/configure-authentication/auth-proxy#team-sync-enterprise-only" >}}) +- [Auth Proxy](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/auth-proxy#team-sync-enterprise-only) - [Azure AD](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/azuread#group-sync-enterprise-only) - [Generic OAuth integration](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/generic-oauth#configure-group-synchronization) - [GitHub OAuth](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/github#configure-group-synchronization) diff --git a/docs/sources/setup-grafana/configure-security/configure-team-sync.md b/docs/sources/setup-grafana/configure-security/configure-team-sync.md index f5d50cff311..89665525574 100644 --- a/docs/sources/setup-grafana/configure-security/configure-team-sync.md +++ b/docs/sources/setup-grafana/configure-security/configure-team-sync.md @@ -27,16 +27,15 @@ This mechanism allows Grafana to remove an existing synchronized user from a tea ## Supported providers -- [Auth Proxy]({{< relref "./configure-authentication/auth-proxy#team-sync-enterprise-only" >}}) -- [Azure AD](https://grafana.com/docs/grafana//configure-authentication/azuread#group-sync-enterprise-only) -- [Azure AD](https://grafana.com/docs/grafana//configure-security/configure-authentication/azuread#group-sync-enterprise-only) -- [Generic OAuth integration](https://grafana.com/docs/grafana//configure-security/configure-authentication/generic-oauth#configure-group-synchronization) -- [GitHub OAuth](https://grafana.com/docs/grafana//configure-security/configure-authentication/github#configure-group-synchronization) -- [GitLab OAuth](https://grafana.com/docs/grafana//configure-security/configure-authentication/gitlab#configure-group-synchronization) -- [Google OAuth](https://grafana.com/docs/grafana//configure-security/configure-authentication/google#configure-group-synchronization) -- [LDAP](https://grafana.com/docs/grafana//configure-security/configure-authentication/enhanced-ldap#ldap-group-synchronization) -- [Okta](https://grafana.com/docs/grafana//configure-security/configure-authentication/okta#configure-group-synchronization-enterprise-only) -- [SAML](https://grafana.com/docs/grafana//configure-security/configure-authentication/saml#configure-group-synchronization) +- [Auth Proxy](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/auth-proxy/#team-sync-enterprise-only) +- [Azure AD](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/azuread#group-sync-enterprise-only) +- [Generic OAuth integration](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/generic-oauth#configure-group-synchronization) +- [GitHub OAuth](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/github#configure-group-synchronization) +- [GitLab OAuth](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/gitlab#configure-group-synchronization) +- [Google OAuth](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/google#configure-group-synchronization) +- [LDAP](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/enhanced-ldap#ldap-group-synchronization) +- [Okta](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/okta#configure-group-synchronization-enterprise-only) +- [SAML](https://grafana.com/docs/grafana//setup-grafana/configure-security/configure-authentication/saml#configure-group-synchronization) ## Synchronize a Grafana team with an external group diff --git a/go.mod b/go.mod index 2dda2cc05ec..e2845126aa1 100644 --- a/go.mod +++ b/go.mod @@ -111,7 +111,7 @@ require ( github.com/influxdata/influxql v1.4.0 // @grafana/partner-datasources github.com/influxdata/line-protocol v0.0.0-20210922203350-b1ad95c89adf // @grafana/grafana-app-platform-squad github.com/jmespath-community/go-jmespath v1.1.1 // @grafana/identity-access-team - github.com/jmespath/go-jmespath v0.4.0 // indirect; // @grafana/grafana-backend-group + github.com/jmespath/go-jmespath v0.4.0 // // @grafana/grafana-backend-group github.com/jmoiron/sqlx v1.3.5 // @grafana/grafana-backend-group github.com/json-iterator/go v1.1.12 // @grafana/grafana-backend-group github.com/lib/pq v1.10.9 // @grafana/grafana-backend-group diff --git a/packages/grafana-schema/src/schema/dashboard/v2alpha0/dashboard.schema.cue b/packages/grafana-schema/src/schema/dashboard/v2alpha0/dashboard.schema.cue index 21012f3461a..f9763bb219b 100644 --- a/packages/grafana-schema/src/schema/dashboard/v2alpha0/dashboard.schema.cue +++ b/packages/grafana-schema/src/schema/dashboard/v2alpha0/dashboard.schema.cue @@ -6,10 +6,7 @@ import ( DashboardV2Spec: { // Title of dashboard. - title: string - - // Description of dashboard. - description?: string + annotations: [...AnnotationQueryKind] // Configuration of dashboard cursor sync behavior. // "Off" for no shared crosshair or tooltip (default). @@ -17,6 +14,19 @@ DashboardV2Spec: { // "Tooltip" for shared crosshair AND shared tooltip. cursorSync: DashboardCursorSync + // Description of dashboard. + description?: string + + // Whether a dashboard is editable or not. + editable?: bool | *true + + elements: [ElementReference.name]: Element + + layout: GridLayoutKind | RowsLayoutKind | ResponsiveGridLayoutKind + + // Links with references to other dashboards or external websites. + links: [...DashboardLink] + // When set to true, the dashboard will redraw panels at an interval matching the pixel width. // This will keep data "moving left" regardless of the query refresh rate. This setting helps // avoid dashboards presenting stale live data. @@ -25,30 +35,20 @@ DashboardV2Spec: { // When set to true, the dashboard will load all panels in the dashboard when it's loaded. preload: bool - // Whether a dashboard is editable or not. - editable?: bool | *true - - // Links with references to other dashboards or external websites. - links: [...DashboardLink] + // Plugins only. The version of the dashboard installed together with the plugin. + // This is used to determine if the dashboard should be updated when the plugin is updated. + revision?: uint16 // Tags associated with dashboard. tags: [...string] timeSettings: TimeSettingsSpec + // Title of dashboard. + title: string + // Configured template variables. variables: [...VariableKind] - - elements: [ElementReference.name]: Element - - annotations: [...AnnotationQueryKind] - - layout: GridLayoutKind | RowsLayoutKind | ResponsiveGridLayoutKind - - - // Plugins only. The version of the dashboard installed together with the plugin. - // This is used to determine if the dashboard should be updated when the plugin is updated. - revision?: uint16 } // Supported dashboard elements diff --git a/packages/grafana-schema/src/schema/dashboard/v2alpha0/types.gen.ts b/packages/grafana-schema/src/schema/dashboard/v2alpha0/types.gen.ts index e85e71996fd..1ad02256fe8 100644 --- a/packages/grafana-schema/src/schema/dashboard/v2alpha0/types.gen.ts +++ b/packages/grafana-schema/src/schema/dashboard/v2alpha0/types.gen.ts @@ -5,49 +5,50 @@ import * as common from '@grafana/schema'; export interface DashboardV2Spec { // Title of dashboard. - title: string; - // Description of dashboard. - description?: string; + annotations: AnnotationQueryKind[]; // Configuration of dashboard cursor sync behavior. // "Off" for no shared crosshair or tooltip (default). // "Crosshair" for shared crosshair. // "Tooltip" for shared crosshair AND shared tooltip. cursorSync: DashboardCursorSync; + // Description of dashboard. + description?: string; + // Whether a dashboard is editable or not. + editable?: boolean; + elements: Record; + layout: GridLayoutKind | RowsLayoutKind | ResponsiveGridLayoutKind; + // Links with references to other dashboards or external websites. + links: DashboardLink[]; // When set to true, the dashboard will redraw panels at an interval matching the pixel width. // This will keep data "moving left" regardless of the query refresh rate. This setting helps // avoid dashboards presenting stale live data. liveNow?: boolean; // When set to true, the dashboard will load all panels in the dashboard when it's loaded. preload: boolean; - // Whether a dashboard is editable or not. - editable?: boolean; - // Links with references to other dashboards or external websites. - links: DashboardLink[]; - // Tags associated with dashboard. - tags: string[]; - timeSettings: TimeSettingsSpec; - // Configured template variables. - variables: VariableKind[]; - elements: Record; - annotations: AnnotationQueryKind[]; - layout: GridLayoutKind | RowsLayoutKind | ResponsiveGridLayoutKind; // Plugins only. The version of the dashboard installed together with the plugin. // This is used to determine if the dashboard should be updated when the plugin is updated. revision?: number; + // Tags associated with dashboard. + tags: string[]; + timeSettings: TimeSettingsSpec; + // Title of dashboard. + title: string; + // Configured template variables. + variables: VariableKind[]; } export const defaultDashboardV2Spec = (): DashboardV2Spec => ({ - title: "", + annotations: [], cursorSync: "Off", - preload: false, editable: true, + elements: {}, + layout: defaultGridLayoutKind(), links: [], + preload: false, tags: [], timeSettings: defaultTimeSettingsSpec(), + title: "", variables: [], - elements: {}, - annotations: [], - layout: defaultGridLayoutKind(), }); // Supported dashboard elements diff --git a/pkg/apis/dashboard/migration/migrate.go b/pkg/apis/dashboard/migration/migrate.go index 36a4679f191..2a419596767 100644 --- a/pkg/apis/dashboard/migration/migrate.go +++ b/pkg/apis/dashboard/migration/migrate.go @@ -9,10 +9,6 @@ func Migrate(dash map[string]interface{}, targetVersion int) error { inputVersion := schemaversion.GetSchemaVersion(dash) dash["schemaVersion"] = inputVersion - if inputVersion < schemaversion.MINIUM_VERSION { - return schemaversion.NewMinimumVersionError(inputVersion) - } - for nextVersion := inputVersion + 1; nextVersion <= targetVersion; nextVersion++ { if migration, ok := schemaversion.Migrations[nextVersion]; ok { if err := migration(dash); err != nil { diff --git a/pkg/apis/dashboard/migration/migrate_test.go b/pkg/apis/dashboard/migration/migrate_test.go index a24baa993ee..e037c37f00f 100644 --- a/pkg/apis/dashboard/migration/migrate_test.go +++ b/pkg/apis/dashboard/migration/migrate_test.go @@ -22,15 +22,6 @@ func TestMigrate(t *testing.T) { files, err := os.ReadDir(INPUT_DIR) require.NoError(t, err) - t.Run("minimum version check", func(t *testing.T) { - err := migration.Migrate(map[string]interface{}{ - "schemaVersion": schemaversion.MINIUM_VERSION - 1, - }, schemaversion.MINIUM_VERSION) - - var minVersionErr = schemaversion.NewMinimumVersionError(schemaversion.MINIUM_VERSION - 1) - require.ErrorAs(t, err, &minVersionErr) - }) - for _, f := range files { if f.IsDir() { continue diff --git a/pkg/apis/dashboard/migration/schemaversion/errors.go b/pkg/apis/dashboard/migration/schemaversion/errors.go index 110a596a1ad..f5bbbe7d1fa 100644 --- a/pkg/apis/dashboard/migration/schemaversion/errors.go +++ b/pkg/apis/dashboard/migration/schemaversion/errors.go @@ -2,23 +2,8 @@ package schemaversion import "fmt" -var _ error = &MinimumVersionError{} var _ error = &MigrationError{} -// MinimumVersionError is an error that is returned when the schema version is below the minimum version. -func NewMinimumVersionError(inputVersion int) *MinimumVersionError { - return &MinimumVersionError{inputVersion: inputVersion} -} - -// MinimumVersionError is an error type for minimum version errors. -type MinimumVersionError struct { - inputVersion int -} - -func (e *MinimumVersionError) Error() string { - return fmt.Errorf("input schema version is below minimum version. input: %d minimum: %d", e.inputVersion, MINIUM_VERSION).Error() -} - // ErrMigrationFailed is an error that is returned when a migration fails. func NewMigrationError(msg string, currentVersion, targetVersion int) *MigrationError { return &MigrationError{ diff --git a/pkg/apis/dashboard/migration/schemaversion/migrations.go b/pkg/apis/dashboard/migration/schemaversion/migrations.go index ef46439a591..3d83da73a3f 100644 --- a/pkg/apis/dashboard/migration/schemaversion/migrations.go +++ b/pkg/apis/dashboard/migration/schemaversion/migrations.go @@ -4,10 +4,7 @@ import "strconv" type SchemaVersionMigrationFunc func(map[string]interface{}) error -const ( - MINIUM_VERSION = 36 - LATEST_VERSION = 41 -) +const LATEST_VERSION = 41 var Migrations = map[int]SchemaVersionMigrationFunc{ 37: V37, diff --git a/pkg/apis/dashboard/v1alpha1/conversion.go b/pkg/apis/dashboard/v1alpha1/conversion.go index 358c482c4e3..c01b13db29c 100644 --- a/pkg/apis/dashboard/v1alpha1/conversion.go +++ b/pkg/apis/dashboard/v1alpha1/conversion.go @@ -1,8 +1,6 @@ package v1alpha1 import ( - "errors" - conversion "k8s.io/apimachinery/pkg/conversion" klog "k8s.io/klog/v2" @@ -15,12 +13,7 @@ func Convert_v0alpha1_Unstructured_To_v1alpha1_DashboardSpec(in *common.Unstruct out.Unstructured = *in err := migration.Migrate(out.Unstructured.Object, schemaversion.LATEST_VERSION) if err != nil { - minErr := &schemaversion.MinimumVersionError{} - if errors.As(err, &minErr) { - out.Unstructured.Object["__migrationError"] = err.Error() - } else { - return err - } + return err } t, ok := out.Unstructured.Object["title"].(string) diff --git a/pkg/apis/dashboard/v2alpha1/conversion.go b/pkg/apis/dashboard/v2alpha1/conversion.go index 280cac67b14..9a1818fc0c3 100644 --- a/pkg/apis/dashboard/v2alpha1/conversion.go +++ b/pkg/apis/dashboard/v2alpha1/conversion.go @@ -1,8 +1,6 @@ package v2alpha1 import ( - "errors" - conversion "k8s.io/apimachinery/pkg/conversion" klog "k8s.io/klog/v2" @@ -15,12 +13,7 @@ func Convert_v0alpha1_Unstructured_To_v2alpha1_DashboardSpec(in *common.Unstruct out.Unstructured = *in err := migration.Migrate(out.Unstructured.Object, schemaversion.LATEST_VERSION) if err != nil { - minErr := &schemaversion.MinimumVersionError{} - if errors.As(err, &minErr) { - out.Unstructured.Object["__migrationError"] = err.Error() - } else { - return err - } + return err } t, ok := out.Unstructured.Object["title"].(string) diff --git a/pkg/services/apiserver/builder/helper.go b/pkg/services/apiserver/builder/helper.go index 94f267c5544..7f4d8cc7843 100644 --- a/pkg/services/apiserver/builder/helper.go +++ b/pkg/services/apiserver/builder/helper.go @@ -289,7 +289,7 @@ func InstallAPIs( if storageOpts.StorageType != options.StorageTypeLegacy { dualWrite = func(gr schema.GroupResource, legacy grafanarest.LegacyStorage, storage grafanarest.Storage) (grafanarest.Storage, error) { // Dashboards + Folders may be managed (depends on feature toggles and database state) - if dualWriteService.ShouldManage(gr) { + if dualWriteService != nil && dualWriteService.ShouldManage(gr) { return dualWriteService.NewStorage(gr, legacy, storage) // eventually this can replace this whole function } diff --git a/pkg/services/ngalert/state/cache.go b/pkg/services/ngalert/state/cache.go index 1abeff1c2a9..a7302fc5518 100644 --- a/pkg/services/ngalert/state/cache.go +++ b/pkg/services/ngalert/state/cache.go @@ -143,81 +143,6 @@ func expandAnnotationsAndLabels(ctx context.Context, log log.Logger, alertRule * return lbs, annotations } -func (c *cache) create(ctx context.Context, log log.Logger, alertRule *ngModels.AlertRule, result eval.Result, extraLabels data.Labels, externalURL *url.URL) *State { - lbs, annotations := expandAnnotationsAndLabels(ctx, log, alertRule, result, extraLabels, externalURL) - - cacheID := lbs.Fingerprint() - // For new states, we set StartsAt & EndsAt to EvaluatedAt as this is the - // expected value for a Normal state during state transition. - newState := State{ - OrgID: alertRule.OrgID, - AlertRuleUID: alertRule.UID, - CacheID: cacheID, - State: eval.Normal, - StateReason: "", - ResultFingerprint: result.Instance.Fingerprint(), // remember original result fingerprint - LatestResult: nil, - Error: nil, - Image: nil, - Annotations: annotations, - Labels: lbs, - Values: nil, - StartsAt: result.EvaluatedAt, - EndsAt: result.EvaluatedAt, - ResolvedAt: nil, - LastSentAt: nil, - LastEvaluationString: "", - LastEvaluationTime: result.EvaluatedAt, - EvaluationDuration: result.EvaluationDuration, - } - - existingState := c.get(alertRule.OrgID, alertRule.UID, cacheID) - if existingState == nil { - return &newState - } - // if there is existing state, copy over the current values that may be needed to determine the final state. - // TODO remove some unnecessary assignments below because they are overridden in setNextState - newState.State = existingState.State - newState.StateReason = existingState.StateReason - newState.Image = existingState.Image - newState.LatestResult = existingState.LatestResult - newState.Error = existingState.Error - newState.Values = existingState.Values - newState.LastEvaluationString = existingState.LastEvaluationString - newState.StartsAt = existingState.StartsAt - newState.EndsAt = existingState.EndsAt - newState.ResolvedAt = existingState.ResolvedAt - newState.LastSentAt = existingState.LastSentAt - // Annotations can change over time, however we also want to maintain - // certain annotations across evaluations - for key := range ngModels.InternalAnnotationNameSet { // Changing in - value, ok := existingState.Annotations[key] - if !ok { - continue - } - // If the annotation is not present then it should be copied from - // the current state to the new state - if _, ok = newState.Annotations[key]; !ok { - newState.Annotations[key] = value - } - } - - // if the current state is "data source error" then it may have additional labels that may not exist in the new state. - // See https://github.com/grafana/grafana/blob/c7fdf8ce706c2c9d438f5e6eabd6e580bac4946b/pkg/services/ngalert/state/state.go#L161-L163 - // copy known labels over to the new instance, it can help reduce flapping - // TODO fix this? - if existingState.State == eval.Error && result.State == eval.Error { - setIfExist := func(lbl string) { - if v, ok := existingState.Labels[lbl]; ok { - newState.Labels[lbl] = v - } - } - setIfExist("datasource_uid") - setIfExist("ref_id") - } - return &newState -} - // expand returns the expanded templates of all annotations or labels for the template data. // If a template cannot be expanded due to an error in the template the original template is // maintained and an error is added to the multierror. All errors in the multierror are diff --git a/pkg/services/ngalert/state/cache_test.go b/pkg/services/ngalert/state/cache_test.go index 9fe50fdb120..6defc25086d 100644 --- a/pkg/services/ngalert/state/cache_test.go +++ b/pkg/services/ngalert/state/cache_test.go @@ -3,15 +3,11 @@ package state import ( "context" "errors" - "fmt" "math/rand" - "net/url" "testing" "time" - "github.com/google/uuid" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log" @@ -118,313 +114,6 @@ func Test_expand(t *testing.T) { }) } -func Test_create(t *testing.T) { - url := &url.URL{ - Scheme: "http", - Host: "localhost:3000", - Path: "/test", - } - l := log.New("test") - c := newCache() - - gen := models.RuleGen - generateRule := gen.With(gen.WithNotEmptyLabels(5, "rule-")).GenerateRef - - t.Run("should combine all labels", func(t *testing.T) { - rule := generateRule() - - extraLabels := models.GenerateAlertLabels(5, "extra-") - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - state := c.create(context.Background(), l, rule, result, extraLabels, url) - for key, expected := range extraLabels { - require.Equal(t, expected, state.Labels[key]) - } - assert.Len(t, state.Labels, len(extraLabels)+len(rule.Labels)+len(result.Instance)) - for key, expected := range extraLabels { - assert.Equal(t, expected, state.Labels[key]) - } - for key, expected := range rule.Labels { - assert.Equal(t, expected, state.Labels[key]) - } - for key, expected := range result.Instance { - assert.Equal(t, expected, state.Labels[key]) - } - }) - t.Run("extra labels should take precedence over rule and result labels", func(t *testing.T) { - rule := generateRule() - - extraLabels := models.GenerateAlertLabels(2, "extra-") - - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - for key := range extraLabels { - rule.Labels[key] = "rule-" + util.GenerateShortUID() - result.Instance[key] = "result-" + util.GenerateShortUID() - } - - state := c.create(context.Background(), l, rule, result, extraLabels, url) - for key, expected := range extraLabels { - require.Equal(t, expected, state.Labels[key]) - } - }) - t.Run("rule labels should take precedence over result labels", func(t *testing.T) { - rule := generateRule() - - extraLabels := models.GenerateAlertLabels(2, "extra-") - - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - for key := range rule.Labels { - result.Instance[key] = "result-" + util.GenerateShortUID() - } - state := c.create(context.Background(), l, rule, result, extraLabels, url) - for key, expected := range rule.Labels { - require.Equal(t, expected, state.Labels[key]) - } - }) - t.Run("rule labels should be able to be expanded with result and extra labels", func(t *testing.T) { - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - rule := generateRule() - - extraLabels := models.GenerateAlertLabels(2, "extra-") - - labelTemplates := make(data.Labels) - for key := range extraLabels { - labelTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) - } - for key := range result.Instance { - labelTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) - } - rule.Labels = labelTemplates - - state := c.create(context.Background(), l, rule, result, extraLabels, url) - for key, expected := range extraLabels { - assert.Equal(t, expected, state.Labels["rule-"+key]) - } - for key, expected := range result.Instance { - assert.Equal(t, expected, state.Labels["rule-"+key]) - } - }) - t.Run("rule annotations should be able to be expanded with result and extra labels", func(t *testing.T) { - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - - rule := generateRule() - - extraLabels := models.GenerateAlertLabels(2, "extra-") - - annotationTemplates := make(data.Labels) - for key := range extraLabels { - annotationTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) - } - for key := range result.Instance { - annotationTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) - } - rule.Annotations = annotationTemplates - - state := c.create(context.Background(), l, rule, result, extraLabels, url) - for key, expected := range extraLabels { - assert.Equal(t, expected, state.Annotations["rule-"+key]) - } - for key, expected := range result.Instance { - assert.Equal(t, expected, state.Annotations["rule-"+key]) - } - }) - t.Run("when result labels collide with system labels from LabelsUserCannotSpecify", func(t *testing.T) { - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - m := models.LabelsUserCannotSpecify - t.Cleanup(func() { - models.LabelsUserCannotSpecify = m - }) - - models.LabelsUserCannotSpecify = map[string]struct{}{ - "__label1__": {}, - "label2__": {}, - "__label3": {}, - "label4": {}, - } - result.Instance["__label1__"] = uuid.NewString() - result.Instance["label2__"] = uuid.NewString() - result.Instance["__label3"] = uuid.NewString() - result.Instance["label4"] = uuid.NewString() - - rule := generateRule() - - state := c.create(context.Background(), l, rule, result, nil, url) - - for key := range models.LabelsUserCannotSpecify { - assert.NotContains(t, state.Labels, key) - } - assert.Contains(t, state.Labels, "label1") - assert.Equal(t, state.Labels["label1"], result.Instance["__label1__"]) - - assert.Contains(t, state.Labels, "label2") - assert.Equal(t, state.Labels["label2"], result.Instance["label2__"]) - - assert.Contains(t, state.Labels, "label3") - assert.Equal(t, state.Labels["label3"], result.Instance["__label3"]) - - assert.Contains(t, state.Labels, "label4_user") - assert.Equal(t, state.Labels["label4_user"], result.Instance["label4"]) - - t.Run("should drop label if renamed collides with existing", func(t *testing.T) { - result.Instance["label1"] = uuid.NewString() - result.Instance["label1_user"] = uuid.NewString() - result.Instance["label4_user"] = uuid.NewString() - - state = c.create(context.Background(), l, rule, result, nil, url) - assert.NotContains(t, state.Labels, "__label1__") - assert.Contains(t, state.Labels, "label1") - assert.Equal(t, state.Labels["label1"], result.Instance["label1"]) - assert.Equal(t, state.Labels["label1_user"], result.Instance["label1_user"]) - - assert.NotContains(t, state.Labels, "label4") - assert.Equal(t, state.Labels["label4_user"], result.Instance["label4_user"]) - }) - }) - - t.Run("creates a state with preset fields if there is no current state", func(t *testing.T) { - rule := generateRule() - - extraLabels := models.GenerateAlertLabels(2, "extra-") - - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - - expectedLbl, expectedAnn := expandAnnotationsAndLabels(context.Background(), l, rule, result, extraLabels, url) - - state := c.create(context.Background(), l, rule, result, extraLabels, url) - - assert.Equal(t, rule.OrgID, state.OrgID) - assert.Equal(t, rule.UID, state.AlertRuleUID) - assert.Equal(t, state.Labels.Fingerprint(), state.CacheID) - assert.Equal(t, result.State, state.State) - assert.Equal(t, "", state.StateReason) - assert.Equal(t, result.Instance.Fingerprint(), state.ResultFingerprint) - assert.Nil(t, state.LatestResult) - assert.Nil(t, state.Error) - assert.Nil(t, state.Image) - assert.EqualValues(t, expectedAnn, state.Annotations) - assert.EqualValues(t, expectedLbl, state.Labels) - assert.Nil(t, state.Values) - assert.Equal(t, result.EvaluatedAt, state.StartsAt) - assert.Equal(t, result.EvaluatedAt, state.EndsAt) - assert.Nil(t, state.ResolvedAt) - assert.Nil(t, state.LastSentAt) - assert.Equal(t, "", state.LastEvaluationString) - assert.Equal(t, result.EvaluatedAt, state.LastEvaluationTime) - assert.Equal(t, result.EvaluationDuration, state.EvaluationDuration) - }) - - t.Run("it populates some fields from the current state if it exists", func(t *testing.T) { - rule := generateRule() - - extraLabels := models.GenerateAlertLabels(2, "extra-") - - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - } - - expectedLbl, expectedAnn := expandAnnotationsAndLabels(context.Background(), l, rule, result, extraLabels, url) - - current := randomSate(rule.GetKey()) - current.CacheID = expectedLbl.Fingerprint() - - c.set(¤t) - - state := c.create(context.Background(), l, rule, result, extraLabels, url) - - assert.Equal(t, rule.OrgID, state.OrgID) - assert.Equal(t, rule.UID, state.AlertRuleUID) - assert.Equal(t, state.Labels.Fingerprint(), state.CacheID) - assert.Equal(t, result.Instance.Fingerprint(), state.ResultFingerprint) - assert.EqualValues(t, expectedAnn, state.Annotations) - assert.EqualValues(t, expectedLbl, state.Labels) - assert.Equal(t, result.EvaluatedAt, state.LastEvaluationTime) - assert.Equal(t, result.EvaluationDuration, state.EvaluationDuration) - - assert.Equal(t, current.State, state.State) - assert.Equal(t, current.StateReason, state.StateReason) - assert.Equal(t, current.Image, state.Image) - assert.Equal(t, current.LatestResult, state.LatestResult) - assert.Equal(t, current.Error, state.Error) - assert.Equal(t, current.Values, state.Values) - assert.Equal(t, current.StartsAt, state.StartsAt) - assert.Equal(t, current.EndsAt, state.EndsAt) - assert.Equal(t, current.ResolvedAt, state.ResolvedAt) - assert.Equal(t, current.LastSentAt, state.LastSentAt) - assert.Equal(t, current.LastEvaluationString, state.LastEvaluationString) - - t.Run("if result Error and current state is Error it should copy datasource_uid and ref_id labels", func(t *testing.T) { - current = randomSate(rule.GetKey()) - current.CacheID = expectedLbl.Fingerprint() - current.State = eval.Error - current.Labels["datasource_uid"] = util.GenerateShortUID() - current.Labels["ref_id"] = util.GenerateShortUID() - - c.set(¤t) - - result.State = eval.Error - state = c.create(context.Background(), l, rule, result, extraLabels, url) - - l := expectedLbl.Copy() - l["datasource_uid"] = current.Labels["datasource_uid"] - l["ref_id"] = current.Labels["ref_id"] - - assert.Equal(t, current.CacheID, state.CacheID) - assert.EqualValues(t, l, state.Labels) - - assert.Equal(t, rule.OrgID, state.OrgID) - assert.Equal(t, rule.UID, state.AlertRuleUID) - - assert.Equal(t, result.Instance.Fingerprint(), state.ResultFingerprint) - assert.EqualValues(t, expectedAnn, state.Annotations) - assert.Equal(t, result.EvaluatedAt, state.LastEvaluationTime) - assert.Equal(t, result.EvaluationDuration, state.EvaluationDuration) - - assert.Equal(t, current.State, state.State) - assert.Equal(t, current.StateReason, state.StateReason) - assert.Equal(t, current.Image, state.Image) - assert.Equal(t, current.LatestResult, state.LatestResult) - assert.Equal(t, current.Error, state.Error) - assert.Equal(t, current.Values, state.Values) - assert.Equal(t, current.StartsAt, state.StartsAt) - assert.Equal(t, current.EndsAt, state.EndsAt) - assert.Equal(t, current.ResolvedAt, state.ResolvedAt) - assert.Equal(t, current.LastSentAt, state.LastSentAt) - assert.Equal(t, current.LastEvaluationString, state.LastEvaluationString) - }) - t.Run("copies system-owned annotations from current state", func(t *testing.T) { - current = randomSate(rule.GetKey()) - current.CacheID = expectedLbl.Fingerprint() - current.State = eval.Error - for key := range models.InternalAnnotationNameSet { - current.Annotations[key] = util.GenerateShortUID() - } - c.set(¤t) - - result.State = eval.Error - state = c.create(context.Background(), l, rule, result, extraLabels, url) - ann := expectedAnn.Copy() - for key := range models.InternalAnnotationNameSet { - ann[key] = current.Annotations[key] - } - assert.EqualValues(t, expectedLbl, state.Labels) - assert.EqualValues(t, ann, state.Annotations) - }) - }) -} - func Test_mergeLabels(t *testing.T) { t.Run("merges two maps", func(t *testing.T) { a := models.GenerateAlertLabels(5, "set1-") diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index cc22a65f517..f66002bbb3b 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -444,9 +444,16 @@ func (st *Manager) setNextStateForRule(ctx context.Context, alertRule *ngModels. } transitions := make([]StateTransition, 0, len(results)) for _, result := range results { - currentState := st.cache.create(ctx, logger, alertRule, result, extraLabels, st.externalURL) - s := st.setNextState(alertRule, currentState, result, nil, logger, takeImageFn) - st.cache.set(currentState) // replace the existing state with the new one + newState := newState(ctx, logger, alertRule, result, extraLabels, st.externalURL) + if curState := st.cache.get(alertRule.OrgID, alertRule.UID, newState.CacheID); curState != nil { + patch(newState, curState, result) + } + start := st.clock.Now() + s := newState.transition(alertRule, result, nil, logger, takeImageFn) + if st.metrics != nil { + st.metrics.StateUpdateDuration.Observe(st.clock.Now().Sub(start).Seconds()) + } + st.cache.set(newState) // replace the existing state with the new one transitions = append(transitions, s) } return transitions @@ -459,8 +466,12 @@ func (st *Manager) setNextStateForAll(alertRule *ngModels.AlertRule, result eval states: make(map[data.Fingerprint]*State, len(currentStates)), } for _, currentState := range currentStates { + start := st.clock.Now() newState := currentState.Copy() - t := st.setNextState(alertRule, newState, result, extraAnnotations, logger, takeImageFn) + t := newState.transition(alertRule, result, extraAnnotations, logger, takeImageFn) + if st.metrics != nil { + st.metrics.StateUpdateDuration.Observe(st.clock.Now().Sub(start).Seconds()) + } updated.states[newState.CacheID] = newState transitions = append(transitions, t) } @@ -468,115 +479,6 @@ func (st *Manager) setNextStateForAll(alertRule *ngModels.AlertRule, result eval return transitions } -// Set the current state based on evaluation results -func (st *Manager) setNextState(alertRule *ngModels.AlertRule, currentState *State, result eval.Result, extraAnnotations data.Labels, logger log.Logger, takeImageFn takeImageFn) StateTransition { - start := st.clock.Now() - - currentState.LastEvaluationTime = result.EvaluatedAt - currentState.EvaluationDuration = result.EvaluationDuration - currentState.SetNextValues(result) - currentState.LatestResult = &Evaluation{ - EvaluationTime: result.EvaluatedAt, - EvaluationState: result.State, - Values: currentState.Values, - Condition: alertRule.Condition, - } - currentState.LastEvaluationString = result.EvaluationString - oldState := currentState.State - oldReason := currentState.StateReason - - // Add the instance to the log context to help correlate log lines for a state - logger = logger.New("instance", result.Instance) - - // if the current state is Error but the result is different, then we need o clean up the extra labels - // that were added after the state key was calculated - // https://github.com/grafana/grafana/blob/1df4d332c982dc5e394201bb2ef35b442727ce63/pkg/services/ngalert/state/state.go#L298-L311 - // Usually, it happens in the case of classic conditions when the evalResult does not have labels. - // - // This is temporary change to make sure that the labels are not persistent in the state after it was in Error state - // TODO yuri. Remove it when correct Error result with labels is provided - if currentState.State == eval.Error && result.State != eval.Error { - // This is possible because state was updated after the CacheID was calculated. - _, curOk := currentState.Labels["ref_id"] - _, resOk := result.Instance["ref_id"] - if curOk && !resOk { - delete(currentState.Labels, "ref_id") - } - _, curOk = currentState.Labels["datasource_uid"] - _, resOk = result.Instance["datasource_uid"] - if curOk && !resOk { - delete(currentState.Labels, "datasource_uid") - } - } - - switch result.State { - case eval.Normal: - logger.Debug("Setting next state", "handler", "resultNormal") - resultNormal(currentState, alertRule, result, logger, "") - case eval.Alerting: - logger.Debug("Setting next state", "handler", "resultAlerting") - resultAlerting(currentState, alertRule, result, logger, "") - case eval.Error: - logger.Debug("Setting next state", "handler", "resultError") - resultError(currentState, alertRule, result, logger) - case eval.NoData: - logger.Debug("Setting next state", "handler", "resultNoData") - resultNoData(currentState, alertRule, result, logger) - case eval.Pending: // we do not emit results with this state - logger.Debug("Ignoring set next state as result is pending") - } - - // Set reason iff: result and state are different, reason is not Alerting or Normal - currentState.StateReason = "" - - if currentState.State != result.State && - result.State != eval.Normal && - result.State != eval.Alerting { - currentState.StateReason = resultStateReason(result, alertRule) - } - - // Set Resolved property so the scheduler knows to send a postable alert - // to Alertmanager. - newlyResolved := false - if oldState == eval.Alerting && currentState.State == eval.Normal { - currentState.ResolvedAt = &result.EvaluatedAt - newlyResolved = true - } else if currentState.State != eval.Normal && currentState.State != eval.Pending { // Retain the last resolved time for Normal->Normal and Normal->Pending. - currentState.ResolvedAt = nil - } - - if reason := shouldTakeImage(currentState.State, oldState, currentState.Image, newlyResolved); reason != "" { - image := takeImageFn(reason) - if image != nil { - currentState.Image = image - } - } - - for key, val := range extraAnnotations { - currentState.Annotations[key] = val - } - - nextState := StateTransition{ - State: currentState, - PreviousState: oldState, - PreviousStateReason: oldReason, - } - - if st.metrics != nil { - st.metrics.StateUpdateDuration.Observe(st.clock.Now().Sub(start).Seconds()) - } - - return nextState -} - -func resultStateReason(result eval.Result, rule *ngModels.AlertRule) string { - if rule.ExecErrState == ngModels.KeepLastErrState || rule.NoDataState == ngModels.KeepLast { - return ngModels.ConcatReasons(result.State.String(), ngModels.StateReasonKeepLast) - } - - return result.State.String() -} - func (st *Manager) GetAll(orgID int64) []*State { allStates := st.cache.getAll(orgID) return allStates diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index aee77903096..54694664cca 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -7,6 +7,7 @@ import ( "fmt" "maps" "math" + "net/url" "strings" "time" @@ -76,6 +77,35 @@ type State struct { EvaluationDuration time.Duration } +func newState(ctx context.Context, log log.Logger, alertRule *models.AlertRule, result eval.Result, extraLabels data.Labels, externalURL *url.URL) *State { + lbs, annotations := expandAnnotationsAndLabels(ctx, log, alertRule, result, extraLabels, externalURL) + + cacheID := lbs.Fingerprint() + // For new states, we set StartsAt & EndsAt to EvaluatedAt as this is the + // expected value for a Normal state during state transition. + return &State{ + OrgID: alertRule.OrgID, + AlertRuleUID: alertRule.UID, + CacheID: cacheID, + State: eval.Normal, + StateReason: "", + ResultFingerprint: result.Instance.Fingerprint(), // remember original result fingerprint + LatestResult: nil, + Error: nil, + Image: nil, + Annotations: annotations, + Labels: lbs, + Values: nil, + StartsAt: result.EvaluatedAt, + EndsAt: result.EvaluatedAt, + ResolvedAt: nil, + LastSentAt: nil, + LastEvaluationString: "", + LastEvaluationTime: result.EvaluatedAt, + EvaluationDuration: result.EvaluationDuration, + } +} + // Copy creates a shallow copy of the State except for labels and annotations. func (a *State) Copy() *State { // Deep copy annotations and labels @@ -664,3 +694,147 @@ func GetRuleExtraLabels(l log.Logger, rule *models.AlertRule, folderTitle string } return extraLabels } + +func patch(newState, existingState *State, result eval.Result) { + // if there is existing state, copy over the current values that may be needed to determine the final state. + // TODO remove some unnecessary assignments below because they are overridden in setNextState + newState.State = existingState.State + newState.StateReason = existingState.StateReason + newState.Image = existingState.Image + newState.LatestResult = existingState.LatestResult + newState.Error = existingState.Error + newState.Values = existingState.Values + newState.LastEvaluationString = existingState.LastEvaluationString + newState.StartsAt = existingState.StartsAt + newState.EndsAt = existingState.EndsAt + newState.ResolvedAt = existingState.ResolvedAt + newState.LastSentAt = existingState.LastSentAt + // Annotations can change over time, however we also want to maintain + // certain annotations across evaluations + for key := range models.InternalAnnotationNameSet { // Changing in + value, ok := existingState.Annotations[key] + if !ok { + continue + } + // If the annotation is not present then it should be copied from + // the current state to the new state + if _, ok = newState.Annotations[key]; !ok { + newState.Annotations[key] = value + } + } + + // if the current state is "data source error" then it may have additional labels that may not exist in the new state. + // See https://github.com/grafana/grafana/blob/c7fdf8ce706c2c9d438f5e6eabd6e580bac4946b/pkg/services/ngalert/state/state.go#L161-L163 + // copy known labels over to the new instance, it can help reduce flapping + // TODO fix this? + if existingState.State == eval.Error && result.State == eval.Error { + setIfExist := func(lbl string) { + if v, ok := existingState.Labels[lbl]; ok { + newState.Labels[lbl] = v + } + } + setIfExist("datasource_uid") + setIfExist("ref_id") + } +} + +func (a *State) transition(alertRule *models.AlertRule, result eval.Result, extraAnnotations data.Labels, logger log.Logger, takeImageFn takeImageFn) StateTransition { + a.LastEvaluationTime = result.EvaluatedAt + a.EvaluationDuration = result.EvaluationDuration + a.SetNextValues(result) + a.LatestResult = &Evaluation{ + EvaluationTime: result.EvaluatedAt, + EvaluationState: result.State, + Values: a.Values, + Condition: alertRule.Condition, + } + a.LastEvaluationString = result.EvaluationString + oldState := a.State + oldReason := a.StateReason + + // Add the instance to the log context to help correlate log lines for a state + logger = logger.New("instance", result.Instance) + + // if the current state is Error but the result is different, then we need o clean up the extra labels + // that were added after the state key was calculated + // https://github.com/grafana/grafana/blob/1df4d332c982dc5e394201bb2ef35b442727ce63/pkg/services/ngalert/state/state.go#L298-L311 + // Usually, it happens in the case of classic conditions when the evalResult does not have labels. + // + // This is temporary change to make sure that the labels are not persistent in the state after it was in Error state + // TODO yuri. Remove it when correct Error result with labels is provided + if a.State == eval.Error && result.State != eval.Error { + // This is possible because state was updated after the CacheID was calculated. + _, curOk := a.Labels["ref_id"] + _, resOk := result.Instance["ref_id"] + if curOk && !resOk { + delete(a.Labels, "ref_id") + } + _, curOk = a.Labels["datasource_uid"] + _, resOk = result.Instance["datasource_uid"] + if curOk && !resOk { + delete(a.Labels, "datasource_uid") + } + } + + switch result.State { + case eval.Normal: + logger.Debug("Setting next state", "handler", "resultNormal") + resultNormal(a, alertRule, result, logger, "") + case eval.Alerting: + logger.Debug("Setting next state", "handler", "resultAlerting") + resultAlerting(a, alertRule, result, logger, "") + case eval.Error: + logger.Debug("Setting next state", "handler", "resultError") + resultError(a, alertRule, result, logger) + case eval.NoData: + logger.Debug("Setting next state", "handler", "resultNoData") + resultNoData(a, alertRule, result, logger) + case eval.Pending: // we do not emit results with this state + logger.Debug("Ignoring set next state as result is pending") + } + + // Set reason iff: result and state are different, reason is not Alerting or Normal + a.StateReason = "" + + if a.State != result.State && + result.State != eval.Normal && + result.State != eval.Alerting { + a.StateReason = resultStateReason(result, alertRule) + } + + // Set Resolved property so the scheduler knows to send a postable alert + // to Alertmanager. + newlyResolved := false + if oldState == eval.Alerting && a.State == eval.Normal { + a.ResolvedAt = &result.EvaluatedAt + newlyResolved = true + } else if a.State != eval.Normal && a.State != eval.Pending { // Retain the last resolved time for Normal->Normal and Normal->Pending. + a.ResolvedAt = nil + } + + if reason := shouldTakeImage(a.State, oldState, a.Image, newlyResolved); reason != "" { + image := takeImageFn(reason) + if image != nil { + a.Image = image + } + } + + for key, val := range extraAnnotations { + a.Annotations[key] = val + } + + nextState := StateTransition{ + State: a, + PreviousState: oldState, + PreviousStateReason: oldReason, + } + return nextState +} + +func resultStateReason(result eval.Result, rule *models.AlertRule) string { + if rule.ExecErrState == models.KeepLastErrState || rule.NoDataState == models.KeepLast { + return models.ConcatReasons(result.State.String(), models.StateReasonKeepLast) + } + + return result.State.String() +} diff --git a/pkg/services/ngalert/state/cache_bench_test.go b/pkg/services/ngalert/state/state_bench_test.go similarity index 85% rename from pkg/services/ngalert/state/cache_bench_test.go rename to pkg/services/ngalert/state/state_bench_test.go index 357eabdd9e3..8f1ec2550c8 100644 --- a/pkg/services/ngalert/state/cache_bench_test.go +++ b/pkg/services/ngalert/state/state_bench_test.go @@ -14,7 +14,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/models" ) -func BenchmarkGetOrCreateTest(b *testing.B) { +func BenchmarkCreateAndPatch(b *testing.B) { cache := newCache() rule := models.RuleGen.With(func(rule *models.AlertRule) { for i := 0; i < 2; i++ { @@ -43,7 +43,11 @@ func BenchmarkGetOrCreateTest(b *testing.B) { // values := make([]int64, count) b.RunParallel(func(pb *testing.PB) { for pb.Next() { - _ = cache.create(ctx, log, rule, result, nil, u) + s := newState(ctx, log, rule, result, nil, u) + current := cache.get(rule.OrgID, rule.UID, s.CacheID) + if current == nil { + patch(s, current, result) + } } }) } diff --git a/pkg/services/ngalert/state/state_test.go b/pkg/services/ngalert/state/state_test.go index 2515fe33190..7216784a65c 100644 --- a/pkg/services/ngalert/state/state_test.go +++ b/pkg/services/ngalert/state/state_test.go @@ -3,14 +3,17 @@ package state import ( "context" "errors" + "fmt" "math" "math/rand" + "net/url" "testing" "time" "github.com/benbjohnson/clock" "github.com/golang/mock/gomock" "github.com/google/uuid" + "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/prometheus/common/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -801,3 +804,331 @@ func TestGetRuleExtraLabels(t *testing.T) { }) } } + +func TestNewState(t *testing.T) { + url := &url.URL{ + Scheme: "http", + Host: "localhost:3000", + Path: "/test", + } + l := log.New("test") + + gen := ngmodels.RuleGen + generateRule := gen.With(gen.WithNotEmptyLabels(5, "rule-")).GenerateRef + + t.Run("should combine all labels", func(t *testing.T) { + rule := generateRule() + + extraLabels := ngmodels.GenerateAlertLabels(5, "extra-") + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + state := newState(context.Background(), l, rule, result, extraLabels, url) + for key, expected := range extraLabels { + require.Equal(t, expected, state.Labels[key]) + } + assert.Len(t, state.Labels, len(extraLabels)+len(rule.Labels)+len(result.Instance)) + for key, expected := range extraLabels { + assert.Equal(t, expected, state.Labels[key]) + } + for key, expected := range rule.Labels { + assert.Equal(t, expected, state.Labels[key]) + } + for key, expected := range result.Instance { + assert.Equal(t, expected, state.Labels[key]) + } + }) + t.Run("extra labels should take precedence over rule and result labels", func(t *testing.T) { + rule := generateRule() + + extraLabels := ngmodels.GenerateAlertLabels(2, "extra-") + + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + for key := range extraLabels { + rule.Labels[key] = "rule-" + util.GenerateShortUID() + result.Instance[key] = "result-" + util.GenerateShortUID() + } + + state := newState(context.Background(), l, rule, result, extraLabels, url) + for key, expected := range extraLabels { + require.Equal(t, expected, state.Labels[key]) + } + }) + t.Run("rule labels should take precedence over result labels", func(t *testing.T) { + rule := generateRule() + + extraLabels := ngmodels.GenerateAlertLabels(2, "extra-") + + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + for key := range rule.Labels { + result.Instance[key] = "result-" + util.GenerateShortUID() + } + state := newState(context.Background(), l, rule, result, extraLabels, url) + for key, expected := range rule.Labels { + require.Equal(t, expected, state.Labels[key]) + } + }) + t.Run("rule labels should be able to be expanded with result and extra labels", func(t *testing.T) { + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + rule := generateRule() + + extraLabels := ngmodels.GenerateAlertLabels(2, "extra-") + + labelTemplates := make(data.Labels) + for key := range extraLabels { + labelTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) + } + for key := range result.Instance { + labelTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) + } + rule.Labels = labelTemplates + + state := newState(context.Background(), l, rule, result, extraLabels, url) + for key, expected := range extraLabels { + assert.Equal(t, expected, state.Labels["rule-"+key]) + } + for key, expected := range result.Instance { + assert.Equal(t, expected, state.Labels["rule-"+key]) + } + }) + t.Run("rule annotations should be able to be expanded with result and extra labels", func(t *testing.T) { + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + + rule := generateRule() + + extraLabels := ngmodels.GenerateAlertLabels(2, "extra-") + + annotationTemplates := make(data.Labels) + for key := range extraLabels { + annotationTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) + } + for key := range result.Instance { + annotationTemplates["rule-"+key] = fmt.Sprintf("{{ with (index .Labels \"%s\") }}{{.}}{{end}}", key) + } + rule.Annotations = annotationTemplates + + state := newState(context.Background(), l, rule, result, extraLabels, url) + for key, expected := range extraLabels { + assert.Equal(t, expected, state.Annotations["rule-"+key]) + } + for key, expected := range result.Instance { + assert.Equal(t, expected, state.Annotations["rule-"+key]) + } + }) + t.Run("when result labels collide with system labels from LabelsUserCannotSpecify", func(t *testing.T) { + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + m := ngmodels.LabelsUserCannotSpecify + t.Cleanup(func() { + ngmodels.LabelsUserCannotSpecify = m + }) + + ngmodels.LabelsUserCannotSpecify = map[string]struct{}{ + "__label1__": {}, + "label2__": {}, + "__label3": {}, + "label4": {}, + } + result.Instance["__label1__"] = uuid.NewString() + result.Instance["label2__"] = uuid.NewString() + result.Instance["__label3"] = uuid.NewString() + result.Instance["label4"] = uuid.NewString() + + rule := generateRule() + + state := newState(context.Background(), l, rule, result, nil, url) + + for key := range ngmodels.LabelsUserCannotSpecify { + assert.NotContains(t, state.Labels, key) + } + assert.Contains(t, state.Labels, "label1") + assert.Equal(t, state.Labels["label1"], result.Instance["__label1__"]) + + assert.Contains(t, state.Labels, "label2") + assert.Equal(t, state.Labels["label2"], result.Instance["label2__"]) + + assert.Contains(t, state.Labels, "label3") + assert.Equal(t, state.Labels["label3"], result.Instance["__label3"]) + + assert.Contains(t, state.Labels, "label4_user") + assert.Equal(t, state.Labels["label4_user"], result.Instance["label4"]) + + t.Run("should drop label if renamed collides with existing", func(t *testing.T) { + result.Instance["label1"] = uuid.NewString() + result.Instance["label1_user"] = uuid.NewString() + result.Instance["label4_user"] = uuid.NewString() + + state = newState(context.Background(), l, rule, result, nil, url) + assert.NotContains(t, state.Labels, "__label1__") + assert.Contains(t, state.Labels, "label1") + assert.Equal(t, state.Labels["label1"], result.Instance["label1"]) + assert.Equal(t, state.Labels["label1_user"], result.Instance["label1_user"]) + + assert.NotContains(t, state.Labels, "label4") + assert.Equal(t, state.Labels["label4_user"], result.Instance["label4_user"]) + }) + }) + + t.Run("creates a state with preset fields if there is no current state", func(t *testing.T) { + rule := generateRule() + + extraLabels := ngmodels.GenerateAlertLabels(2, "extra-") + + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + + expectedLbl, expectedAnn := expandAnnotationsAndLabels(context.Background(), l, rule, result, extraLabels, url) + + state := newState(context.Background(), l, rule, result, extraLabels, url) + + assert.Equal(t, rule.OrgID, state.OrgID) + assert.Equal(t, rule.UID, state.AlertRuleUID) + assert.Equal(t, state.Labels.Fingerprint(), state.CacheID) + assert.Equal(t, result.State, state.State) + assert.Equal(t, "", state.StateReason) + assert.Equal(t, result.Instance.Fingerprint(), state.ResultFingerprint) + assert.Nil(t, state.LatestResult) + assert.Nil(t, state.Error) + assert.Nil(t, state.Image) + assert.EqualValues(t, expectedAnn, state.Annotations) + assert.EqualValues(t, expectedLbl, state.Labels) + assert.Nil(t, state.Values) + assert.Equal(t, result.EvaluatedAt, state.StartsAt) + assert.Equal(t, result.EvaluatedAt, state.EndsAt) + assert.Nil(t, state.ResolvedAt) + assert.Nil(t, state.LastSentAt) + assert.Equal(t, "", state.LastEvaluationString) + assert.Equal(t, result.EvaluatedAt, state.LastEvaluationTime) + assert.Equal(t, result.EvaluationDuration, state.EvaluationDuration) + }) +} + +func TestPatch(t *testing.T) { + key := ngmodels.GenerateRuleKey(1) + t.Run("it populates some fields from the current state if it exists", func(t *testing.T) { + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + + state := randomSate(key) + orig := state.Copy() + current := randomSate(key) + + patch(&state, ¤t, result) + + // Fields that should not change + assert.Equal(t, orig.OrgID, state.OrgID) + assert.Equal(t, orig.AlertRuleUID, state.AlertRuleUID) + assert.Equal(t, orig.CacheID, state.CacheID) + assert.Equal(t, orig.ResultFingerprint, state.ResultFingerprint) + assert.EqualValues(t, orig.Annotations, state.Annotations) + assert.EqualValues(t, orig.Labels, state.Labels) + assert.Equal(t, orig.LastEvaluationTime, state.LastEvaluationTime) + assert.Equal(t, orig.EvaluationDuration, state.EvaluationDuration) + + assert.Equal(t, current.State, state.State) + assert.Equal(t, current.StateReason, state.StateReason) + assert.Equal(t, current.Image, state.Image) + assert.Equal(t, current.LatestResult, state.LatestResult) + assert.Equal(t, current.Error, state.Error) + assert.Equal(t, current.Values, state.Values) + assert.Equal(t, current.StartsAt, state.StartsAt) + assert.Equal(t, current.EndsAt, state.EndsAt) + assert.Equal(t, current.ResolvedAt, state.ResolvedAt) + assert.Equal(t, current.LastSentAt, state.LastSentAt) + assert.Equal(t, current.LastEvaluationString, state.LastEvaluationString) + }) + + t.Run("copies system-owned annotations from current state", func(t *testing.T) { + state := randomSate(key) + orig := state.Copy() + expectedAnnotations := data.Labels(state.Annotations).Copy() + current := randomSate(key) + + for key := range ngmodels.InternalAnnotationNameSet { + val := util.GenerateShortUID() + current.Annotations[key] = val + expectedAnnotations[key] = val + } + + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + } + + patch(&state, ¤t, result) + + assert.EqualValues(t, expectedAnnotations, state.Annotations) + assert.Equal(t, current.State, state.State) + assert.Equal(t, current.StateReason, state.StateReason) + assert.Equal(t, current.Image, state.Image) + assert.Equal(t, current.LatestResult, state.LatestResult) + assert.Equal(t, current.Error, state.Error) + assert.Equal(t, current.Values, state.Values) + assert.Equal(t, current.StartsAt, state.StartsAt) + assert.Equal(t, current.EndsAt, state.EndsAt) + assert.Equal(t, current.ResolvedAt, state.ResolvedAt) + assert.Equal(t, current.LastSentAt, state.LastSentAt) + assert.Equal(t, current.LastEvaluationString, state.LastEvaluationString) + + // Fields that should not change + assert.Equal(t, orig.OrgID, state.OrgID) + assert.Equal(t, orig.AlertRuleUID, state.AlertRuleUID) + assert.Equal(t, orig.CacheID, state.CacheID) + assert.Equal(t, orig.ResultFingerprint, state.ResultFingerprint) + assert.EqualValues(t, orig.Labels, state.Labels) + assert.Equal(t, orig.LastEvaluationTime, state.LastEvaluationTime) + assert.Equal(t, orig.EvaluationDuration, state.EvaluationDuration) + }) + + t.Run("if result Error and current state is Error it should copy datasource_uid and ref_id labels", func(t *testing.T) { + state := randomSate(key) + orig := state.Copy() + current := randomSate(key) + current.State = eval.Error + current.Labels["datasource_uid"] = util.GenerateShortUID() + current.Labels["ref_id"] = util.GenerateShortUID() + + result := eval.Result{ + Instance: ngmodels.GenerateAlertLabels(5, "result-"), + State: eval.Error, + } + + expectedLabels := orig.Labels.Copy() + expectedLabels["datasource_uid"] = current.Labels["datasource_uid"] + expectedLabels["ref_id"] = current.Labels["ref_id"] + + patch(&state, ¤t, result) + + assert.Equal(t, expectedLabels, state.Labels) + assert.Equal(t, current.State, state.State) + assert.Equal(t, current.StateReason, state.StateReason) + assert.Equal(t, current.Image, state.Image) + assert.Equal(t, current.LatestResult, state.LatestResult) + assert.Equal(t, current.Error, state.Error) + assert.Equal(t, current.Values, state.Values) + assert.Equal(t, current.StartsAt, state.StartsAt) + assert.Equal(t, current.EndsAt, state.EndsAt) + assert.Equal(t, current.ResolvedAt, state.ResolvedAt) + assert.Equal(t, current.LastSentAt, state.LastSentAt) + assert.Equal(t, current.LastEvaluationString, state.LastEvaluationString) + + // Fields that should not change + assert.Equal(t, orig.OrgID, state.OrgID) + assert.Equal(t, orig.AlertRuleUID, state.AlertRuleUID) + assert.Equal(t, orig.CacheID, state.CacheID) + assert.Equal(t, orig.ResultFingerprint, state.ResultFingerprint) + assert.Equal(t, orig.LastEvaluationTime, state.LastEvaluationTime) + assert.Equal(t, orig.EvaluationDuration, state.EvaluationDuration) + assert.EqualValues(t, orig.Annotations, state.Annotations) + }) +} diff --git a/public/app/core/utils/object.test.ts b/public/app/core/utils/object.test.ts index 8e27feccc6b..c84f7d71d9e 100644 --- a/public/app/core/utils/object.test.ts +++ b/public/app/core/utils/object.test.ts @@ -7,6 +7,7 @@ describe('objects', () => { deeper: 10, foo: null, arr: [null, 1, 'hello'], + value: -Infinity, }, bar: undefined, simple: 'A', diff --git a/public/app/core/utils/object.ts b/public/app/core/utils/object.ts index a5cccf419c2..a51d593b635 100644 --- a/public/app/core/utils/object.ts +++ b/public/app/core/utils/object.ts @@ -1,16 +1,17 @@ import { isArray, isPlainObject } from 'lodash'; /** @returns a deep clone of the object, but with any null value removed */ -export function sortedDeepCloneWithoutNulls(value: T): T { +export function sortedDeepCloneWithoutNulls(value: T): T { if (isArray(value)) { return value.map(sortedDeepCloneWithoutNulls) as unknown as T; } if (isPlainObject(value)) { - return Object.keys(value) + return Object.keys(value as { [key: string]: any }) .sort() .reduce((acc: any, key) => { const v = (value as any)[key]; - if (v != null) { + // Remove null values and also -Infinity which is not a valid JSON value + if (v != null && v !== -Infinity) { acc[key] = sortedDeepCloneWithoutNulls(v); } return acc; diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx b/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx index e95ede9eebc..9f02500d65e 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx +++ b/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx @@ -40,7 +40,8 @@ export function DashboardScenePage({ route, queryParams, location }: Props) { } else { stateManager.loadDashboard({ uid: (route.routeName === DashboardRoutes.Provisioning ? path : uid) ?? '', - slug: slug, + type, + slug, route: route.routeName as DashboardRoutes, urlFolderUid: queryParams.folderUid, }); diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts index db5a64048c7..9426cf3e7a6 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts +++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts @@ -267,11 +267,12 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag } public async fetchDashboard({ + type, + slug, uid, route, urlFolderUid, params, - slug, }: LoadDashboardOptions): Promise { const cacheKey = route === DashboardRoutes.Home ? HOME_DASHBOARD_CACHE_KEY : uid; @@ -328,7 +329,7 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag } : undefined; - rsp = await dashboardLoaderSrv.loadDashboard('db', '', uid, queryParams); + rsp = await dashboardLoaderSrv.loadDashboard(type || 'db', slug || '', uid, queryParams); if (route === DashboardRoutes.Embedded) { rsp.meta.isEmbedded = true; @@ -482,6 +483,8 @@ export class DashboardScenePageStateManagerV2 extends DashboardScenePageStateMan } public async fetchDashboard({ + type, + slug, uid, route, urlFolderUid, @@ -534,7 +537,7 @@ export class DashboardScenePageStateManagerV2 extends DashboardScenePageStateMan ...params.variables, } : undefined; - rsp = await this.dashboardLoader.loadDashboard('db', '', uid, queryParams); + rsp = await this.dashboardLoader.loadDashboard(type || 'db', slug || '', uid, queryParams); if (route === DashboardRoutes.Embedded) { throw new Error('Method not implemented.'); // rsp.meta.isEmbedded = true; diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index 3f9c1b5910a..d760f81f225 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.tsx @@ -30,6 +30,7 @@ import { ScrollRefElement } from 'app/core/components/NativeScrollbar'; import { LS_PANEL_COPY_KEY } from 'app/core/constants'; import { getNavModel } from 'app/core/selectors/navModel'; import store from 'app/core/store'; +import { sortedDeepCloneWithoutNulls } from 'app/core/utils/object'; import { DashboardWithAccessInfo } from 'app/features/dashboard/api/types'; import { SaveDashboardAsOptions } from 'app/features/dashboard/components/SaveDashboard/types'; import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; @@ -675,7 +676,7 @@ export class DashboardScene extends SceneObjectBase { saveModel?: Dashboard | DashboardV2Spec, meta?: DashboardMeta | DashboardWithAccessInfo['metadata'] ): void { - this._serializer.initialSaveModel = saveModel; + this._serializer.initialSaveModel = sortedDeepCloneWithoutNulls(saveModel); this._serializer.metadata = meta; } diff --git a/public/app/features/dashboard-scene/serialization/DashboardSceneSerializer.test.ts b/public/app/features/dashboard-scene/serialization/DashboardSceneSerializer.test.ts index 747c69eaf07..74d58c10719 100644 --- a/public/app/features/dashboard-scene/serialization/DashboardSceneSerializer.test.ts +++ b/public/app/features/dashboard-scene/serialization/DashboardSceneSerializer.test.ts @@ -36,6 +36,26 @@ jest.mock('@grafana/runtime', () => ({ getInstanceSettings: jest.fn(), }; }, + config: { + ...jest.requireActual('@grafana/runtime').config, + bootData: { + settings: { + defaultDatasource: '-- Grafana --', + datasources: { + '-- Grafana --': { + name: 'Grafana', + meta: { id: 'grafana' }, + type: 'datasource', + }, + prometheus: { + name: 'prometheus', + meta: { id: 'prometheus' }, + type: 'datasource', + }, + }, + }, + }, + }, })); describe('DashboardSceneSerializer', () => { diff --git a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.test.ts b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.test.ts index 90033cc9856..108a1e1d159 100644 --- a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.test.ts +++ b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.test.ts @@ -293,7 +293,7 @@ describe('transformSaveModelSchemaV2ToScene', () => { expect(getQueryRunnerFor(vizPanels[0])?.state.datasource?.uid).toBe(MIXED_DATASOURCE_NAME); }); - it('should set panel ds as undefined if it is not mixed DS', () => { + it('should set ds if it is not mixed DS', () => { const dashboard = cloneDeep(defaultDashboard); getPanelElement(dashboard.spec, 'panel-1')?.spec.data.spec.queries.push({ kind: 'PanelQuery', @@ -317,10 +317,13 @@ describe('transformSaveModelSchemaV2ToScene', () => { const vizPanels = (scene.state.body as DashboardLayoutManager).getVizPanels(); expect(vizPanels.length).toBe(3); - expect(getQueryRunnerFor(vizPanels[0])?.state.datasource).toBeUndefined(); + expect(getQueryRunnerFor(vizPanels[0])?.state.queries[0].datasource).toEqual({ + type: 'prometheus', + uid: 'datasource1', + }); }); - it('should set panel ds as mixed if one ds is undefined', () => { + it('should set panel ds as mixed if no panels have ds defined', () => { const dashboard = cloneDeep(defaultDashboard); getPanelElement(dashboard.spec, 'panel-1')?.spec.data.spec.queries.push({ diff --git a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts index a8f97ad4287..40b5b01fbf9 100644 --- a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts +++ b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts @@ -234,7 +234,7 @@ function getPanelDataSource(panel: PanelKind): DataSourceRef | undefined { } }); - return isMixedDatasource ? { type: 'mixed', uid: MIXED_DATASOURCE_NAME } : undefined; + return isMixedDatasource ? { type: 'mixed', uid: MIXED_DATASOURCE_NAME } : datasource; } function panelQueryKindToSceneQuery(query: PanelQueryKind): SceneDataQuery { diff --git a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts index 979fba6483c..3672567fdb9 100644 --- a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts +++ b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts @@ -11,6 +11,7 @@ import { VizPanel, } from '@grafana/scenes'; import { DataSourceRef } from '@grafana/schema'; +import { sortedDeepCloneWithoutNulls } from 'app/core/utils/object'; import { DashboardV2Spec, @@ -73,7 +74,7 @@ export function transformSceneToSaveModelSchemaV2(scene: DashboardScene, isSnaps const dashboardSchemaV2: DeepPartial = { //dashboard settings title: sceneDash.title, - description: sceneDash.description ?? '', + description: sceneDash.description, cursorSync: getCursorSync(sceneDash), liveNow: getLiveNow(sceneDash), preload: sceneDash.preload, @@ -116,7 +117,7 @@ export function transformSceneToSaveModelSchemaV2(scene: DashboardScene, isSnaps try { // validateDashboardSchemaV2 will throw an error if the dashboard is not valid if (validateDashboardSchemaV2(dashboardSchemaV2)) { - return dashboardSchemaV2; + return sortedDeepCloneWithoutNulls(dashboardSchemaV2); } // should never reach this point, validation should throw an error throw new Error('Error we could transform the dashboard to schema v2: ' + dashboardSchemaV2); @@ -241,7 +242,7 @@ function getVizPanelQueries(vizPanel: VizPanel): PanelQueryKind[] { const queries: PanelQueryKind[] = []; const queryRunner = getQueryRunnerFor(vizPanel); const vizPanelQueries = queryRunner?.state.queries; - const datasource = queryRunner?.state.datasource; + const datasource = queryRunner?.state.datasource ?? getDefaultDataSourceRef(); if (vizPanelQueries) { vizPanelQueries.forEach((query) => { @@ -250,7 +251,7 @@ function getVizPanelQueries(vizPanel: VizPanel): PanelQueryKind[] { spec: omit(query, 'datasource', 'refId', 'hide'), }; const querySpec: PanelQuerySpec = { - datasource: datasource ?? getDefaultDataSourceRef(), + datasource: query.datasource ?? datasource, query: dataQuery, refId: query.refId, hidden: Boolean(query.hide), @@ -446,7 +447,7 @@ function validateDashboardSchemaV2(dash: unknown): dash is DashboardV2Spec { if ('title' in dash && typeof dash.title !== 'string') { throw new Error('Title is not a string'); } - if ('description' in dash && typeof dash.description !== 'string') { + if ('description' in dash && dash.description !== undefined && typeof dash.description !== 'string') { throw new Error('Description is not a string'); } if ('cursorSync' in dash && typeof dash.cursorSync !== 'string') { diff --git a/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx b/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx index 265d32c9e72..01894fe9d9e 100644 --- a/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx +++ b/public/app/features/dashboard-scene/solo/SoloPanelPage.tsx @@ -26,12 +26,12 @@ export interface Props extends GrafanaRouteComponentProps { - stateManager.loadDashboard({ uid, route: DashboardRoutes.Embedded }); + stateManager.loadDashboard({ uid, type, slug, route: DashboardRoutes.Embedded }); return () => stateManager.clearState(); - }, [stateManager, queryParams, uid]); + }, [stateManager, queryParams, uid, type, slug]); if (!queryParams.panelId) { return ; @@ -64,7 +64,6 @@ export function SoloPanelRenderer({ dashboard, panelId }: { dashboard: Dashboard const [panel, error] = useSoloPanel(dashboard, panelId); const { controls } = dashboard.useState(); const refreshPicker = controls?.useState()?.refreshPicker; - const styles = useStyles2(getStyles); useEffect(() => { diff --git a/public/app/features/dashboard/api/ResponseTransformers.test.ts b/public/app/features/dashboard/api/ResponseTransformers.test.ts index c598db36084..4a7a29cf711 100644 --- a/public/app/features/dashboard/api/ResponseTransformers.test.ts +++ b/public/app/features/dashboard/api/ResponseTransformers.test.ts @@ -487,7 +487,7 @@ describe('ResponseTransformers', () => { expect(layout.spec.items[0].spec).toEqual({ element: { kind: 'ElementReference', - name: '1', + name: 'panel-1', }, x: 0, y: 0, @@ -495,7 +495,7 @@ describe('ResponseTransformers', () => { height: 8, repeat: { value: 'var1', direction: 'h', mode: 'variable', maxPerRow: undefined }, }); - expect(spec.elements['1']).toEqual({ + expect(spec.elements['panel-1']).toEqual({ kind: 'Panel', spec: { title: 'Panel Title', @@ -550,14 +550,14 @@ describe('ResponseTransformers', () => { expect(layout.spec.items[1].spec).toEqual({ element: { kind: 'ElementReference', - name: '2', + name: 'panel-2', }, x: 0, y: 8, width: 12, height: 8, }); - expect(spec.elements['2']).toEqual({ + expect(spec.elements['panel-2']).toEqual({ kind: 'LibraryPanel', spec: { libraryPanel: { @@ -580,7 +580,7 @@ describe('ResponseTransformers', () => { expect(panelInRow).toEqual({ element: { kind: 'ElementReference', - name: '4', + name: 'panel-4', }, x: 0, y: 0, @@ -598,7 +598,7 @@ describe('ResponseTransformers', () => { expect(panelInCollapsedRow).toEqual({ element: { kind: 'ElementReference', - name: '5', + name: 'panel-5', }, x: 0, y: 0, diff --git a/public/app/features/dashboard/api/ResponseTransformers.ts b/public/app/features/dashboard/api/ResponseTransformers.ts index 69edee427e4..3d810f3eedf 100644 --- a/public/app/features/dashboard/api/ResponseTransformers.ts +++ b/public/app/features/dashboard/api/ResponseTransformers.ts @@ -383,7 +383,7 @@ function buildElement(p: Panel): [PanelKind | LibraryPanelKind, string] { }, }; - return [panelKind, p.id!.toString()]; + return [panelKind, `panel-${p.id}`]; } else { // PanelKind @@ -433,7 +433,7 @@ function buildElement(p: Panel): [PanelKind | LibraryPanelKind, string] { }, }; - return [panelKind, p.id!.toString()]; + return [panelKind, `panel-${p.id}`]; } } diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.tsx index 52d91d5d956..1570fbc6ee7 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.tsx @@ -51,6 +51,7 @@ function DashboardPageProxy(props: DashboardPageProxyProps) { route: props.route.routeName as DashboardRoutes, uid: params.uid ?? '', type: params.type, + slug: params.slug, }); }, [params.uid, props.route.routeName]); diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts index f640cc8cbb6..c54e1743f96 100644 --- a/public/app/features/dashboard/state/DashboardModel.ts +++ b/public/app/features/dashboard/state/DashboardModel.ts @@ -286,6 +286,8 @@ export class DashboardModel implements TimeModel { * * @internal and experimental */ + // TODO: remove this as it's not being used anymore + // Also remove public/app/features/dashboard/utils/panelMerge.ts updatePanels(panels: IPanelModel[]): PanelMergeInfo { const info = mergePanels(this.panels, panels ?? []); if (info.changed) { diff --git a/public/app/features/dashboard/utils/panelMerge.test.ts b/public/app/features/dashboard/utils/panelMerge.test.ts index 4bc201aa8ad..51536b2d426 100644 --- a/public/app/features/dashboard/utils/panelMerge.test.ts +++ b/public/app/features/dashboard/utils/panelMerge.test.ts @@ -4,7 +4,8 @@ import { FieldColorModeId, ThresholdsMode } from '@grafana/schema/src'; import { DashboardModel } from '../state/DashboardModel'; import { createDashboardModelFixture, createPanelSaveModel } from '../state/__fixtures__/dashboardFixtures'; -describe('Merge dashboard panels', () => { +// skipping these tests because panelMerge is not used +describe.skip('Merge dashboard panels', () => { describe('simple changes', () => { let dashboard: DashboardModel; let rawPanels: PanelModel[]; diff --git a/public/app/routes/routes.tsx b/public/app/routes/routes.tsx index 62f6dc3ce83..0847577522f 100644 --- a/public/app/routes/routes.tsx +++ b/public/app/routes/routes.tsx @@ -97,8 +97,10 @@ export function getAppRoutes(): RouteDescriptor[] { path: '/dashboard-solo/:type/:slug', routeName: DashboardRoutes.Normal, chromeless: true, - component: SafeDynamicImport( - () => import(/* webpackChunkName: "SoloPanelPage" */ '../features/dashboard/containers/SoloPanelPage') + component: SafeDynamicImport(() => + config.featureToggles.dashboardSceneSolo + ? import(/* webpackChunkName: "SoloPanelPage" */ '../features/dashboard-scene/solo/SoloPanelPage') + : import(/* webpackChunkName: "SoloPanelPageOld" */ '../features/dashboard/containers/SoloPanelPage') ), }, { diff --git a/public/dashboards/scripted.js b/public/dashboards/scripted.js index daa4c4efb70..03de55bff44 100644 --- a/public/dashboards/scripted.js +++ b/public/dashboards/scripted.js @@ -13,16 +13,11 @@ 'use strict'; -// accessible variables in this scope -let window, document, $, jQuery, moment, kbn; +// accessible variables in this scope: window, document, $, jQuery, moment, kbn; // Setup some variables let dashboard; -// All url parameters are available via the ARGS object -// eslint-disable-next-line no-redeclare -let ARGS; - // Initialize a skeleton with nothing but a rows array and service object dashboard = { rows: [], @@ -56,6 +51,7 @@ for (let i = 0; i < rows; i++) { height: '300px', panels: [ { + id: 1, title: 'Events', type: 'graph', span: 12, @@ -63,23 +59,17 @@ for (let i = 0; i < rows; i++) { linewidth: 2, targets: [ { - target: "randomWalk('" + seriesName + "')", + scenarioId: 'random_walk', + refId: 'A', + seriesCount: 1, + alias: seriesName, }, { - target: "randomWalk('random walk2')", + scenarioId: 'random_walk', + refId: 'B', + seriesCount: 1, }, ], - seriesOverrides: [ - { - alias: '/random/', - yaxis: 2, - fill: 0, - linewidth: 5, - }, - ], - tooltip: { - shared: true, - }, }, ], }); diff --git a/public/dashboards/scripted_async.js b/public/dashboards/scripted_async.js index 98fb144d243..d13a80eb034 100644 --- a/public/dashboards/scripted_async.js +++ b/public/dashboards/scripted_async.js @@ -17,7 +17,7 @@ 'use strict'; // accessible variables in this scope -let window, document, ARGS, $, jQuery, moment, kbn; +// let window, document, ARGS, $, jQuery, moment, kbn; return function (callback) { // Setup some variables @@ -60,6 +60,7 @@ return function (callback) { height: '300px', panels: [ { + id: 1, title: 'Async dashboard test', type: 'text', span: 12, diff --git a/public/dashboards/scripted_templated.js b/public/dashboards/scripted_templated.js index df8647ff86e..0b09fa50986 100644 --- a/public/dashboards/scripted_templated.js +++ b/public/dashboards/scripted_templated.js @@ -14,14 +14,14 @@ 'use strict'; // accessible variables in this scope -let window, document, $, jQuery, moment, kbn; +// let window, document, $, jQuery, moment, kbn; // Setup some variables let dashboard; // All url parameters are available via the ARGS object // eslint-disable-next-line no-redeclare -let ARGS; +// let ARGS; // Initialize a skeleton with nothing but a rows array and service object dashboard = { @@ -44,19 +44,22 @@ dashboard.templating = { list: [ { name: 'test', - query: 'apps.backend.*', - refresh: 1, - type: 'query', - datasource: null, hide: 2, + includeAll: false, + multi: false, + query: 'a,b,c\n', + skipUrlSync: false, + type: 'custom', }, { - name: 'test2', - query: '*', - refresh: 1, - type: 'query', - datasource: null, - hide: 2, + name: 'seriesName', + label: 'Series name', + hide: 0, + includeAll: false, + multi: false, + query: 'series1,series2,series3\n', + skipUrlSync: false, + type: 'custom', }, ], }; @@ -78,6 +81,7 @@ for (let i = 0; i < rows; i++) { height: '300px', panels: [ { + id: 1, title: 'Events', type: 'graph', span: 12, @@ -85,10 +89,16 @@ for (let i = 0; i < rows; i++) { linewidth: 2, targets: [ { - target: "randomWalk('" + seriesName + "')", + scenarioId: 'random_walk', + refId: 'A', + seriesCount: 1, + alias: seriesName, }, { - target: "randomWalk('[[test2]]')", + scenarioId: 'random_walk', + refId: 'B', + seriesCount: 1, + alias: '${seriesName}', }, ], },