From a3e85d831995014bdc43c0e2058fdd982147eed4 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Thu, 12 Jun 2025 17:37:07 +0200 Subject: [PATCH 01/51] Advisor: Fix issues (#106612) --- apps/advisor/pkg/app/app.go | 4 +- .../pkg/app/checkscheduler/checkscheduler.go | 27 ++++++- .../app/checkscheduler/checkscheduler_test.go | 71 +++++++++++++------ 3 files changed, 76 insertions(+), 26 deletions(-) diff --git a/apps/advisor/pkg/app/app.go b/apps/advisor/pkg/app/app.go index b647e86a964..ba84a14f7f7 100644 --- a/apps/advisor/pkg/app/app.go +++ b/apps/advisor/pkg/app/app.go @@ -71,7 +71,7 @@ func New(cfg app.Config) (app.App, error) { logger.Error("Error getting requester", "error", err) return } - ctx = identity.WithRequester(context.Background(), requester) + ctx = identity.WithServiceIdentityContext(context.WithoutCancel(ctx), requester.GetOrgID()) err = processCheck(ctx, logger, client, typesClient, req.Object, check) if err != nil { logger.Error("Error processing check", "error", err) @@ -87,7 +87,7 @@ func New(cfg app.Config) (app.App, error) { logger.Error("Error getting requester", "error", err) return } - ctx = identity.WithRequester(context.Background(), requester) + ctx = identity.WithServiceIdentityContext(context.WithoutCancel(ctx), requester.GetOrgID()) err = processCheckRetry(ctx, logger, client, typesClient, req.Object, check) if err != nil { logger.Error("Error processing check retry", "error", err) diff --git a/apps/advisor/pkg/app/checkscheduler/checkscheduler.go b/apps/advisor/pkg/app/checkscheduler/checkscheduler.go index b8bf81299bf..adf0cc1569d 100644 --- a/apps/advisor/pkg/app/checkscheduler/checkscheduler.go +++ b/apps/advisor/pkg/app/checkscheduler/checkscheduler.go @@ -22,10 +22,16 @@ import ( const defaultEvaluationInterval = 7 * 24 * time.Hour // 7 days const defaultMaxHistory = 10 +var ( + waitInterval = 5 * time.Second + waitMaxRetries = 3 +) + // Runner is a "runnable" app used to be able to expose and API endpoint // with the existing checks types. This does not need to be a CRUD resource, but it is // the only way existing at the moment to expose the check types. type Runner struct { + checkRegistry checkregistry.CheckService client resource.Client typesClient resource.Client evaluationInterval time.Duration @@ -41,6 +47,7 @@ func New(cfg app.Config, log logging.Logger) (app.Runnable, error) { if !ok { return nil, fmt.Errorf("invalid config type") } + checkRegistry := specificConfig.CheckRegistry evalInterval, err := getEvaluationInterval(specificConfig.PluginConfig) if err != nil { return nil, err @@ -66,6 +73,7 @@ func New(cfg app.Config, log logging.Logger) (app.Runnable, error) { } return &Runner{ + checkRegistry: checkRegistry, client: client, typesClient: typesClient, evaluationInterval: evalInterval, @@ -85,7 +93,7 @@ func (r *Runner) Run(ctx context.Context) error { } else { // do an initial creation if necessary if lastCreated.IsZero() { - err = r.createChecks(ctx) + err = r.createChecks(ctx, logger) if err != nil { logger.Error("Error creating new check reports", "error", err) } else { @@ -101,7 +109,7 @@ func (r *Runner) Run(ctx context.Context) error { for { select { case <-ticker.C: - err = r.createChecks(ctx) + err = r.createChecks(ctx, logger) if err != nil { logger.Error("Error creating new check reports", "error", err) } @@ -150,12 +158,25 @@ func (r *Runner) checkLastCreated(ctx context.Context, log logging.Logger) (time } // createChecks creates a new check for each check type in the registry. -func (r *Runner) createChecks(ctx context.Context) error { +func (r *Runner) createChecks(ctx context.Context, logger logging.Logger) error { // List existing CheckType objects list, err := r.typesClient.List(ctx, r.namespace, resource.ListOptions{}) if err != nil { return fmt.Errorf("error listing check types: %w", err) } + // This may be run before the check types are registered, so we need to wait for them to be registered. + allChecksRegistered := len(list.GetItems()) == len(r.checkRegistry.Checks()) + retryCount := 0 + for !allChecksRegistered && retryCount < waitMaxRetries { + logger.Error("Waiting for all check types to be registered", "retryCount", retryCount, "waitInterval", waitInterval) + time.Sleep(waitInterval) + list, err = r.typesClient.List(ctx, r.namespace, resource.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing check types: %w", err) + } + allChecksRegistered = len(list.GetItems()) == len(r.checkRegistry.Checks()) + retryCount++ + } // Create checks for each CheckType for _, item := range list.GetItems() { diff --git a/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go b/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go index b699b50e7b7..69b4e809b4c 100644 --- a/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go +++ b/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go @@ -36,7 +36,7 @@ func TestRunner_Run(t *testing.T) { runner := &Runner{ client: mockClient, typesClient: mockTypesClient, - log: logging.DefaultLogger, + log: &logging.NoOpLogger{}, evaluationInterval: 1 * time.Hour, } @@ -56,10 +56,10 @@ func TestRunner_checkLastCreated_ErrorOnList(t *testing.T) { runner := &Runner{ client: mockClient, - log: logging.DefaultLogger, + log: &logging.NoOpLogger{}, } - lastCreated, err := runner.checkLastCreated(context.Background(), logging.DefaultLogger) + lastCreated, err := runner.checkLastCreated(context.Background(), &logging.NoOpLogger{}) assert.Error(t, err) assert.True(t, lastCreated.IsZero()) } @@ -89,10 +89,10 @@ func TestRunner_checkLastCreated_UnprocessedCheck(t *testing.T) { runner := &Runner{ client: mockClient, - log: logging.DefaultLogger, + log: &logging.NoOpLogger{}, } - lastCreated, err := runner.checkLastCreated(context.Background(), logging.DefaultLogger) + lastCreated, err := runner.checkLastCreated(context.Background(), &logging.NoOpLogger{}) assert.NoError(t, err) assert.True(t, lastCreated.IsZero()) assert.Equal(t, "check-1", identifier.Name) @@ -104,6 +104,8 @@ func TestRunner_checkLastCreated_UnprocessedCheck(t *testing.T) { } func TestRunner_createChecks_ErrorOnCreate(t *testing.T) { + mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "check-1"}}} + mockClient := &MockClient{ createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) { return nil, errors.New("create error") @@ -121,16 +123,19 @@ func TestRunner_createChecks_ErrorOnCreate(t *testing.T) { } runner := &Runner{ - client: mockClient, - typesClient: mockTypesClient, - log: logging.DefaultLogger, + checkRegistry: mockCheckService, + client: mockClient, + typesClient: mockTypesClient, + log: &logging.NoOpLogger{}, } - err := runner.createChecks(context.Background()) + err := runner.createChecks(context.Background(), &logging.NoOpLogger{}) assert.Error(t, err) } func TestRunner_createChecks_Success(t *testing.T) { + mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "check-1"}}} + mockClient := &MockClient{ createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) { return &advisorv0alpha1.Check{}, nil @@ -148,12 +153,13 @@ func TestRunner_createChecks_Success(t *testing.T) { } runner := &Runner{ - client: mockClient, - typesClient: mockTypesClient, - log: logging.DefaultLogger, + checkRegistry: mockCheckService, + client: mockClient, + typesClient: mockTypesClient, + log: &logging.NoOpLogger{}, } - err := runner.createChecks(context.Background()) + err := runner.createChecks(context.Background(), &logging.NoOpLogger{}) assert.NoError(t, err) } @@ -166,10 +172,10 @@ func TestRunner_cleanupChecks_ErrorOnList(t *testing.T) { runner := &Runner{ client: mockClient, - log: logging.DefaultLogger, + log: &logging.NoOpLogger{}, } - err := runner.cleanupChecks(context.Background(), logging.DefaultLogger) + err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{}) assert.Error(t, err) } @@ -187,10 +193,10 @@ func TestRunner_cleanupChecks_WithinMax(t *testing.T) { runner := &Runner{ client: mockClient, - log: logging.DefaultLogger, + log: &logging.NoOpLogger{}, } - err := runner.cleanupChecks(context.Background(), logging.DefaultLogger) + err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{}) assert.NoError(t, err) } @@ -217,9 +223,9 @@ func TestRunner_cleanupChecks_ErrorOnDelete(t *testing.T) { runner := &Runner{ client: mockClient, maxHistory: defaultMaxHistory, - log: logging.DefaultLogger, + log: &logging.NoOpLogger{}, } - err := runner.cleanupChecks(context.Background(), logging.DefaultLogger) + err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{}) assert.ErrorContains(t, err, "delete error") } @@ -253,9 +259,9 @@ func TestRunner_cleanupChecks_Success(t *testing.T) { runner := &Runner{ client: mockClient, maxHistory: defaultMaxHistory, - log: logging.DefaultLogger, + log: &logging.NoOpLogger{}, } - err := runner.cleanupChecks(context.Background(), logging.DefaultLogger) + err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{}) assert.NoError(t, err) assert.Equal(t, []string{"check-0"}, itemsDeleted) } @@ -334,3 +340,26 @@ func (m *MockClient) Delete(ctx context.Context, identifier resource.Identifier, func (m *MockClient) PatchInto(ctx context.Context, identifier resource.Identifier, patch resource.PatchRequest, options resource.PatchOptions, into resource.Object) error { return m.patchFunc(ctx, identifier, patch, options, into) } + +type MockCheckService struct { + checks []checks.Check +} + +func (m *MockCheckService) Checks() []checks.Check { + return m.checks +} + +type mockCheck struct { + checks.Check + + id string + steps []checks.Step +} + +func (m *mockCheck) ID() string { + return m.id +} + +func (m *mockCheck) Steps() []checks.Step { + return m.steps +} From 45b92f2a980792d4b2a18394596063d83cd9725d Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Thu, 12 Jun 2025 17:43:48 +0200 Subject: [PATCH 02/51] Partner: Add PRs to project (#106641) Add PRs to project --- .github/pr-commands.json | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/.github/pr-commands.json b/.github/pr-commands.json index f9934555782..f97a0738700 100644 --- a/.github/pr-commands.json +++ b/.github/pr-commands.json @@ -413,5 +413,71 @@ ], "action": "updateLabel", "addLabel": "area/panel/table" + }, + { + "type": "changedfiles", + "matches": [ + "public/app/plugins/datasource/azuremonitor/**/*", + "pkg/tsdb/azuremonitor/**/*" + ], + "action": "addToProject", + "addToProject": { + "url": "https://github.com/orgs/grafana/projects/190" + } + }, + { + "type": "changedfiles", + "matches": [ + "public/app/plugins/datasource/graphite/**/*", + "pkg/tsdb/graphite/**/*" + ], + "action": "addToProject", + "addToProject": { + "url": "https://github.com/orgs/grafana/projects/190" + } + }, + { + "type": "changedfiles", + "matches": [ + "public/app/plugins/datasource/influxdb/**/*", + "pkg/tsdb/influx/**/*" + ], + "action": "addToProject", + "addToProject": { + "url": "https://github.com/orgs/grafana/projects/190" + } + }, + { + "type": "changedfiles", + "matches": [ + "public/app/plugins/datasource/elasticsearch/**/*", + "pkg/tsdb/elasticsearch/**/*" + ], + "action": "addToProject", + "addToProject": { + "url": "https://github.com/orgs/grafana/projects/190" + } + }, + { + "type": "changedfiles", + "matches": [ + "public/app/plugins/datasource/cloud-monitoring/**/*", + "pkg/tsdb/cloud-monitoring/**/*" + ], + "action": "addToProject", + "addToProject": { + "url": "https://github.com/orgs/grafana/projects/190" + } + }, + { + "type": "changedfiles", + "matches": [ + "public/app/plugins/datasource/opentsdb/**/*", + "pkg/tsdb/opentsdb/**/*" + ], + "action": "addToProject", + "addToProject": { + "url": "https://github.com/orgs/grafana/projects/190" + } } ] From f02ad33fd22cfd93f081a5668bbf697733e147e2 Mon Sep 17 00:00:00 2001 From: Jacob Valdez Date: Thu, 12 Jun 2025 10:47:13 -0500 Subject: [PATCH 03/51] Docs: adding information on adjusting short link expiration time in Grafana cloud (#106112) * Docs: adding information on adjusting short link expiration time in Grafana cloud * changing admonition and adding info on changing config for cloud * adjusting wording * fixing typo * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Isabel Matwawana <76437239+imatwawana@users.noreply.github.com> --------- Co-authored-by: Isabel Matwawana <76437239+imatwawana@users.noreply.github.com> --- docs/sources/setup-grafana/configure-grafana/_index.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index 5442fd9c46a..14bfd8404ce 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -54,6 +54,10 @@ By default, the configuration file is located at `/opt/homebrew/etc/grafana/graf For a Grafana instance installed using Homebrew, edit the `grafana.ini` file directly. Otherwise, add a configuration file named `custom.ini` to the `conf` directory to override the settings defined in `conf/defaults.ini`. +### Grafana Cloud + +There is no local configuration file for Grafana Cloud stacks, but many of these settings are still configurable. To edit configurable settings, open a support ticket. + ## Remove comments in the .ini files Grafana uses semicolons (`;`) to comment out lines in the INI file. @@ -2101,7 +2105,7 @@ Setting `0` means the short links are cleaned up approximately every 10 minutes. A negative value such as `-1` disables expiry. {{< admonition type="caution" >}} -Short links without an expiration increase the size of the database and can't be deleted. +Short links without an expiration increase the size of the database and can't be deleted. Grafana recommends setting a duration based on your specific use case {{< /admonition >}}
From e0d27dc0d7175e2fc459694ca6b5b5501f54c965 Mon Sep 17 00:00:00 2001 From: Chris Hodges Date: Thu, 12 Jun 2025 10:51:46 -0500 Subject: [PATCH 04/51] Dashboard: Add configurable quick ranges for the time picker (#102254) * Dashboard: Add configurable quick ranges for the time picker * fix test and linter errors * update from array to TimeOption * Switching to grafana-scenes (Part 1 - remove grafana-ui changes * Update SceneTimePicker initialization * betterer * remove hallucinated argument * Revert "Bump scenes and fix types (#105167)" This reverts commit c6428dfc7463250f786a9089c46a2fac910333cb. * make gen-go * reset files * Shorten documentation to increase maintainability * Update _index.md * the --------- Co-authored-by: joshhunt Co-authored-by: Jacob Valdez --- conf/defaults.ini | 5 + conf/sample.ini | 5 + .../setup-grafana/configure-grafana/_index.md | 34 +++++ packages/grafana-data/src/types/config.ts | 2 + packages/grafana-runtime/src/config.ts | 2 + pkg/api/dtos/frontend_settings.go | 3 +- pkg/api/frontendsettings.go | 1 + pkg/setting/setting.go | 6 + pkg/setting/setting_time_picker.go | 57 ++++++++ pkg/setting/setting_time_picker_test.go | 130 ++++++++++++++++++ .../transformSaveModelSchemaV2ToScene.ts | 1 + .../transformSaveModelToScene.ts | 1 + 12 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 pkg/setting/setting_time_picker.go create mode 100644 pkg/setting/setting_time_picker_test.go diff --git a/conf/defaults.ini b/conf/defaults.ini index ca4561fc4e0..1a4e1e828b6 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -1987,6 +1987,11 @@ provider = static # Default system date format used in time range picker and other places where full time is displayed full_date = YYYY-MM-DD HH:mm:ss +[time_picker] +# Custom quick ranges for the time picker. Each quick range has a display name, a from value, and a to value. +# Format: [{"from":"now-5m","to":"now","display":"Last 5 minutes"},{"from":"now-15m","to":"now","display":"Last 15 minutes"}] +quick_ranges = + # Used by graph and other places where we only show small intervals interval_second = HH:mm:ss interval_minute = HH:mm diff --git a/conf/sample.ini b/conf/sample.ini index 5d50ad20207..868bc750d73 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -1924,6 +1924,11 @@ default_datasource_uid = # Default timezone for user preferences. Options are 'browser' for the browser local timezone or a timezone name from IANA Time Zone database, e.g. 'UTC' or 'Europe/Amsterdam' etc. ;default_timezone = browser +[time_picker] +# Custom quick ranges for the time picker. Each quick range has a display name, a from value, and a to value. +# Format: [{"from":"now-5m","to":"now","display":"Last 5 minutes"},{"from":"now-15m","to":"now","display":"Last 15 minutes"}] +;quick_ranges = + [expressions] # Enable or disable the expressions functionality. ;enabled = true diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index 14bfd8404ce..9d3d20baa10 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -2814,6 +2814,40 @@ Used as the default time zone for user preferences. Can be either `browser` for Set the default start of the week, valid values are: `saturday`, `sunday`, `monday` or `browser` to use the browser locale to define the first day of the week. Default is `browser`. +### `[time_picker]` + +This section controls system-wide defaults for the time picker, such as the default quick ranges. + +#### `quick_ranges` + +Set the default set of quick relative offset time ranges that show up in the right column of the time picker. Each configuration entry must have a `from`, `to`, and `display` field. Any configuration for this field must be in valid JSON format made up of a list of quick range configurations. + +The `from` and `to` fields should be valid relative time ranges. For more information the relative time formats, refer to [Time units and relative ranges.](/docs/grafana//dashboards/use-dashboards/#time-units-and-relative-ranges). The `from` field is required, but omitting `to` will result in the `from` value being used in both fields. + +If no configuration is provided, the default time ranges will be used. + +For example: + +```ini +[time_picker] +quick_ranges = [ + { + "display": "Last 5 minutes", + "from": "now-5m", + "to": "now", + }, + { + "display": "Yesterday", + "from": "now-1d/d", + }, + { + "display": "Today so far", + "from": "now/d", + "to": "now", + } +] +``` + ### `[expressions]` #### `enabled` diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index e8fa818a250..a52d60c7e27 100644 --- a/packages/grafana-data/src/types/config.ts +++ b/packages/grafana-data/src/types/config.ts @@ -9,6 +9,7 @@ import { NavLinkDTO } from './navModel'; import { OrgRole } from './orgs'; import { PanelPluginMeta } from './panel'; import { GrafanaTheme } from './theme'; +import { TimeOption } from './time'; /** * Describes the build information that will be available via the Grafana configuration. @@ -240,6 +241,7 @@ export interface GrafanaConfig { reportingStaticContext?: Record; exploreDefaultTimeOffset?: string; exploreHideLogsDownload?: boolean; + quickRanges?: TimeOption[]; // The namespace to use for kubernetes apiserver requests namespace: string; diff --git a/packages/grafana-runtime/src/config.ts b/packages/grafana-runtime/src/config.ts index c19bae362d5..50e7ced3448 100644 --- a/packages/grafana-runtime/src/config.ts +++ b/packages/grafana-runtime/src/config.ts @@ -20,6 +20,7 @@ import { PluginLoadingStrategy, PluginDependencies, PluginExtensions, + TimeOption, } from '@grafana/data'; export interface AzureSettings { @@ -203,6 +204,7 @@ export class GrafanaBootConfig implements GrafanaConfig { reportingStaticContext?: Record; exploreDefaultTimeOffset = '1h'; exploreHideLogsDownload: boolean | undefined; + quickRanges?: TimeOption[]; /** * Language used in Grafana's UI. This is after the user's preference (or deteceted locale) is resolved to one of diff --git a/pkg/api/dtos/frontend_settings.go b/pkg/api/dtos/frontend_settings.go index b3e402b0f62..2c4406f41c9 100644 --- a/pkg/api/dtos/frontend_settings.go +++ b/pkg/api/dtos/frontend_settings.go @@ -274,7 +274,8 @@ type FrontendSettingsDTO struct { CloudMigrationIsTarget bool `json:"cloudMigrationIsTarget"` CloudMigrationPollIntervalMs int `json:"cloudMigrationPollIntervalMs"` - DateFormats setting.DateFormats `json:"dateFormats,omitempty"` + DateFormats setting.DateFormats `json:"dateFormats,omitempty"` + QuickRanges []setting.QuickRange `json:"quickRanges,omitempty"` LoginError string `json:"loginError,omitempty"` diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 5ef55403d4d..3a062fedad4 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -247,6 +247,7 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro TrustedTypesDefaultPolicyEnabled: trustedTypesDefaultPolicyEnabled, CSPReportOnlyEnabled: hs.Cfg.CSPReportOnlyEnabled, DateFormats: hs.Cfg.DateFormats, + QuickRanges: hs.Cfg.QuickRanges, SecureSocksDSProxyEnabled: hs.Cfg.SecureSocksDSProxy.Enabled && hs.Cfg.SecureSocksDSProxy.ShowUI, EnableFrontendSandboxForPlugins: hs.Cfg.EnableFrontendSandboxForPlugins, PublicDashboardAccessToken: c.PublicDashboardAccessToken, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index fcab4269a3f..ff53a2ffb5b 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -312,6 +312,7 @@ type Cfg struct { Anonymous AnonymousSettings DateFormats DateFormats + QuickRanges QuickRanges // User UserInviteMaxLifetime time.Duration @@ -1388,6 +1389,11 @@ func (cfg *Cfg) parseINIFile(iniFile *ini.File) error { cfg.ScopesListScopesURL = scopesSection.Key("list_scopes_endpoint").MustString("") cfg.ScopesListDashboardsURL = scopesSection.Key("list_dashboards_endpoint").MustString("") + // Time picker settings + if err := cfg.readTimePicker(); err != nil { + return err + } + // unified storage config cfg.setUnifiedStorageConfig() diff --git a/pkg/setting/setting_time_picker.go b/pkg/setting/setting_time_picker.go new file mode 100644 index 00000000000..bae55f00441 --- /dev/null +++ b/pkg/setting/setting_time_picker.go @@ -0,0 +1,57 @@ +package setting + +import ( + "encoding/json" + "fmt" +) + +// QuickRanges is a slice of QuickRange objects that can be directly used in frontend +type QuickRanges []QuickRange + +// QuickRange represents a time range option in the time picker. +// It defines a preset time range that users can select from the time picker dropdown. +type QuickRange struct { + // Display is the user-friendly label shown in the UI for this time range + Display string `json:"display"` + // From is the start of the time range in a format like "now-6h" or an absolute time + From string `json:"from"` + // To is the end of the time range, defaults to "now" if omitted + To string `json:"to,omitempty"` +} + +func (cfg *Cfg) readTimePicker() error { + timePickerSection := cfg.Raw.Section("time_picker") + quickRangesStr := timePickerSection.Key("quick_ranges").String() + + if quickRangesStr == "" { + cfg.QuickRanges = []QuickRange{} + return nil + } + + var quickRanges []QuickRange + err := json.Unmarshal([]byte(quickRangesStr), &quickRanges) + if err != nil { + cfg.Logger.Error("Failed to parse quick_ranges", "error", err) + return fmt.Errorf("failed to parse quick_ranges: %w", err) + } + + // Validate the quick ranges and set defaults + for i, qr := range quickRanges { + if qr.Display == "" { + cfg.Logger.Error("Quick range is missing display name", "index", i) + return fmt.Errorf("quick range at index %d is missing display name", i) + } + if qr.From == "" { + cfg.Logger.Error("Quick range is missing 'from' field", "display", qr.Display) + return fmt.Errorf("quick range '%s' is missing 'from' field", qr.Display) + } + // Set default value for To field if it's empty + if qr.To == "" { + quickRanges[i].To = "now" + } + } + + cfg.QuickRanges = quickRanges + + return nil +} diff --git a/pkg/setting/setting_time_picker_test.go b/pkg/setting/setting_time_picker_test.go new file mode 100644 index 00000000000..50414ced1c5 --- /dev/null +++ b/pkg/setting/setting_time_picker_test.go @@ -0,0 +1,130 @@ +package setting + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/ini.v1" +) + +func TestReadTimePicker(t *testing.T) { + t.Run("Default values when quick_ranges not specified", func(t *testing.T) { + cfg := NewCfg() + iniContent := ` +[time_picker] +` + iniFile, err := ini.Load([]byte(iniContent)) + require.NoError(t, err) + cfg.Raw = iniFile + + err = cfg.readTimePicker() + require.NoError(t, err) + + // Default values should be used + assert.Empty(t, cfg.QuickRanges) + }) + + t.Run("Parse valid quick_ranges", func(t *testing.T) { + cfg := NewCfg() + iniContent := ` +[time_picker] +quick_ranges = [{"display":"Last 5 minutes","from":"now-5m","to":"now"},{"display":"Yesterday","from":"now-1d/d"},{"display":"Today so far","from":"now/d","to":"now"}] +` + iniFile, err := ini.Load([]byte(iniContent)) + require.NoError(t, err) + cfg.Raw = iniFile + + err = cfg.readTimePicker() + require.NoError(t, err) + + // Validate parsed values + require.Len(t, cfg.QuickRanges, 3) + + // First range + assert.Equal(t, "Last 5 minutes", cfg.QuickRanges[0].Display) + assert.Equal(t, "now-5m", cfg.QuickRanges[0].From) + assert.Equal(t, "now", cfg.QuickRanges[0].To) + + // Second range (defaulted to 'now') + assert.Equal(t, "Yesterday", cfg.QuickRanges[1].Display) + assert.Equal(t, "now-1d/d", cfg.QuickRanges[1].From) + assert.Equal(t, "now", cfg.QuickRanges[1].To) + + // Third range + assert.Equal(t, "Today so far", cfg.QuickRanges[2].Display) + assert.Equal(t, "now/d", cfg.QuickRanges[2].From) + assert.Equal(t, "now", cfg.QuickRanges[2].To) + }) + + t.Run("QuickRange with missing To field gets default value", func(t *testing.T) { + cfg := NewCfg() + iniContent := ` +[time_picker] +quick_ranges = [{"display":"Yesterday","from":"now-1d/d"}] +` + iniFile, err := ini.Load([]byte(iniContent)) + require.NoError(t, err) + cfg.Raw = iniFile + + err = cfg.readTimePicker() + require.NoError(t, err) + + // Validate the parsed value + require.Len(t, cfg.QuickRanges, 1) + assert.Equal(t, "Yesterday", cfg.QuickRanges[0].Display) + assert.Equal(t, "now-1d/d", cfg.QuickRanges[0].From) + assert.Equal(t, "now", cfg.QuickRanges[0].To) + + jsonBytes, err := json.Marshal(cfg.QuickRanges) + require.NoError(t, err) + assert.Contains(t, string(jsonBytes), "\"to\":\"now\"") + }) + + t.Run("Invalid JSON format", func(t *testing.T) { + cfg := NewCfg() + iniContent := ` +[time_picker] +quick_ranges = [{"display":"Last 5 minutes","from":"now-5m","to":"now"}, INVALID JSON] +` + iniFile, err := ini.Load([]byte(iniContent)) + require.NoError(t, err) + cfg.Raw = iniFile + + err = cfg.readTimePicker() + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "failed to parse quick_ranges")) + }) + + t.Run("Missing display field", func(t *testing.T) { + cfg := NewCfg() + iniContent := ` +[time_picker] +quick_ranges = [{"from":"now-5m","to":"now"}] +` + iniFile, err := ini.Load([]byte(iniContent)) + require.NoError(t, err) + cfg.Raw = iniFile + + err = cfg.readTimePicker() + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "missing display name")) + }) + + t.Run("Missing from field", func(t *testing.T) { + cfg := NewCfg() + iniContent := ` +[time_picker] +quick_ranges = [{"display":"Last 5 minutes","to":"now"}] +` + iniFile, err := ini.Load([]byte(iniContent)) + require.NoError(t, err) + cfg.Raw = iniFile + + err = cfg.readTimePicker() + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "missing 'from' field")) + }) +} diff --git a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts index fe2ef3ec1f9..10dbc1d11e9 100644 --- a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts +++ b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts @@ -207,6 +207,7 @@ export function transformSaveModelSchemaV2ToScene(dto: DashboardWithAccessInfo Date: Thu, 12 Jun 2025 18:07:16 +0100 Subject: [PATCH 05/51] Alerting: Correctly persist FiredAt in SyncRuleStatePersister (#106658) Correctly persist FiredAt --- pkg/services/ngalert/state/persister_sync_rule.go | 1 + pkg/services/ngalert/state/persister_sync_rule_test.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/pkg/services/ngalert/state/persister_sync_rule.go b/pkg/services/ngalert/state/persister_sync_rule.go index 846c23a239d..da2494d3647 100644 --- a/pkg/services/ngalert/state/persister_sync_rule.go +++ b/pkg/services/ngalert/state/persister_sync_rule.go @@ -53,6 +53,7 @@ func (a *SyncRuleStatePersister) Sync(ctx context.Context, span trace.Span, rule LastEvalTime: s.LastEvaluationTime, CurrentStateSince: s.StartsAt, CurrentStateEnd: s.EndsAt, + FiredAt: s.FiredAt, ResolvedAt: s.ResolvedAt, LastSentAt: s.LastSentAt, ResultFingerprint: s.ResultFingerprint.String(), diff --git a/pkg/services/ngalert/state/persister_sync_rule_test.go b/pkg/services/ngalert/state/persister_sync_rule_test.go index b8a1a69057d..534ba678244 100644 --- a/pkg/services/ngalert/state/persister_sync_rule_test.go +++ b/pkg/services/ngalert/state/persister_sync_rule_test.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/util" ) func TestSyncRuleStatePersister_Sync(t *testing.T) { @@ -33,6 +34,7 @@ func TestSyncRuleStatePersister_Sync(t *testing.T) { Labels: data.Labels{ "label-1": "value-1", }, + FiredAt: util.Pointer(time.Now()), LastEvaluationTime: time.Now(), StartsAt: time.Now(), EndsAt: time.Now(), @@ -72,6 +74,7 @@ func TestSyncRuleStatePersister_Sync(t *testing.T) { LastEvalTime: s.LastEvaluationTime, CurrentStateSince: s.StartsAt, CurrentStateEnd: s.EndsAt, + FiredAt: s.FiredAt, ResolvedAt: s.ResolvedAt, LastSentAt: s.LastSentAt, ResultFingerprint: s.ResultFingerprint.String(), From 7b70271ce6e8a11c445ca102ed7d17bb4407ba30 Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Thu, 12 Jun 2025 20:43:13 +0200 Subject: [PATCH 06/51] New Logs Panel: add syntax highlighting option (#106611) --- .../logs/panelcfg/x/LogsPanelCfg_types.gen.ts | 1 + public/app/plugins/panel/logs/LogsPanel.tsx | 3 +- public/app/plugins/panel/logs/module.tsx | 30 ++++++++++++------- public/app/plugins/panel/logs/panelcfg.cue | 1 + public/app/plugins/panel/logs/panelcfg.gen.ts | 1 + 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/grafana-schema/src/raw/composable/logs/panelcfg/x/LogsPanelCfg_types.gen.ts b/packages/grafana-schema/src/raw/composable/logs/panelcfg/x/LogsPanelCfg_types.gen.ts index 9129f7c3d85..dbf274e51bc 100644 --- a/packages/grafana-schema/src/raw/composable/logs/panelcfg/x/LogsPanelCfg_types.gen.ts +++ b/packages/grafana-schema/src/raw/composable/logs/panelcfg/x/LogsPanelCfg_types.gen.ts @@ -41,6 +41,7 @@ export interface Options { showLogContextToggle: boolean; showTime: boolean; sortOrder: common.LogsSortOrder; + syntaxHighlighting?: boolean; wrapLogMessage: boolean; } diff --git a/public/app/plugins/panel/logs/LogsPanel.tsx b/public/app/plugins/panel/logs/LogsPanel.tsx index b1b6a1a8285..a55bdd30a46 100644 --- a/public/app/plugins/panel/logs/LogsPanel.tsx +++ b/public/app/plugins/panel/logs/LogsPanel.tsx @@ -152,6 +152,7 @@ export const LogsPanel = ({ enableInfiniteScrolling, onNewLogsReceived, fontSize, + syntaxHighlighting, ...options }, id, @@ -568,7 +569,7 @@ export const LogsPanel = ({ showTime={showTime} sortOrder={sortOrder} logOptionsStorageKey={storageKey} - syntaxHighlighting={prettifyLogMessage} + syntaxHighlighting={syntaxHighlighting} timeRange={data.timeRange} timeZone={timeZone} wrapLogMessage={wrapLogMessage} diff --git a/public/app/plugins/panel/logs/module.tsx b/public/app/plugins/panel/logs/module.tsx index 5ee555bb09a..da674495d77 100644 --- a/public/app/plugins/panel/logs/module.tsx +++ b/public/app/plugins/panel/logs/module.tsx @@ -30,19 +30,29 @@ export const plugin = new PanelPlugin(LogsPanel) }); } - builder - .addBooleanSwitch({ - path: 'wrapLogMessage', - name: 'Wrap lines', - description: '', - defaultValue: false, - }) - .addBooleanSwitch({ + builder.addBooleanSwitch({ + path: 'wrapLogMessage', + name: 'Wrap lines', + description: '', + defaultValue: false, + }); + + if (config.featureToggles.newLogsPanel) { + builder.addBooleanSwitch({ + path: 'syntaxHighlighting', + name: 'Enable syntax highlighting', + description: 'Use a predefined syntax coloring grammar to highlight relevant parts of the log lines', + }); + } else { + builder.addBooleanSwitch({ path: 'prettifyLogMessage', - name: config.featureToggles.newLogsPanel ? 'Enable log message highlighting' : 'Prettify JSON', + name: 'Prettify JSON', description: '', defaultValue: false, - }) + }); + } + + builder .addBooleanSwitch({ path: 'enableLogDetails', name: 'Enable log details', diff --git a/public/app/plugins/panel/logs/panelcfg.cue b/public/app/plugins/panel/logs/panelcfg.cue index 821bc66321a..cd21fc2e041 100644 --- a/public/app/plugins/panel/logs/panelcfg.cue +++ b/public/app/plugins/panel/logs/panelcfg.cue @@ -35,6 +35,7 @@ composableKinds: PanelCfg: { wrapLogMessage: bool prettifyLogMessage: bool enableLogDetails: bool + syntaxHighlighting?: bool sortOrder: common.LogsSortOrder dedupStrategy: common.LogsDedupStrategy enableInfiniteScrolling?: bool diff --git a/public/app/plugins/panel/logs/panelcfg.gen.ts b/public/app/plugins/panel/logs/panelcfg.gen.ts index 9a9a47b82d1..5af4d61f578 100644 --- a/public/app/plugins/panel/logs/panelcfg.gen.ts +++ b/public/app/plugins/panel/logs/panelcfg.gen.ts @@ -39,6 +39,7 @@ export interface Options { showLogContextToggle: boolean; showTime: boolean; sortOrder: common.LogsSortOrder; + syntaxHighlighting?: boolean; wrapLogMessage: boolean; } From 1e41c07920eb312dcf13604aef4451e437d8a357 Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Thu, 12 Jun 2025 15:13:25 -0400 Subject: [PATCH 07/51] StateTimeline: Support `NaN` and `null` value mappings (#105638) * fix(#92944): add StateTimeline Null+NaN handling * chore: remove console.warns from debugging * test: initialize a couple of simple tests * test: more tests for hasMappedNaN and hasMappedNull * chore: revert some of the let-const syntax cleanup for a later PR * chore: rename should draw method * chore: fix comment typo * refactor(timeline-chart-utils): un-nest hasSpecialMappedValue() helper * test(timeline-chart-utils): unit test hasSpecialMappedValue() helper * chore: fix code comment typos in changed files * refactor(timeline-chart): reduce helper DRY-ness for better performance * fix(timeline-chart): check Y value for truthiness, not if it is finite * test(state-timeline): additional gdev test panels with null + NN values * fix(timeline-chart): allow Y value of zero in checks --------- Co-authored-by: Jesse David Peterson Co-authored-by: Adela Almasan --- .../timeline-thresholds-mappings.json | 331 ++++++++++++++++-- packages/grafana-data/src/types/dataFrame.ts | 4 +- .../src/utils/valueMappings.test.ts | 92 +++++ .../components/TimelineChart/timeline.test.ts | 211 +++++++++++ .../core/components/TimelineChart/timeline.ts | 31 +- .../components/TimelineChart/utils.test.ts | 49 ++- .../core/components/TimelineChart/utils.ts | 25 +- 7 files changed, 692 insertions(+), 51 deletions(-) create mode 100644 public/app/core/components/TimelineChart/timeline.test.ts diff --git a/devenv/dev-dashboards/panel-timeline/timeline-thresholds-mappings.json b/devenv/dev-dashboards/panel-timeline/timeline-thresholds-mappings.json index 137bbfe04f2..39cc956a7f8 100644 --- a/devenv/dev-dashboards/panel-timeline/timeline-thresholds-mappings.json +++ b/devenv/dev-dashboards/panel-timeline/timeline-thresholds-mappings.json @@ -24,19 +24,27 @@ "editable": true, "fiscalYearStartMonth": 0, "graphTooltip": 0, - "id": 1263, + "id": 15116, "links": [], - "liveNow": false, "panels": [ { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "thresholds" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -45,7 +53,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -74,10 +83,12 @@ "rowHeight": 0.9, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -93,14 +104,23 @@ "type": "state-timeline" }, { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "thresholds" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -109,7 +129,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -146,10 +167,12 @@ "rowHeight": 0.9, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -165,14 +188,23 @@ "type": "state-timeline" }, { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "thresholds" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -181,7 +213,8 @@ "mode": "percentage", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -218,10 +251,12 @@ "rowHeight": 0.9, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -237,14 +272,23 @@ "type": "state-timeline" }, { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "thresholds" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -253,7 +297,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -275,7 +320,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -314,10 +360,12 @@ "rowHeight": 0.9, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -333,14 +381,23 @@ "type": "state-timeline" }, { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "palette-classic" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -394,7 +451,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 } ] } @@ -416,13 +474,15 @@ "showLegend": true }, "mergeValues": true, - "rowHeight": 0.9, + "rowHeight": 0.47, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -438,14 +498,23 @@ "type": "state-timeline" }, { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "palette-classic" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -454,7 +523,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 } ] } @@ -537,10 +607,12 @@ "rowHeight": 0.9, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -556,14 +628,23 @@ "type": "state-timeline" }, { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "thresholds" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -572,7 +653,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -601,10 +683,12 @@ "rowHeight": 0.9, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -620,14 +704,23 @@ "type": "state-timeline" }, { - "datasource": { "type": "testdata" }, + "datasource": { + "type": "testdata" + }, "fieldConfig": { "defaults": { "color": { "mode": "thresholds" }, "custom": { + "axisPlacement": "auto", "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, "lineWidth": 0, "spanNulls": false }, @@ -636,7 +729,8 @@ "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -665,10 +759,12 @@ "rowHeight": 0.9, "showValue": "auto", "tooltip": { + "hideZeros": false, "mode": "single", "sort": "none" } }, + "pluginVersion": "12.1.0-pre", "targets": [ { "datasource": { @@ -709,10 +805,192 @@ } ], "type": "state-timeline" + }, + { + "datasource": { + "type": "testdata" + }, + "fieldConfig": { + "defaults": { + "color": { + "mode": "palette-classic" + }, + "custom": { + "axisPlacement": "auto", + "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, + "lineWidth": 0, + "spanNulls": false + }, + "fieldMinMax": false, + "mappings": [ + { + "options": { + "match": "null", + "result": { + "color": "purple", + "index": 0, + "text": "null" + } + }, + "type": "special" + }, + { + "options": { + "match": "nan", + "result": { + "color": "red", + "index": 1, + "text": "NaN" + } + }, + "type": "special" + } + ], + "thresholds": { + "mode": "absolute", + "steps": [ + { + "color": "green", + "value": 0 + } + ] + } + }, + "overrides": [] + }, + "gridPos": { + "h": 9, + "w": 6, + "x": 0, + "y": 18 + }, + "id": 12, + "options": { + "alignValue": "center", + "legend": { + "displayMode": "list", + "placement": "bottom", + "showLegend": true + }, + "mergeValues": true, + "rowHeight": 0.9, + "showValue": "auto", + "tooltip": { + "hideZeros": false, + "mode": "single", + "sort": "none" + } + }, + "pluginVersion": "12.1.0-pre", + "targets": [ + { + "datasource": { + "type": "testdata", + "uid": "PD8C576611E62080A" + }, + "rawFrameContent": "[\n {\n \"schema\": {\n \"refId\": \"A\",\n \"fields\": [\n {\n \"name\": \"time\",\n \"type\": \"time\",\n \"typeInfo\": {\n \"frame\": \"time\",\n \"nullable\": true\n },\n \"config\": {}\n },\n {\n \"name\": \"value\",\n \"type\": \"number\",\n \"typeInfo\": {\n \"frame\": \"int64\",\n \"nullable\": true\n },\n \"config\": {\n \"thresholds\": {\n \"mode\": \"absolute\",\n \"steps\": [\n {\n \"color\": \"green\",\n \"value\": null\n }\n ]\n }\n }\n }\n ]\n },\n \"data\": {\n \"values\": [\n [\n 1674732835000,\n 1674736435000,\n 1674740035000,\n 1674743635000,\n 1674747235000\n ],\n [\n 5,\n null,\n 20,\n null,\n 40\n ]\n ],\n \"entities\": [null, { \"NaN\": [3]}]\n }\n }\n]", + "refId": "A", + "scenarioId": "raw_frame" + } + ], + "title": "special null | NaN value mapping from data", + "type": "state-timeline" + }, + { + "datasource": { + "type": "testdata" + }, + "fieldConfig": { + "defaults": { + "color": { + "mode": "palette-classic" + }, + "custom": { + "axisPlacement": "auto", + "fillOpacity": 70, + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "insertNulls": false, + "lineWidth": 0, + "spanNulls": false + }, + "fieldMinMax": false, + "mappings": [ + { + "options": { + "match": "null+nan", + "result": { + "color": "super-light-red", + "index": 0, + "text": "null + NaN" + } + }, + "type": "special" + } + ], + "thresholds": { + "mode": "absolute", + "steps": [ + { + "color": "green", + "value": 0 + } + ] + } + }, + "overrides": [] + }, + "gridPos": { + "h": 9, + "w": 6, + "x": 6, + "y": 18 + }, + "id": 13, + "options": { + "alignValue": "center", + "legend": { + "displayMode": "list", + "placement": "bottom", + "showLegend": true + }, + "mergeValues": true, + "rowHeight": 0.9, + "showValue": "auto", + "tooltip": { + "hideZeros": false, + "mode": "single", + "sort": "none" + } + }, + "pluginVersion": "12.1.0-pre", + "targets": [ + { + "datasource": { + "type": "testdata", + "uid": "PD8C576611E62080A" + }, + "rawFrameContent": "[\n {\n \"schema\": {\n \"refId\": \"A\",\n \"fields\": [\n {\n \"name\": \"time\",\n \"type\": \"time\",\n \"typeInfo\": {\n \"frame\": \"time\",\n \"nullable\": true\n },\n \"config\": {}\n },\n {\n \"name\": \"value\",\n \"type\": \"number\",\n \"typeInfo\": {\n \"frame\": \"int64\",\n \"nullable\": true\n },\n \"config\": {\n \"thresholds\": {\n \"mode\": \"absolute\",\n \"steps\": [\n {\n \"color\": \"green\",\n \"value\": null\n }\n ]\n }\n }\n }\n ]\n },\n \"data\": {\n \"values\": [\n [\n 1674732835000,\n 1674736435000,\n 1674740035000,\n 1674743635000,\n 1674747235000\n ],\n [\n 5,\n null,\n 20,\n null,\n 40\n ]\n ],\n \"entities\": [null, { \"NaN\": [1]}]\n }\n }\n]", + "refId": "A", + "scenarioId": "raw_frame" + } + ], + "title": "special null + NaN value mapping from data", + "type": "state-timeline" } ], - "refresh": false, - "schemaVersion": 38, + "preload": false, + "refresh": "", + "schemaVersion": 41, "tags": [ "gdev", "panel-tests", @@ -723,13 +1001,12 @@ "list": [] }, "time": { - "from": "2023-01-26T11:33:55.000Z", - "to": "2023-01-26T14:33:55.000Z" + "from": "2023-01-26T11:29:47.180Z", + "to": "2023-01-26T16:29:39.205Z" }, "timepicker": {}, - "timezone": "", + "timezone": "utc", "title": "StateTimeline - Thresholds & Mappings", "uid": "Kce7z9TVz", - "version": 14, - "weekStart": "" + "version": 13 } diff --git a/packages/grafana-data/src/types/dataFrame.ts b/packages/grafana-data/src/types/dataFrame.ts index 4a0f8adbe58..e8b6ca13f7b 100644 --- a/packages/grafana-data/src/types/dataFrame.ts +++ b/packages/grafana-data/src/types/dataFrame.ts @@ -54,7 +54,7 @@ export interface FieldConfig { description?: string; /** - * An explict path to the field in the datasource. When the frame meta includes a path, + * An explicit path to the field in the datasource. When the frame meta includes a path, * This will default to `${frame.meta.path}/${field.name} * * When defined, this value can be used as an identifier within the datasource scope, and @@ -158,7 +158,7 @@ export interface Field { /** * When type === FieldType.Time, this can optionally store - * the nanosecond-precison fractions as integers between + * the nanosecond-precision fractions as integers between * 0 and 999999. */ nanos?: number[]; diff --git a/packages/grafana-data/src/utils/valueMappings.test.ts b/packages/grafana-data/src/utils/valueMappings.test.ts index 90686b52895..e03f0ad2585 100644 --- a/packages/grafana-data/src/utils/valueMappings.test.ts +++ b/packages/grafana-data/src/utils/valueMappings.test.ts @@ -281,3 +281,95 @@ describe('isNumeric', () => { expect(isNumeric(value)).toEqual(expected); }); }); + +describe('null and NaN special mapping', () => { + it('should return null for NaN', () => { + const value = Number.NaN; + expect( + getValueMappingResult( + [ + { + type: MappingType.SpecialValue, + options: { + match: SpecialValueMatch.NullAndNaN, + result: { text: 'it is null or nan' }, + }, + }, + ], + value + ) + ).toEqual({ text: 'it is null or nan' }); + }); + + it('should return null for null', () => { + const value = null; + expect( + getValueMappingResult( + [ + { + type: MappingType.SpecialValue, + options: { + match: SpecialValueMatch.NullAndNaN, + result: { text: 'it is null or nan' }, + }, + }, + ], + value + ) + ).toEqual({ text: 'it is null or nan' }); + }); + + it('should return null for undefined', () => { + const value = undefined; + expect( + getValueMappingResult( + [ + { + type: MappingType.SpecialValue, + options: { + match: SpecialValueMatch.NullAndNaN, + result: { text: 'it is null or nan' }, + }, + }, + ], + value + ) + ).toEqual({ text: 'it is null or nan' }); + }); + + it('should return null for numeric non-NaN', () => { + const value = 42; + expect( + getValueMappingResult( + [ + { + type: MappingType.SpecialValue, + options: { + match: SpecialValueMatch.NullAndNaN, + result: { text: 'it is null or nan' }, + }, + }, + ], + value + ) + ).toBeNull(); + }); + + it('should return null for string', () => { + const value = 'foo'; + expect( + getValueMappingResult( + [ + { + type: MappingType.SpecialValue, + options: { + match: SpecialValueMatch.NullAndNaN, + result: { text: 'it is null or nan' }, + }, + }, + ], + value + ) + ).toBeNull(); + }); +}); diff --git a/public/app/core/components/TimelineChart/timeline.test.ts b/public/app/core/components/TimelineChart/timeline.test.ts new file mode 100644 index 00000000000..1fd72e9bec5 --- /dev/null +++ b/public/app/core/components/TimelineChart/timeline.test.ts @@ -0,0 +1,211 @@ +import uPlot from 'uplot'; + +import { getDefaultTimeRange, createTheme } from '@grafana/data'; +import { VisibilityMode } from '@grafana/schema'; + +import { getConfig, TimelineCoreOptions } from './timeline'; +import { TimelineMode } from './utils'; + +jest.mock('uplot'); + +describe('StateTimeline uPlot integration', () => { + const buildTestCoreOptions = (opts: Partial = {}): TimelineCoreOptions => ({ + mode: TimelineMode.Changes, + numSeries: 1, + theme: createTheme(), + showValue: VisibilityMode.Always, + isDiscrete: jest.fn(() => true), + hasMappedNull: jest.fn(() => false), + hasMappedNaN: jest.fn(() => false), + getValueColor: jest.fn(() => '#fff'), + label: jest.fn(() => 'foo'), + getTimeRange: jest.fn(() => getDefaultTimeRange()), + getFieldConfig: jest.fn(() => ({})), + hoverMulti: false, + ...opts, + }); + + const buildMockUplotInstance = ( + data: Array> = [ + [0, 0, 0], + [0, 1, 2], + ] + ) => + ({ + ctx: { + save: jest.fn(), + restore: jest.fn(), + rect: jest.fn(), + clip: jest.fn(), + font: '', + fill: jest.fn(), + fillStyle: '', + fillText: jest.fn(), + measureText: jest.fn(() => ({ width: 0 })), + beginPath: jest.fn(), + }, + root: document.createElement('div'), + bbox: { left: 0, top: 0, width: 100, height: 100 }, + data, + cursor: { left: 0, top: 0 }, + series: [{}], + scales: {}, + opts: {}, + pxRatio: 1, + posToVal: jest.fn(), + valToPos: jest.fn(), + }) as unknown as uPlot; + + const callOrientCallback = (mockUplot: uPlot) => { + const orientCallback = jest.mocked(uPlot.orient).mock.calls[jest.mocked(uPlot.orient).mock.calls.length - 1][2]; + const methods = { + moveTo: jest.fn(() => {}), + lineTo: jest.fn(() => {}), + rect: jest.fn(() => {}), + arc: jest.fn(() => {}), + bezierCurveTo: jest.fn(() => {}), + }; + + orientCallback( + mockUplot.series[0], + mockUplot.data[0] as number[], + mockUplot.data[1] as number[], + mockUplot.scales.x, + mockUplot.scales.y, + jest.fn(() => 1), + jest.fn(() => 1), + 0, + 0, + 100, + 100, + methods.moveTo, + methods.lineTo, + methods.rect, + methods.arc, + methods.bezierCurveTo + ); + + return methods; + }; + + describe('#drawPoints', () => { + it('returns a `drawPoints` method when a `formatValue` function is provided', () => { + const config = getConfig(buildTestCoreOptions({ formatValue: () => 'foo' })); + expect(typeof config.drawPoints).toBe('function'); + }); + + it('returns false for `drawPoints` when no `formatValue` function is provided', () => { + const config = getConfig(buildTestCoreOptions()); + expect(config.drawPoints).toBe(false); + }); + + it('returns false for `drawPoints` if the visibility mode is `never`', () => { + const config = getConfig(buildTestCoreOptions({ formatValue: () => 'foo', showValue: VisibilityMode.Never })); + expect(config.drawPoints).toBe(false); + }); + + it('returns a function for `drawPoints` if the conditions are met', () => { + const { drawPoints } = getConfig(buildTestCoreOptions({ formatValue: () => 'foo' })); + if (!drawPoints) { + throw new Error('drawPoints is not defined'); + } + const mockUplot = buildMockUplotInstance(); + expect(drawPoints(mockUplot, 1, 2, 3, null)).toBe(false); + expect(uPlot.orient).toHaveBeenCalledWith(mockUplot, 1, expect.any(Function)); + }); + + describe('#drawPaths', () => { + describe('null and NaN values', () => { + // these tests are attempting to determine whether `shouldDrawYVal` is returning false + // and preventing a draw for a given value. this is being done by checking the number of + // calls to `rect` by the `orient` callback created by a `drawPaths` call. + + it('should draw boxes for null values when hasMappedNull returns true and isDiscrete returns true', () => { + const { drawClear, drawPaths } = getConfig( + buildTestCoreOptions({ + hasMappedNull: jest.fn(() => true), + isDiscrete: jest.fn(() => true), + formatValue: () => 'foo', + }) + ); + const mockUplot = buildMockUplotInstance([[0], [null]]); + + drawClear(mockUplot); + drawPaths(mockUplot, 1, 0, 1); + + const { rect } = callOrientCallback(mockUplot); + expect(rect).toHaveBeenCalledTimes(2); + }); + + it('should not draw boxes for null values when hasMappedNull returns false and isDiscrete returns true', () => { + const { drawClear, drawPaths } = getConfig( + buildTestCoreOptions({ + hasMappedNull: jest.fn(() => false), + isDiscrete: jest.fn(() => true), + formatValue: () => 'foo', + }) + ); + const mockUplot = buildMockUplotInstance([[0], [null]]); + + drawClear(mockUplot); + drawPaths(mockUplot, 1, 0, 1); + + const { rect } = callOrientCallback(mockUplot); + expect(rect).toHaveBeenCalledTimes(1); + }); + + it('should draw boxes for NaN values when hasMappedNaN returns true and isDiscrete returns true', () => { + const { drawClear, drawPaths } = getConfig( + buildTestCoreOptions({ + hasMappedNaN: jest.fn(() => true), + isDiscrete: jest.fn(() => true), + formatValue: () => 'foo', + }) + ); + const mockUplot = buildMockUplotInstance([[0], [NaN]]); + + drawClear(mockUplot); + drawPaths(mockUplot, 1, 0, 1); + + const { rect } = callOrientCallback(mockUplot); + expect(rect).toHaveBeenCalledTimes(2); + }); + + it('should not draw boxes for NaN values when hasMappedNaN returns false and isDiscrete returns true', () => { + const { drawClear, drawPaths } = getConfig( + buildTestCoreOptions({ + hasMappedNaN: jest.fn(() => false), + isDiscrete: jest.fn(() => true), + formatValue: () => 'foo', + }) + ); + const mockUplot = buildMockUplotInstance([[0], [NaN]]); + + drawClear(mockUplot); + drawPaths(mockUplot, 1, 0, 1); + + const { rect } = callOrientCallback(mockUplot); + expect(rect).toHaveBeenCalledTimes(1); + }); + + it('should not draw boxes for NaN or null values when isDiscrete returns false', () => { + const { drawClear, drawPaths } = getConfig( + buildTestCoreOptions({ + hasMappedNaN: jest.fn(() => true), + hasMappedNull: jest.fn(() => true), + isDiscrete: jest.fn(() => false), + formatValue: () => 'foo', + }) + ); + const mockUplot = buildMockUplotInstance([[0], [NaN, null]]); + + drawClear(mockUplot); + drawPaths(mockUplot, 1, 0, 1); + + const { rect } = callOrientCallback(mockUplot); + expect(rect).toHaveBeenCalledTimes(1); + }); + }); + }); + }); +}); diff --git a/public/app/core/components/TimelineChart/timeline.ts b/public/app/core/components/TimelineChart/timeline.ts index 8fa0a8faef1..302437593e9 100644 --- a/public/app/core/components/TimelineChart/timeline.ts +++ b/public/app/core/components/TimelineChart/timeline.ts @@ -47,6 +47,7 @@ export interface TimelineCoreOptions { mergeValues?: boolean; isDiscrete: (seriesIdx: number) => boolean; hasMappedNull: (seriesIdx: number) => boolean; + hasMappedNaN: (seriesIdx: number) => boolean; getValueColor: (seriesIdx: number, value: unknown) => string; label: (seriesIdx: number) => string; getTimeRange: () => TimeRange; @@ -64,6 +65,7 @@ export function getConfig(opts: TimelineCoreOptions) { numSeries, isDiscrete, hasMappedNull, + hasMappedNaN, rowHeight = 0, colWidth = 0, showValue, @@ -196,9 +198,9 @@ export function getConfig(opts: TimelineCoreOptions) { sidx, (series, dataX, dataY, scaleX, scaleY, valToPosX, valToPosY, xOff, yOff, xDim, yDim, moveTo, lineTo, rect) => { let strokeWidth = round((series.width || 0) * uPlot.pxRatio); - - let discrete = isDiscrete(sidx); - let mappedNull = discrete && hasMappedNull(sidx); + const discrete = isDiscrete(sidx); + const mappedNull = discrete && hasMappedNull(sidx); + const mappedNaN = discrete && hasMappedNaN(sidx); u.ctx.save(); rect(u.ctx, u.bbox.left, u.bbox.top, u.bbox.width, u.bbox.height); @@ -209,7 +211,10 @@ export function getConfig(opts: TimelineCoreOptions) { for (let ix = 0; ix < dataY.length; ix++) { let yVal = dataY[ix]; - if (yVal != null || mappedNull) { + const shouldDrawY = + !!yVal || yVal === 0 || (yVal === null && mappedNull) || (Number.isNaN(yVal) && mappedNaN); + + if (shouldDrawY) { let left = Math.round(valToPosX(dataX[ix], scaleX, xDim, xOff)); let nextIx = ix; @@ -252,8 +257,10 @@ export function getConfig(opts: TimelineCoreOptions) { for (let ix = idx0; ix <= idx1; ix++) { let yVal = dataY[ix]; + const shouldDrawY = + !!yVal || yVal === 0 || (yVal === null && mappedNull) || (Number.isNaN(yVal) && mappedNaN); - if (yVal != null || mappedNull) { + if (shouldDrawY) { // TODO: all xPos can be pre-computed once for all series in aligned set let left = valToPosX(dataX[ix], scaleX, xDim, xOff); @@ -306,14 +313,18 @@ export function getConfig(opts: TimelineCoreOptions) { sidx, (series, dataX, dataY, scaleX, scaleY, valToPosX, valToPosY, xOff, yOff, xDim, yDim) => { let strokeWidth = round((series.width || 0) * uPlot.pxRatio); - - let discrete = isDiscrete(sidx); - let mappedNull = discrete && hasMappedNull(sidx); - let y = round(valToPosY(ySplits[sidx - 1], scaleY, yDim, yOff)); + const discrete = isDiscrete(sidx); + const mappedNull = discrete && hasMappedNull(sidx); + const mappedNaN = discrete && hasMappedNaN(sidx); + for (let ix = 0; ix < dataY.length; ix++) { - if (dataY[ix] != null || mappedNull) { + const yVal = dataY[ix]; + const shouldDrawY = + !!yVal || yVal === 0 || (yVal == null && mappedNull) || (Number.isNaN(yVal) && mappedNaN); + + if (shouldDrawY) { const boxRect = boxRectsBySeries[sidx - 1][ix]; if (!boxRect || boxRect.x >= xDim) { diff --git a/public/app/core/components/TimelineChart/utils.test.ts b/public/app/core/components/TimelineChart/utils.test.ts index a3c181ef42b..6dc82194d0c 100644 --- a/public/app/core/components/TimelineChart/utils.test.ts +++ b/public/app/core/components/TimelineChart/utils.test.ts @@ -8,8 +8,10 @@ import { DataFrame, fieldMatchers, FieldMatcherID, + Field, + SpecialValueMatch, } from '@grafana/data'; -import { LegendDisplayMode, VizLegendOptions } from '@grafana/schema'; +import { LegendDisplayMode, MappingType, VizLegendOptions } from '@grafana/schema'; import { preparePlotFrame } from '../GraphNG/utils'; @@ -17,6 +19,7 @@ import { findNextStateIndex, fmtDuration, getThresholdItems, + hasSpecialMappedValue, makeFramePerSeries, prepareTimelineFields, prepareTimelineLegendItems, @@ -504,3 +507,47 @@ describe('duration', () => { expect(result).toEqual(expected); }); }); + +describe('hasSpecialMappedValue', () => { + const makeField = (mappingsType: MappingType | SpecialValueMatch, optionsMatch: MappingType | SpecialValueMatch) => + ({ + name: 'Field', + type: FieldType.frame, + config: { + mappings: [ + { + type: mappingsType, + options: { match: optionsMatch, result: {} }, + }, + ], + }, + values: [], + }) as Field; + + it.each([ + [[MappingType.SpecialValue, SpecialValueMatch.Null], SpecialValueMatch.Null, true, 'should match Null with Null'], + [[MappingType.SpecialValue, SpecialValueMatch.NaN], SpecialValueMatch.NaN, true, 'should match NaN with NaN'], + [ + [MappingType.SpecialValue, SpecialValueMatch.NullAndNaN], + SpecialValueMatch.NullAndNaN, + true, + 'should match Null and NaN with Null and NaN', + ], + [ + [MappingType.SpecialValue, SpecialValueMatch.NullAndNaN], + SpecialValueMatch.Empty, + false, + 'should NOT match Null and NaN with Empty', + ], + [ + [MappingType.ValueToText, SpecialValueMatch.Null], + SpecialValueMatch.Null, + false, + 'should NOT match non-special value', + ], + ])('%s', ([mappingsType, optionsMatch], valueMatch, expected, _) => { + const field = makeField(mappingsType, optionsMatch); + + expect(hasSpecialMappedValue(field, valueMatch)).toEqual(expected); + }); +}); diff --git a/public/app/core/components/TimelineChart/utils.ts b/public/app/core/components/TimelineChart/utils.ts index a9af674a839..c22dfc574e3 100644 --- a/public/app/core/components/TimelineChart/utils.ts +++ b/public/app/core/components/TimelineChart/utils.ts @@ -20,6 +20,7 @@ import { ThresholdsConfig, applyNullInsertThreshold, nullToValue, + SpecialValueMatch, } from '@grafana/data'; import { maybeSortFrame, NULL_RETAIN } from '@grafana/data/internal'; import { @@ -72,6 +73,12 @@ const defaultConfig: PanelFieldConfig = { fillOpacity: 80, }; +/** Checks if a mapped value of the specified type exists for the given field */ +export const hasSpecialMappedValue = (field: Field, match: SpecialValueMatch): boolean => + field.config.mappings?.some( + (mapping: ValueMapping): boolean => mapping.type === MappingType.SpecialValue && mapping.options.match === match + ) || false; + export const preparePlotConfigBuilder: UPlotConfigPrepFn = ({ frame, theme, @@ -94,15 +101,6 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ( const mode = field.config?.color?.mode; return !(mode && field.display && mode.startsWith('continuous-')); }; - - const hasMappedNull = (field: Field) => { - return ( - field.config.mappings?.some( - (mapping) => mapping.type === MappingType.SpecialValue && mapping.options.match === 'null' - ) || false - ); - }; - const getValueColorFn = (seriesIdx: number, value: unknown) => { const field = frame.fields[seriesIdx]; @@ -121,7 +119,12 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ( mode: mode!, numSeries: frame.fields.length - 1, isDiscrete: (seriesIdx) => isDiscrete(frame.fields[seriesIdx]), - hasMappedNull: (seriesIdx) => hasMappedNull(frame.fields[seriesIdx]), + hasMappedNull: (seriesIdx) => + hasSpecialMappedValue(frame.fields[seriesIdx], SpecialValueMatch.Null) || + hasSpecialMappedValue(frame.fields[seriesIdx], SpecialValueMatch.NullAndNaN), + hasMappedNaN: (seriesIdx) => + hasSpecialMappedValue(frame.fields[seriesIdx], SpecialValueMatch.NaN) || + hasSpecialMappedValue(frame.fields[seriesIdx], SpecialValueMatch.NullAndNaN), mergeValues, rowHeight: rowHeight, colWidth: colWidth, @@ -664,7 +667,7 @@ export function findNextStateIndex(field: Field, datapointIdx: number) { * This function calculates with 30 days month and 365 days year. * adapted from https://gist.github.com/remino/1563878 * @param milliSeconds The duration in milliseconds - * @returns A formated string of the duration + * @returns A formatted string of the duration */ export function fmtDuration(milliSeconds: number): string { if (milliSeconds < 0 || Number.isNaN(milliSeconds)) { From 5135d5c87d37ba39d830ca7cdc046310887f4e2a Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Thu, 12 Jun 2025 14:34:48 -0500 Subject: [PATCH 08/51] Unified storage: Reconstruct index in the background every 24h (#106422) --- pkg/setting/setting.go | 1 + pkg/setting/setting_unified_storage.go | 2 + pkg/storage/unified/resource/search.go | 100 ++++++++++++++++++++++--- pkg/storage/unified/resource/server.go | 3 + pkg/storage/unified/search/options.go | 9 ++- 5 files changed, 102 insertions(+), 13 deletions(-) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index ff53a2ffb5b..3ed1cc48e37 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -556,6 +556,7 @@ type Cfg struct { IndexMaxBatchSize int IndexFileThreshold int IndexMinCount int + IndexRebuildInterval time.Duration EnableSharding bool MemberlistBindAddr string MemberlistAdvertiseAddr string diff --git a/pkg/setting/setting_unified_storage.go b/pkg/setting/setting_unified_storage.go index 8da4e942e69..b77329bd03f 100644 --- a/pkg/setting/setting_unified_storage.go +++ b/pkg/setting/setting_unified_storage.go @@ -63,6 +63,8 @@ func (cfg *Cfg) setUnifiedStorageConfig() { cfg.InstanceID = section.Key("instance_id").String() cfg.IndexFileThreshold = section.Key("index_file_threshold").MustInt(10) cfg.IndexMinCount = section.Key("index_min_count").MustInt(1) + // default to 24 hours because usage insights summarizes the data every 24 hours + cfg.IndexRebuildInterval = section.Key("index_rebuild_interval").MustDuration(24 * time.Hour) cfg.SprinklesApiServer = section.Key("sprinkles_api_server").String() cfg.SprinklesApiServerPageLimit = section.Key("sprinkles_api_server_page_limit").MustInt(100) cfg.CACertPath = section.Key("ca_cert_path").String() diff --git a/pkg/storage/unified/resource/search.go b/pkg/storage/unified/resource/search.go index 421a104beb8..b6ecfe0bd72 100644 --- a/pkg/storage/unified/resource/search.go +++ b/pkg/storage/unified/resource/search.go @@ -121,6 +121,9 @@ type searchSupport struct { // testing clientIndexEventsChan chan *IndexEvent + + // periodic rebuilding of the indexes to keep usage insights up to date + rebuildInterval time.Duration } var ( @@ -153,6 +156,7 @@ func newSearchSupport(opts SearchOptions, storage StorageBackend, access types.A clientIndexEventsChan: opts.IndexEventsChan, indexEventsChan: make(chan *IndexEvent), indexQueueProcessors: make(map[string]*indexQueueProcessor), + rebuildInterval: opts.RebuildInterval, } info, err := opts.Resources.GetDocumentBuilders() @@ -377,34 +381,52 @@ func (s *searchSupport) GetStats(ctx context.Context, req *resourcepb.ResourceSt return rsp, nil } -// init is called during startup. any failure will block startup and continued execution -func (s *searchSupport) init(ctx context.Context) error { - ctx, span := s.tracer.Start(ctx, tracingPrexfixSearch+"Init") - defer span.End() - start := time.Now().Unix() - +func (s *searchSupport) buildIndexes(ctx context.Context, rebuild bool) (int, error) { totalBatchesIndexed := 0 group := errgroup.Group{} group.SetLimit(s.initWorkers) stats, err := s.storage.GetResourceStats(ctx, "", s.initMinSize) if err != nil { - return err + return 0, err } for _, info := range stats { + // only periodically rebuild the dashboard index, specifically to update the usage insights data + if rebuild && info.Resource != dashboardv1.DASHBOARD_RESOURCE { + continue + } + group.Go(func() error { - s.log.Debug("initializing search index", "namespace", info.Namespace, "group", info.Group, "resource", info.Resource) + if rebuild { + // we need to clear the cache to make sure we get the latest usage insights data + s.builders.clearNamespacedCache(info.NamespacedResource) + } + s.log.Debug("building index", "namespace", info.Namespace, "group", info.Group, "resource", info.Resource) totalBatchesIndexed++ - _, _, err = s.build(ctx, info.NamespacedResource, info.Count, info.ResourceVersion) + _, _, err := s.build(ctx, info.NamespacedResource, info.Count, info.ResourceVersion) return err }) } err = group.Wait() + if err != nil { + return totalBatchesIndexed, err + } + + return totalBatchesIndexed, nil +} + +func (s *searchSupport) init(ctx context.Context) error { + ctx, span := s.tracer.Start(ctx, tracingPrexfixSearch+"Init") + defer span.End() + start := time.Now().Unix() + + totalBatchesIndexed, err := s.buildIndexes(ctx, false) if err != nil { return err } + span.AddEvent("namespaces indexed", trace.WithAttributes(attribute.Int("namespaced_indexed", totalBatchesIndexed))) // Now start listening for new events @@ -428,6 +450,12 @@ func (s *searchSupport) init(ctx context.Context) error { go s.monitorIndexEvents(ctx) + // since usage insights is not in unified storage, we need to periodically rebuild the index + // to make sure these data points are up to date. + if s.rebuildInterval > 0 { + go s.startPeriodicRebuild(watchctx) + } + end := time.Now().Unix() s.log.Info("search index initialized", "duration_secs", end-start, "total_docs", s.search.TotalDocs()) if s.indexMetrics != nil { @@ -508,6 +536,54 @@ func (s *searchSupport) monitorIndexEvents(ctx context.Context) { } } +func (s *searchSupport) startPeriodicRebuild(ctx context.Context) { + ticker := time.NewTicker(s.rebuildInterval) + defer ticker.Stop() + + s.log.Info("starting periodic index rebuild", "interval", s.rebuildInterval) + + for { + select { + case <-ctx.Done(): + s.log.Info("stopping periodic index rebuild due to context cancellation") + return + case <-ticker.C: + s.log.Info("starting periodic index rebuild") + if err := s.rebuildDashboardIndexes(ctx); err != nil { + s.log.Error("error during periodic index rebuild", "error", err) + } else { + s.log.Info("periodic index rebuild completed successfully") + } + } + } +} + +func (s *searchSupport) rebuildDashboardIndexes(ctx context.Context) error { + ctx, span := s.tracer.Start(ctx, tracingPrexfixSearch+"RebuildDashboardIndexes") + defer span.End() + + start := time.Now() + s.log.Info("rebuilding all search indexes") + + totalBatchesIndexed, err := s.buildIndexes(ctx, true) + if err != nil { + return fmt.Errorf("failed to rebuild dashboard indexes: %w", err) + } + + end := time.Now() + duration := end.Sub(start) + s.log.Info("completed rebuilding all dashboard search indexes", + "duration", duration, + "rebuilt_indexes", totalBatchesIndexed, + "total_docs", s.search.TotalDocs()) + + if s.indexMetrics != nil { + s.indexMetrics.IndexCreationTime.WithLabelValues().Observe(duration.Seconds()) + } + + return nil +} + func (s *searchSupport) getOrCreateIndex(ctx context.Context, key NamespacedResource) (ResourceIndex, error) { if s == nil || s.search == nil { return nil, fmt.Errorf("search is not configured properly (missing unifiedStorageSearch feature toggle?)") @@ -770,3 +846,9 @@ func (s *searchSupport) getOrCreateIndexQueueProcessor(index ResourceIndex, nsr s.indexQueueProcessors[key] = indexQueueProcessor return indexQueueProcessor, nil } + +func (s *builderCache) clearNamespacedCache(key NamespacedResource) { + s.mu.Lock() + defer s.mu.Unlock() + s.ns.Remove(key) +} diff --git a/pkg/storage/unified/resource/server.go b/pkg/storage/unified/resource/server.go index 2aa15a547ca..f25f45927b0 100644 --- a/pkg/storage/unified/resource/server.go +++ b/pkg/storage/unified/resource/server.go @@ -158,6 +158,9 @@ type SearchOptions struct { // Channel to watch for index events (for testing) IndexEventsChan chan *IndexEvent + + // Interval for periodic index rebuilds (0 disables periodic rebuilds) + RebuildInterval time.Duration } type ResourceServerOptions struct { diff --git a/pkg/storage/unified/search/options.go b/pkg/storage/unified/search/options.go index 1dae7eebe02..4fd15ac2679 100644 --- a/pkg/storage/unified/search/options.go +++ b/pkg/storage/unified/search/options.go @@ -33,10 +33,11 @@ func NewSearchOptions(features featuremgmt.FeatureToggles, cfg *setting.Cfg, tra } return resource.SearchOptions{ - Backend: bleve, - Resources: docs, - WorkerThreads: cfg.IndexWorkers, - InitMinCount: cfg.IndexMinCount, + Backend: bleve, + Resources: docs, + WorkerThreads: cfg.IndexWorkers, + InitMinCount: cfg.IndexMinCount, + RebuildInterval: cfg.IndexRebuildInterval, }, nil } return resource.SearchOptions{}, nil From 0016b574860ea7aa3ed52bfb9df4ebc7c3a9d0e1 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Thu, 12 Jun 2025 17:00:09 -0400 Subject: [PATCH 09/51] Alerting: Add OAuth2 Support for Webhook Receiver (#106302) * Add to available channels * Export * Fix bug in deeply nested secrets BE: Slice re-use bug when traversing deeply. FE: Only at most one level of nesting was being taken into account when determining secureFields keys. This change adds a new field on NotificationChannelOption: secureFieldKey. This is populated on API GET via transform. This change gives us the option to hardcode secureFieldKey in the backend and no longer calculate the key via settings topology. * Update grafana/alerting to 3e20fda3b872 * Prettier * Linting * Fix IntegrationConfig test to catch secure field mismatch --- go.mod | 2 +- go.sum | 4 +- .../api/tooling/definitions/contact_points.go | 36 +++ pkg/services/ngalert/models/receivers.go | 13 +- pkg/services/ngalert/models/receivers_test.go | 9 +- .../channels_config/available_channels.go | 203 +++++++++++--- .../available_channels_test.go | 13 +- .../alerting/unified/api/alertmanagerApi.ts | 21 +- .../receivers/form/ChannelOptions.tsx | 10 +- .../receivers/form/ChannelSubForm.tsx | 29 +- .../form/GrafanaReceiverForm.test.tsx | 139 +++++++++- .../GrafanaReceiverForm.test.tsx.snap | 26 +- .../receivers/form/fields/OptionField.tsx | 15 +- .../receivers/form/fields/SubformField.tsx | 6 +- .../alerting/unified/mockGrafanaNotifiers.ts | 252 ++++++++++++++++++ public/app/types/alerting.ts | 1 + 16 files changed, 697 insertions(+), 82 deletions(-) diff --git a/go.mod b/go.mod index 6f85332903a..b5d8de4cb23 100644 --- a/go.mod +++ b/go.mod @@ -77,7 +77,7 @@ require ( github.com/googleapis/gax-go/v2 v2.14.1 // @grafana/grafana-backend-group github.com/gorilla/mux v1.8.1 // @grafana/grafana-backend-group github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // @grafana/grafana-app-platform-squad - github.com/grafana/alerting v0.0.0-20250605155607-02235095d018 // @grafana/alerting-backend + github.com/grafana/alerting v0.0.0-20250610043455-3e20fda3b872 // @grafana/alerting-backend github.com/grafana/authlib v0.0.0-20250515162837-2f4a8263eabb // @grafana/identity-access-team github.com/grafana/authlib/types v0.0.0-20250325095148-d6da9c164a7d // @grafana/identity-access-team github.com/grafana/dataplane/examples v0.0.1 // @grafana/observability-metrics diff --git a/go.sum b/go.sum index e8c24ae49bc..2d4348b27c0 100644 --- a/go.sum +++ b/go.sum @@ -1569,8 +1569,8 @@ github.com/gorilla/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7Fsg github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5THxAzdVpqr6/geYxZytqFMBCOtn/ujyeo= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= -github.com/grafana/alerting v0.0.0-20250605155607-02235095d018 h1:sGEBflMw3DUsfV4SCl1at+QRwGi5h29iiAuk8ZZn5Mw= -github.com/grafana/alerting v0.0.0-20250605155607-02235095d018/go.mod h1:e2nDocz4yTgLYGJFMrlL3DTjdMTEYPABorKc191b8/w= +github.com/grafana/alerting v0.0.0-20250610043455-3e20fda3b872 h1:bhUBPxMHtQB+AJaPcASElgwC7WFjWQvEiD3FoMEAv0E= +github.com/grafana/alerting v0.0.0-20250610043455-3e20fda3b872/go.mod h1:VANfU4LMnOR4Gv4hkCN9fGyQ/8Lf2DKOvzG70vI2IS4= github.com/grafana/authlib v0.0.0-20250515162837-2f4a8263eabb h1:oTl2j6/4miQUYmXANp2pBuYCWA5f8NVYFfCWpczpFso= github.com/grafana/authlib v0.0.0-20250515162837-2f4a8263eabb/go.mod h1:PBtQaXwkFu4BAt2aXsR7w8p8NVpdjV5aJYhqRDei9Us= github.com/grafana/authlib/types v0.0.0-20250325095148-d6da9c164a7d h1:34E6btDAhdDOiSEyrMaYaHwnJpM8w9QKzVQZIBzLNmM= diff --git a/pkg/services/ngalert/api/tooling/definitions/contact_points.go b/pkg/services/ngalert/api/tooling/definitions/contact_points.go index 9acbe144e88..7f98ad65c92 100644 --- a/pkg/services/ngalert/api/tooling/definitions/contact_points.go +++ b/pkg/services/ngalert/api/tooling/definitions/contact_points.go @@ -324,6 +324,7 @@ type WebhookIntegration struct { Message *string `json:"message,omitempty" yaml:"message,omitempty" hcl:"message"` TLSConfig *TLSConfig `json:"tlsConfig,omitempty" yaml:"tlsConfig,omitempty" hcl:"tlsConfig,block"` HMACConfig *HMACConfig `json:"hmacConfig,omitempty" yaml:"hmacConfig,omitempty" hcl:"hmacConfig,block"` + HTTPConfig *HTTPClientConfig `json:"http_config,omitempty" yaml:"http_config,omitempty" hcl:"http_config,block"` Payload *CustomPayload `json:"payload,omitempty" yaml:"payload,omitempty" hcl:"payload,block"` } @@ -343,6 +344,41 @@ type HMACConfig struct { TimestampHeader string `yaml:"timestampHeader,omitempty" json:"timestampHeader,omitempty" hcl:"timestamp_header"` } +// HTTPClientConfig holds common configurations for notifier HTTP clients. +type HTTPClientConfig struct { + OAuth2Config *OAuth2Config `json:"oauth2,omitempty" yaml:"oauth2,omitempty" hcl:"oauth2,block"` +} + +type ProxyConfig struct { + // ProxyURL is the HTTP proxy server to use to connect to the targets. + ProxyURL *string `yaml:"proxy_url,omitempty" json:"proxy_url,omitempty" hcl:"proxy_url"` + // NoProxy contains addresses that should not use a proxy. + NoProxy *string `yaml:"no_proxy,omitempty" json:"no_proxy,omitempty" hcl:"no_proxy"` + // ProxyFromEnvironment uses environment HTTP_PROXY, HTTPS_PROXY and NO_PROXY to determine proxies. + ProxyFromEnvironment *bool `yaml:"proxy_from_environment,omitempty" json:"proxy_from_environment,omitempty" hcl:"proxy_from_environment"` + // ProxyConnectHeader optionally specifies headers to send to proxies during CONNECT requests. + ProxyConnectHeader *map[string]string `yaml:"proxy_connect_header,omitempty" json:"proxy_connect_header,omitempty" hcl:"proxy_connect_header"` +} + +type OAuth2Config struct { + // ClientID is the OAuth2 client ID. + ClientID string `json:"client_id" yaml:"client_id" hcl:"client_id"` + // ClientSecret is the OAuth2 client secret. + ClientSecret *Secret `json:"client_secret" yaml:"client_secret" hcl:"client_secret"` + // TokenURL is the URL to get the OAuth2 token. + TokenURL string `json:"token_url" yaml:"token_url" hcl:"token_url"` + + // Scopes is the optional list of OAuth2 scopes. + Scopes *[]string `json:"scopes,omitempty" yaml:"scopes,omitempty" hcl:"scopes"` + // EndpointParams is the optional map of additional parameters to include in the token request. + EndpointParams *map[string]string `json:"endpoint_params,omitempty" yaml:"endpoint_params,omitempty" hcl:"endpoint_params"` + // TLSConfig is the optional TLS configuration to use for the OAuth2 token request. + TLSConfig *TLSConfig `json:"tls_config,omitempty" yaml:"tls_config,omitempty" hcl:"tls_config,block"` + + // ProxyConfig is the optional proxy configuration to use for the OAuth2 token request. + ProxyConfig *ProxyConfig `json:"proxy_config,omitempty" yaml:"proxy_config,omitempty" hcl:"proxy_config,block"` +} + type WecomIntegration struct { DisableResolveMessage *bool `json:"-" yaml:"-" hcl:"disable_resolve_message"` diff --git a/pkg/services/ngalert/models/receivers.go b/pkg/services/ngalert/models/receivers.go index 60d95aaad04..5e1a16485ee 100644 --- a/pkg/services/ngalert/models/receivers.go +++ b/pkg/services/ngalert/models/receivers.go @@ -191,8 +191,12 @@ func (f IntegrationFieldPath) String() string { return strings.Join(f, ".") } -func (f IntegrationFieldPath) Append(segment string) IntegrationFieldPath { - return append(f, segment) +func (f IntegrationFieldPath) With(segment string) IntegrationFieldPath { + // Copy the existing path to avoid modifying the original slice. + newPath := make(IntegrationFieldPath, len(f)+1) + copy(newPath, f) + newPath[len(newPath)-1] = segment + return newPath } // IntegrationConfigFromType returns an integration configuration for a given integration type. If the integration type is @@ -250,11 +254,12 @@ func (config *IntegrationConfig) GetSecretFields() []IntegrationFieldPath { func traverseFields(flds map[string]IntegrationField, parentPath IntegrationFieldPath, predicate func(i IntegrationField) bool) []IntegrationFieldPath { var result []IntegrationFieldPath for key, field := range flds { + path := parentPath.With(key) if predicate(field) { - result = append(result, parentPath.Append(key)) + result = append(result, path) } if len(field.Fields) > 0 { - result = append(result, traverseFields(field.Fields, parentPath.Append(key), predicate)...) + result = append(result, traverseFields(field.Fields, path, predicate)...) } } return result diff --git a/pkg/services/ngalert/models/receivers_test.go b/pkg/services/ngalert/models/receivers_test.go index d83f346f0ae..9ca835e7fe9 100644 --- a/pkg/services/ngalert/models/receivers_test.go +++ b/pkg/services/ngalert/models/receivers_test.go @@ -244,11 +244,14 @@ func TestIntegrationConfig(t *testing.T) { allSecrets[key] = struct{}{} } - for field := range config.Fields { - _, isSecret := allSecrets[field] - assert.Equalf(t, isSecret, config.IsSecureField(NewIntegrationFieldPath(field)), "field '%s' is expected to be secret", field) + secretFields := config.GetSecretFields() + for _, path := range secretFields { + _, isSecret := allSecrets[path.String()] + assert.Equalf(t, isSecret, config.IsSecureField(path), "field '%s' is expected to be secret", path) + delete(allSecrets, path.String()) } assert.False(t, config.IsSecureField(IntegrationFieldPath{"__--**unknown_field**--__"})) + assert.Empty(t, allSecrets, "mismatched secret fields for integration type %s: %v", integrationType, allSecrets) }) } diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels.go b/pkg/services/ngalert/notifier/channels_config/available_channels.go index 5b70d7f58fa..b1df07f320a 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels.go @@ -113,6 +113,163 @@ func GetAvailableNotifiers() []*NotifierPlugin { }, } + tlsSubformOptions := func() []NotifierOption { + return []NotifierOption{ + { + Label: "Disable certificate verification", + Element: ElementTypeCheckbox, + Description: "Do not verify the server's certificate chain and host name.", + PropertyName: "insecureSkipVerify", + Required: false, + }, + { + Label: "CA Certificate", + Element: ElementTypeTextArea, + Description: "Certificate in PEM format to use when verifying the server's certificate chain.", + InputType: InputTypeText, + PropertyName: "caCertificate", + Required: false, + Secure: true, + }, + { + Label: "Client Certificate", + Element: ElementTypeTextArea, + Description: "Client certificate in PEM format to use when connecting to the server.", + InputType: InputTypeText, + PropertyName: "clientCertificate", + Required: false, + Secure: true, + }, + { + Label: "Client Key", + Element: ElementTypeTextArea, + Description: "Client key in PEM format to use when connecting to the server.", + InputType: InputTypeText, + PropertyName: "clientKey", + Required: false, + Secure: true, + }, + } + } + + proxyOption := func() NotifierOption { + return NotifierOption{ // New in 12.1. + Label: "Proxy Config", + PropertyName: "proxy_config", + Description: "Optional proxy configuration.", + Element: ElementTypeSubform, + SubformOptions: []NotifierOption{ + { + Label: "Proxy URL", + PropertyName: "proxy_url", + Description: "HTTP proxy server to use to connect to the targets.", + Element: ElementTypeInput, + InputType: InputTypeText, + Placeholder: "https://proxy.example.com", + Required: false, + Secure: false, + }, + { + Label: "Proxy from environment", + PropertyName: "proxy_from_environment", + Description: "Use environment HTTP_PROXY, HTTPS_PROXY and NO_PROXY to determine proxies.", + Element: ElementTypeCheckbox, + Required: false, + Secure: false, + }, + { + Label: "No Proxy", + PropertyName: "no_proxy", + Description: "Comma-separated list of addresses that should not use a proxy.", + Element: ElementTypeInput, + InputType: InputTypeText, + Placeholder: "example.com,1.2.3.4", + Required: false, + Secure: false, + }, + { + Label: "Proxy Connect Header", + PropertyName: "proxy_connect_header", + Description: "Optional headers to send to proxies during CONNECT requests.", + Element: ElementTypeKeyValueMap, + InputType: InputTypeText, + Required: false, + Secure: false, + }, + }, + } + } + + commonHttpClientOption := func() NotifierOption { + return NotifierOption{ // New in 12.1. + Label: "HTTP Config", + PropertyName: "http_config", + Description: "Common HTTP client options.", + Element: ElementTypeSubform, + SubformOptions: []NotifierOption{ + { // New in 12.1. + Label: "OAuth2", + PropertyName: "oauth2", + Description: "OAuth2 configuration options", + Element: ElementTypeSubform, + SubformOptions: []NotifierOption{ + { + Label: "Token URL", + PropertyName: "token_url", + Element: ElementTypeInput, + Description: "URL for the access token endpoint.", + InputType: InputTypeText, + Required: true, + Secure: false, + }, + { + Label: "Client ID", + PropertyName: "client_id", + Element: ElementTypeInput, + Description: "Client ID to use when authenticating.", + InputType: InputTypeText, + Required: true, + Secure: false, + }, + { + Label: "Client Secret", + PropertyName: "client_secret", + Element: ElementTypeInput, + Description: "Client secret to use when authenticating.", + InputType: InputTypeText, + Required: true, + Secure: true, + }, + { + Label: "Scopes", + PropertyName: "scopes", + Element: ElementStringArray, + Description: "Optional scopes to request when obtaining an access token.", + Required: false, + Secure: false, + }, + { + Label: "Endpoint Parameters", + PropertyName: "endpoint_params", + Element: ElementTypeKeyValueMap, + Description: "Optional parameters to append to the access token request.", + Required: false, + Secure: false, + }, + { + Label: "TLS", + PropertyName: "tls_config", + Description: "Optional TLS configuration options for OAuth2 requests.", + Element: ElementTypeSubform, + SubformOptions: tlsSubformOptions(), + }, + proxyOption(), + }, + }, + }, + } + } + return []*NotifierPlugin{ { Type: "dingding", @@ -1006,46 +1163,11 @@ func GetAvailableNotifiers() []*NotifierPlugin { }, { - Label: "TLS", - PropertyName: "tlsConfig", - Description: "TLS configuration options", - Element: ElementTypeSubform, - SubformOptions: []NotifierOption{ - { - Label: "Disable certificate verification", - Element: ElementTypeCheckbox, - Description: "Do not verify the server's certificate chain and host name.", - PropertyName: "insecureSkipVerify", - Required: false, - }, - { - Label: "CA Certificate", - Element: ElementTypeTextArea, - Description: "Certificate in PEM format to use when verifying the server's certificate chain.", - InputType: InputTypeText, - PropertyName: "caCertificate", - Required: false, - Secure: true, - }, - { - Label: "Client Certificate", - Element: ElementTypeTextArea, - Description: "Client certificate in PEM format to use when connecting to the server.", - InputType: InputTypeText, - PropertyName: "clientCertificate", - Required: false, - Secure: true, - }, - { - Label: "Client Key", - Element: ElementTypeTextArea, - Description: "Client key in PEM format to use when connecting to the server.", - InputType: InputTypeText, - PropertyName: "clientKey", - Required: false, - Secure: true, - }, - }, + Label: "TLS", + PropertyName: "tlsConfig", + Description: "TLS configuration options", + Element: ElementTypeSubform, + SubformOptions: tlsSubformOptions(), }, { Label: "HMAC Signature", @@ -1083,6 +1205,7 @@ func GetAvailableNotifiers() []*NotifierPlugin { }, }, }, + commonHttpClientOption(), // New in 12.1. }, }, { diff --git a/pkg/services/ngalert/notifier/channels_config/available_channels_test.go b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go index 0fe4330faf6..ed7564db17d 100644 --- a/pkg/services/ngalert/notifier/channels_config/available_channels_test.go +++ b/pkg/services/ngalert/notifier/channels_config/available_channels_test.go @@ -22,7 +22,18 @@ func TestGetSecretKeysForContactPointType(t *testing.T) { {receiverType: "sensugo", expectedSecretFields: []string{"apikey"}}, {receiverType: "teams", expectedSecretFields: []string{}}, {receiverType: "telegram", expectedSecretFields: []string{"bottoken"}}, - {receiverType: "webhook", expectedSecretFields: []string{"password", "authorization_credentials", "tlsConfig.caCertificate", "tlsConfig.clientCertificate", "tlsConfig.clientKey", "hmacConfig.secret"}}, + {receiverType: "webhook", expectedSecretFields: []string{ + "password", + "authorization_credentials", + "tlsConfig.caCertificate", + "tlsConfig.clientCertificate", + "tlsConfig.clientKey", + "hmacConfig.secret", + "http_config.oauth2.client_secret", + "http_config.oauth2.tls_config.caCertificate", + "http_config.oauth2.tls_config.clientCertificate", + "http_config.oauth2.tls_config.clientKey", + }}, {receiverType: "wecom", expectedSecretFields: []string{"url", "secret"}}, {receiverType: "prometheus-alertmanager", expectedSecretFields: []string{"basicAuthPassword"}}, {receiverType: "discord", expectedSecretFields: []string{"url"}}, diff --git a/public/app/features/alerting/unified/api/alertmanagerApi.ts b/public/app/features/alerting/unified/api/alertmanagerApi.ts index 5f5bb7728c3..0d579e37e53 100644 --- a/public/app/features/alerting/unified/api/alertmanagerApi.ts +++ b/public/app/features/alerting/unified/api/alertmanagerApi.ts @@ -14,7 +14,7 @@ import { GrafanaAlertingConfiguration, Matcher, } from '../../../../plugins/datasource/alertmanager/types'; -import { NotifierDTO } from '../../../../types'; +import { NotificationChannelOption, NotifierDTO } from '../../../../types'; import { withPerformanceLogging } from '../Analytics'; import { matcherToMatcherField } from '../utils/alertmanager'; import { @@ -106,6 +106,25 @@ export const alertmanagerApi = alertingApi.injectEndpoints({ grafanaNotifiers: build.query({ query: () => ({ url: '/api/alert-notifiers' }), + transformResponse: (response: NotifierDTO[]) => { + const populateSecureFieldKey = ( + option: NotificationChannelOption, + prefix: string + ): NotificationChannelOption => ({ + ...option, + secureFieldKey: option.secure && !option.secureFieldKey ? `${prefix}${option.propertyName}` : undefined, + subformOptions: option.subformOptions?.map((suboption) => + populateSecureFieldKey(suboption, `${prefix}${option.propertyName}.`) + ), + }); + + return response.map((notifier) => ({ + ...notifier, + options: notifier.options.map((option) => { + return populateSecureFieldKey(option, ''); + }), + })); + }, }), // this endpoint requires administrator privileges diff --git a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx index 0648a964d4a..e402c4f8875 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx @@ -18,7 +18,7 @@ export interface Props { selectedChannelOptions: NotificationChannelOption[]; onResetSecureField: (key: string) => void; - onDeleteSubform?: (propertyName: string) => void; + onDeleteSubform?: (settingsPath: string, option: NotificationChannelOption) => void; errors?: FieldErrors; /** * The path for the integration in the array of integrations. @@ -66,7 +66,7 @@ export function ChannelOptions({ return null; } - if (secureFields && secureFields[option.propertyName]) { + if (secureFields && secureFields[option.secureFieldKey ?? option.propertyName]) { return ( ({ > onResetSecureField(option.propertyName)} + onReset={() => onResetSecureField(option.secureFieldKey ?? option.propertyName)} isConfigured /> @@ -85,7 +85,7 @@ export function ChannelOptions({ const error: FieldError | DeepMap | undefined = ( (option.secure ? errors?.secureFields : errors?.settings) as DeepMap | undefined - )?.[option.propertyName]; + )?.[option.secureFieldKey ?? option.propertyName]; const defaultValue = defaultValues?.settings?.[option.propertyName]; @@ -122,6 +122,7 @@ const determineRequired = ( return option.required ? 'Required' : false; } + // TODO: This doesn't work with nested secureFields. const dependentOn = Boolean(settings[option.dependsOn]) || Boolean(secureFields[option.dependsOn]); if (dependentOn) { @@ -140,5 +141,6 @@ const determineReadOnly = ( return false; } + // TODO: This doesn't work with nested secureFields. return Boolean(settings[option.dependsOn]) || Boolean(secureFields[option.dependsOn]); }; diff --git a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx index e06d26bc4ed..7e47f08b5a2 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx @@ -2,11 +2,12 @@ import { css } from '@emotion/css'; import { sortBy } from 'lodash'; import * as React from 'react'; import { useEffect, useMemo } from 'react'; -import { Controller, FieldErrors, useFormContext, useWatch } from 'react-hook-form'; +import { Controller, FieldErrors, useFormContext } from 'react-hook-form'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; import { Alert, Button, Field, Select, Stack, Text, useStyles2 } from '@grafana/ui'; +import { NotificationChannelOption } from 'app/types'; import { useUnifiedAlertingSelector } from '../../../hooks/useUnifiedAlertingSelector'; import { @@ -99,9 +100,6 @@ export function ChannelSubForm({ return () => subscription.unsubscribe(); }, [selectedType, initialValues, setValue, settingsFieldPath, typeFieldPath, watch]); - // const [_secureFields, setSecureFields] = useState>(secureFields ?? {}); - const formSecureFields = useWatch({ control, name: `${channelFieldPath}.secureFields` }); - const onResetSecureField = (key: string) => { // formSecureFields might not be up to date if this function is called multiple times in a row const currentSecureFields = getValues(`${channelFieldPath}.secureFields`); @@ -110,12 +108,29 @@ export function ChannelSubForm({ } }; - const onDeleteSubform = (propertyName: string) => { - const relatedSecureFields = Object.keys(formSecureFields).filter((key) => key.startsWith(propertyName)); + const findSecureFieldsRecursively = (options: NotificationChannelOption[]): string[] => { + const secureFields: string[] = []; + options?.forEach((option) => { + if (option.secure && option.secureFieldKey) { + secureFields.push(option.secureFieldKey); + } + if (option.subformOptions) { + secureFields.push(...findSecureFieldsRecursively(option.subformOptions)); + } + }); + return secureFields; + }; + + const onDeleteSubform = (settingsPath: string, option: NotificationChannelOption) => { + // Get all subform options with secure=true recursively. + const relatedSecureFields = findSecureFieldsRecursively(option.subformOptions ?? []); relatedSecureFields.forEach((key) => { onResetSecureField(key); }); - setValue(`${channelFieldPath}.settings.${propertyName}`, undefined); + const fieldPath = settingsPath.startsWith(`${channelFieldPath}.settings.`) + ? settingsPath.slice(`${channelFieldPath}.settings.`.length) + : settingsPath; + setValue(`${settingsFieldPath}.${fieldPath}`, undefined); }; const typeOptions = useMemo( diff --git a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx index f8962f5c174..937c64f4afb 100644 --- a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx @@ -65,12 +65,26 @@ const ui = { webhook: { url: byRole('textbox', { name: /^URL/ }), tlsConfig: { - header: byRole('heading', { name: /TLS/ }), - caCertificate: byRole('textbox', { name: /^CA certificate/ }), - clientCert: byRole('textbox', { name: /^Client certificate/ }), - clientKey: byRole('textbox', { name: /^Client key/ }), + container: byTestId('items.0.settings.tlsConfig.container'), + caCertificate: byRole('textbox', { name: /^CA Certificate/ }), + clientCert: byRole('textbox', { name: /^Client Certificate/ }), + clientKey: byRole('textbox', { name: /^Client Key/ }), deleteButton: byTestId('items.0.settings.tlsConfig.delete-button'), }, + httpConfig: { + container: byTestId('items.0.settings.http_config.container'), + oauth2: { + container: byTestId('items.0.settings.http_config.oauth2.container'), + clientSecret: byRole('textbox', { name: /^Client Secret/ }), + tls_config: { + container: byTestId('items.0.settings.http_config.oauth2.tls_config.container'), + caCertificate: byRole('textbox', { name: /^CA Certificate/ }), + clientCert: byRole('textbox', { name: /^Client Certificate/ }), + clientKey: byRole('textbox', { name: /^Client Key/ }), + deleteButton: byTestId('items.0.settings.http_config.oauth2.tls_config.delete-button'), + }, + }, + }, optionalSettings: byRole('button', { name: /optional webhook settings/i }), }, }; @@ -416,6 +430,81 @@ describe('GrafanaReceiverForm', () => { }); describe('Webhook contact point', () => { + it('should mark secure fields as configured when values exist', async () => { + const contactPointName = 'webhook-test'; + const contactPoint = alertingFactory.alertmanager.grafana.contactPoint + .withIntegrations((integrationFactory) => [ + integrationFactory + .webhook() + .params({ + settings: { + url: 'http://example.com', + tlsConfig: { + insecureSkipVerify: false, + }, + http_config: { + oauth2: { + client_id: 'client-id', + token_url: 'http://example.com/oauth2/token', + scopes: ['scope1', 'scope2'], + endpoint_params: { + param1: 'value1', + param2: 'value2', + }, + tls_config: { + insecureSkipVerify: false, + }, + proxy_config: { + proxy_url: 'http://example.com/proxy', + no_proxy: 'example.com', + proxy_from_environment: true, + proxy_connect_header: { + 'X-Custom-Header': 'custom-value', + }, + }, + }, + }, + }, + secureFields: { + 'tlsConfig.caCertificate': true, + 'tlsConfig.clientCertificate': true, + 'tlsConfig.clientKey': true, + 'http_config.oauth2.client_secret': true, + 'http_config.oauth2.tls_config.caCertificate': true, + 'http_config.oauth2.tls_config.clientCertificate': true, + 'http_config.oauth2.tls_config.clientKey': true, + }, + }) + .build(), + ]) + .build({ id: 'webhook-id', name: contactPointName, metadata: { name: contactPointName } }); + + const { user } = renderWithProvider(); + + await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument()); + await waitFor(() => expect(ui.webhook.optionalSettings.query()).toBeInTheDocument()); + await user.click(ui.webhook.optionalSettings.get()); + + const tlsContainer = await ui.webhook.tlsConfig.container.find(); + const caCertField = ui.webhook.tlsConfig.caCertificate.get(tlsContainer); + const clientCertField = ui.webhook.tlsConfig.clientCert.get(tlsContainer); + const clientKeyField = ui.webhook.tlsConfig.clientKey.get(tlsContainer); + expect(caCertField).toHaveValue('configured'); + expect(clientCertField).toHaveValue('configured'); + expect(clientKeyField).toHaveValue('configured'); + + // Deeply nested secure fields. + const oauth2Container = await ui.webhook.httpConfig.oauth2.container.find(); + const clientSecretField = ui.webhook.httpConfig.oauth2.clientSecret.get(oauth2Container); + const oauthCaCertField = ui.webhook.httpConfig.oauth2.tls_config.caCertificate.get(oauth2Container); + const oauthClientCertField = ui.webhook.httpConfig.oauth2.tls_config.clientCert.get(oauth2Container); + const oauthClientKeyField = ui.webhook.httpConfig.oauth2.tls_config.clientKey.get(oauth2Container); + expect(clientSecretField).toHaveValue('configured'); + expect(oauthCaCertField).toHaveValue('configured'); + expect(oauthClientCertField).toHaveValue('configured'); + expect(oauthClientKeyField).toHaveValue('configured'); + }); + it('should properly remove TLS config when deleted', async () => { const contactPointName = 'webhook-test'; const contactPoint = alertingFactory.alertmanager.grafana.contactPoint @@ -431,6 +520,35 @@ describe('GrafanaReceiverForm', () => { clientKey: 'client-key', insecureSkipVerify: false, }, + http_config: { + oauth2: { + client_id: 'client-id', + token_url: 'http://example.com/oauth2/token', + scopes: ['scope1', 'scope2'], + endpoint_params: { + param1: 'value1', + param2: 'value2', + }, + tls_config: { + // This tls config has existing values via secureFields, delete should remove this correctly as well. + insecureSkipVerify: false, + }, + proxy_config: { + proxy_url: 'http://example.com/proxy', + no_proxy: 'example.com', + proxy_from_environment: true, + proxy_connect_header: { + 'X-Custom-Header': 'custom-value', + }, + }, + }, + }, + }, + secureFields: { + 'http_config.oauth2.client_secret': true, + 'http_config.oauth2.tls_config.caCertificate': true, + 'http_config.oauth2.tls_config.clientCertificate': true, + 'http_config.oauth2.tls_config.clientKey': true, }, }) .build(), @@ -448,9 +566,14 @@ describe('GrafanaReceiverForm', () => { // Find and click the delete button next to TLS config await user.click(ui.webhook.optionalSettings.get()); - expect(await ui.webhook.tlsConfig.header.find(undefined)).toBeInTheDocument(); + // Delete new tlsConfig values. + expect(await ui.webhook.tlsConfig.container.find()).toBeInTheDocument(); await user.click(await ui.webhook.tlsConfig.deleteButton.find()); + // Delete existing oauth2 values. + expect(await ui.webhook.httpConfig.oauth2.tls_config.container.find()).toBeInTheDocument(); + await user.click(await ui.webhook.httpConfig.oauth2.tls_config.deleteButton.find()); + await user.click(ui.saveButton.get()); const requests = await capture; @@ -467,6 +590,12 @@ describe('GrafanaReceiverForm', () => { expect(integrationPayload.secureFields).not.toHaveProperty('tlsConfig.clientCert'); expect(integrationPayload.secureFields).not.toHaveProperty('tlsConfig.clientKey'); + // Verify that OAuth2 TLS config is not present in the settings + expect(integrationPayload.settings).not.toHaveProperty('http_config.oauth2.tls_config'); + expect(integrationPayload.secureFields).not.toHaveProperty('http_config.oauth2.tls_config.caCertificate'); + expect(integrationPayload.secureFields).not.toHaveProperty('http_config.oauth2.tls_config.clientCert'); + expect(integrationPayload.secureFields).not.toHaveProperty('http_config.oauth2.tls_config.clientKey'); + expect(postRequestBody).toMatchSnapshot(); }); }); diff --git a/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap b/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap index dd2fba64876..84de2030021 100644 --- a/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap +++ b/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap @@ -41,8 +41,32 @@ exports[`GrafanaReceiverForm Webhook contact point should properly remove TLS co { "disableResolveMessage": false, "name": "webhook-test", - "secureFields": {}, + "secureFields": { + "http_config.oauth2.client_secret": true, + }, "settings": { + "http_config": { + "oauth2": { + "client_id": "client-id", + "endpoint_params": { + "param1": "value1", + "param2": "value2", + }, + "proxy_config": { + "no_proxy": "example.com", + "proxy_connect_header": { + "X-Custom-Header": "custom-value", + }, + "proxy_from_environment": true, + "proxy_url": "http://example.com/proxy", + }, + "scopes": [ + "scope1", + "scope2", + ], + "token_url": "http://example.com/oauth2/token", + }, + }, "url": "http://example.com", }, "type": "webhook", diff --git a/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx b/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx index 15a3c62f46e..62db9294dc3 100644 --- a/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/fields/OptionField.tsx @@ -26,21 +26,18 @@ interface Props { defaultValue: any; option: NotificationChannelOption; getOptionMeta?: (option: NotificationChannelOption) => OptionMeta; - // this is defined if the option is rendered inside a subform - parentOption?: NotificationChannelOption; invalid?: boolean; pathPrefix: string; error?: FieldError | DeepMap; readOnly?: boolean; customValidator?: (value: string) => boolean | string | Promise; onResetSecureField?: (propertyName: string) => void; - onDeleteSubform?: (propertyName: string) => void; + onDeleteSubform?: (settingsPath: string, option: NotificationChannelOption) => void; secureFields: NotificationChannelSecureFields; } export const OptionField: FC = ({ option, - parentOption, invalid, pathPrefix, error, @@ -94,7 +91,6 @@ export const OptionField: FC = ({ invalid={invalid} pathPrefix={pathPrefix} readOnly={readOnly} - parentOption={parentOption} customValidator={customValidator} onResetSecureField={onResetSecureField} secureFields={secureFields} @@ -113,7 +109,6 @@ const OptionInput: FC = ({ customValidator, onResetSecureField, secureFields = {}, - parentOption, getOptionMeta, }) => { const styles = useStyles2(getStyles); @@ -122,9 +117,9 @@ const OptionInput: FC = ({ const optionMeta = getOptionMeta?.(option); const name = `${pathPrefix}${option.propertyName}`; - const nestedKey = parentOption ? `${parentOption.propertyName}.${option.propertyName}` : option.propertyName; - const isEncryptedInput = secureFields?.[nestedKey]; + const secureFieldKey = option.secure && option.secureFieldKey ? option.secureFieldKey : ''; + const isEncryptedInput = secureFieldKey && secureFields?.[secureFieldKey]; // workaround for https://github.com/react-hook-form/react-hook-form/issues/4993#issuecomment-829012506 useEffect( @@ -162,7 +157,7 @@ const OptionInput: FC = ({ onSelectTemplate={onSelectTemplate} > {isEncryptedInput ? ( - onResetSecureField?.(nestedKey)} isConfigured /> + onResetSecureField?.(secureFieldKey)} isConfigured /> ) : ( = ({ onSelectTemplate={onSelectTemplate} > {isEncryptedInput ? ( - onResetSecureField?.(nestedKey)} isConfigured /> + onResetSecureField?.(secureFieldKey)} isConfigured /> ) : (