Compare commits

...

3 Commits

Author SHA1 Message Date
Nikhil Soni
c40ebb027b Revert "fix(querier): use collector-stamped insert time for last_observed stats" (#12560)
Some checks are pending
build-staging / prepare (push) Waiting to run
build-staging / js-build (push) Blocked by required conditions
build-staging / go-build (push) Blocked by required conditions
build-staging / staging (push) Blocked by required conditions
cacheci / tests (push) Waiting to run
Release Drafter / update_release_draft (push) Waiting to run
Reverts SigNoz/signoz#12455 since the new column the PR was querying on
is not indexed and leading to latency across deployments.

Part of https://github.com/SigNoz/engineering-pod/issues/5916
2026-08-14 17:36:08 +00:00
Nikhil Soni
789a4626fc fix(saved-views): recover legacy-shaped selectedFields entries (#12549)
Some checks failed
build-staging / staging (push) Has been cancelled
build-staging / prepare (push) Has been cancelled
build-staging / js-build (push) Has been cancelled
build-staging / go-build (push) Has been cancelled
cacheci / tests (push) Has been cancelled
Release Drafter / update_release_draft (push) Has been cancelled
### Description

Old saved views still store `selectedFields` as `key`/`dataType`/`type`.
That shape unmarshals cleanly into a zero-valued `TelemetryFieldKey`, so
migration 111 saw no error and skipped those rows — they now read back
with an empty `name`, which breaks the explorer UI.

- Migration 113 remaps `key` → `name`, `type` → `fieldContext`,
`dataType` → `fieldDataType`, including the old spellings with no
current alias (`spanSearchScope`, `array(string)` and friends). Entries
with neither `name` nor `key` are dropped; valid entries are left
untouched.
- `SavedViewSpec.Validate` now requires `selectedFields[].name`, so this
can't be written again.

Closes https://github.com/SigNoz/engineering-pod/issues/5909
2026-08-14 14:24:40 +00:00
Nikhil Mantri
2dcd4d9a66 feat(alert-channel-integrations): improve the existing go tests (#12526)
#### Description

- Splits testify usage in the existing alert channel tests (email,
slack, pagerduty, opsgenie, msteamsv2, webhook) per the convention
established in #12314: `require` for error checks and guards before
indexing/dereferencing, `assert` for the independent value checks so one
failure doesn't mask the rest.
- Fixes illegal `require`/`t.Fatal` calls inside `httptest` handlers
(pagerduty, slack), which run on the server's goroutine where `FailNow`
must not be called; these now use `assert`.
- Adds missing guards before unchecked indexing and pointer dereferences
(opsgenie request slice, msteamsv2 blocks, slack field pointer, email
HTML/Text pointers).
- Normalizes leftover raw `t.Fatal`/`t.Errorf` and redundant `if err !=
nil { require.NoError }` patterns to plain testify calls.

#### Issues closed by this PR

Closes SigNoz/pulse-pod#164
2026-08-14 10:55:02 +00:00
11 changed files with 419 additions and 221 deletions

View File

@@ -223,9 +223,7 @@ func TestEmailNotifyWithErrors(t *testing.T) {
}
c, err := loadEmailTestConfiguration(cfgFile)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
for _, tc := range []struct {
title string
@@ -288,11 +286,6 @@ func TestEmailNotifyWithErrors(t *testing.T) {
},
} {
t.Run(tc.title, func(t *testing.T) {
if len(tc.errMsg) == 0 {
t.Fatal("please define the expected error message")
return
}
emailCfg := &config.EmailConfig{
Smarthost: c.Smarthost,
To: emailTo,
@@ -309,15 +302,15 @@ func TestEmailNotifyWithErrors(t *testing.T) {
_, retry, err := notifyEmail(t, emailCfg, c.Server)
require.Error(t, err)
require.Contains(t, err.Error(), tc.errMsg)
require.False(t, retry)
assert.Contains(t, err.Error(), tc.errMsg)
assert.False(t, retry)
e, err := c.Server.getLastEmail(t)
require.NoError(t, err)
if tc.hasEmail {
require.NotNil(t, e)
assert.NotNil(t, e)
} else {
require.Nil(t, e)
assert.Nil(t, e)
}
})
}
@@ -331,9 +324,7 @@ func TestEmailNotifyWithDoneContext(t *testing.T) {
}
c, err := loadEmailTestConfiguration(cfgFile)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
ctx, cancel := context.WithCancel(context.Background())
cancel()
@@ -350,7 +341,7 @@ func TestEmailNotifyWithDoneContext(t *testing.T) {
c.Server,
)
require.Error(t, err)
require.Contains(t, err.Error(), "establish connection to server")
assert.Contains(t, err.Error(), "establish connection to server")
}
// TestEmailNotifyWithoutAuthentication sends an email to an instance of
@@ -363,9 +354,7 @@ func TestEmailNotifyWithoutAuthentication(t *testing.T) {
}
c, err := loadEmailTestConfiguration(cfgFile)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
mail, _, err := notifyEmail(
t,
@@ -390,7 +379,7 @@ func TestEmailNotifyWithoutAuthentication(t *testing.T) {
}
headers = append(headers, k)
}
require.True(t, foundMsgID, "Couldn't find 'message-id' in %v", headers)
assert.True(t, foundMsgID, "Couldn't find 'message-id' in %v", headers)
}
// TestEmailNotifyWithSTARTTLS connects to the server, upgrades the connection
@@ -406,9 +395,7 @@ func TestEmailNotifyWithSTARTTLS(t *testing.T) {
}
c, err := loadEmailTestConfiguration(cfgFile)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
trueVar := true
_, _, err = notifyEmail(
@@ -437,9 +424,7 @@ func TestEmailNotifyWithAuthentication(t *testing.T) {
}
c, err := loadEmailTestConfiguration(cfgFile)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
td := t.TempDir()
fileWithCorrectPassword, err := os.CreateTemp(td, "smtp-password-correct")
@@ -583,13 +568,13 @@ func TestEmailNotifyWithAuthentication(t *testing.T) {
e, retry, err := notifyEmail(t, emailCfg, c.Server)
if len(tc.errMsg) > 0 {
require.Error(t, err)
require.Contains(t, err.Error(), tc.errMsg)
require.Equal(t, tc.retry, retry)
assert.Contains(t, err.Error(), tc.errMsg)
assert.Equal(t, tc.retry, retry)
return
}
require.NoError(t, err)
require.Equal(t, "1 firing alert(s)", e.Subject)
assert.Equal(t, "1 firing alert(s)", e.Subject)
getAddresses := func(addresses []map[string]string) []string {
res := make([]string, 0, len(addresses))
@@ -600,19 +585,21 @@ func TestEmailNotifyWithAuthentication(t *testing.T) {
}
to := getAddresses(e.To)
from := getAddresses(e.From)
require.Equal(t, strings.Split(emailCfg.To, ","), to)
require.Equal(t, strings.Split(emailCfg.From, ","), from)
assert.Equal(t, strings.Split(emailCfg.To, ","), to)
assert.Equal(t, strings.Split(emailCfg.From, ","), from)
if len(emailCfg.HTML) > 0 {
require.Equal(t, emailCfg.HTML, *e.HTML)
require.NotNil(t, e.HTML)
assert.Equal(t, emailCfg.HTML, *e.HTML)
} else {
require.Nil(t, e.HTML)
assert.Nil(t, e.HTML)
}
if len(emailCfg.Text) > 0 {
require.Equal(t, emailCfg.Text, *e.Text)
require.NotNil(t, e.Text)
assert.Equal(t, emailCfg.Text, *e.Text)
} else {
require.Nil(t, e.Text)
assert.Nil(t, e.Text)
}
})
}
@@ -624,7 +611,7 @@ func TestEmailConfigNoAuthMechs(t *testing.T) {
}
_, err := email.auth("")
require.Error(t, err)
require.Equal(t, "unknown auth mechanism: ", err.Error())
assert.Equal(t, "unknown auth mechanism: ", err.Error())
}
func TestEmailConfigMissingAuthParam(t *testing.T) {
@@ -634,19 +621,19 @@ func TestEmailConfigMissingAuthParam(t *testing.T) {
}
_, err := email.auth("CRAM-MD5")
require.Error(t, err)
require.Equal(t, "missing secret for CRAM-MD5 auth mechanism", err.Error())
assert.Equal(t, "missing secret for CRAM-MD5 auth mechanism", err.Error())
_, err = email.auth("PLAIN")
require.Error(t, err)
require.Equal(t, "missing password for PLAIN auth mechanism", err.Error())
assert.Equal(t, "missing password for PLAIN auth mechanism", err.Error())
_, err = email.auth("LOGIN")
require.Error(t, err)
require.Equal(t, "missing password for LOGIN auth mechanism", err.Error())
assert.Equal(t, "missing password for LOGIN auth mechanism", err.Error())
_, err = email.auth("PLAIN LOGIN")
require.Error(t, err)
require.Equal(t, "missing password for PLAIN auth mechanism\nmissing password for LOGIN auth mechanism", err.Error())
assert.Equal(t, "missing password for PLAIN auth mechanism\nmissing password for LOGIN auth mechanism", err.Error())
}
func TestEmailNoUsernameCustomError(t *testing.T) {
@@ -655,7 +642,7 @@ func TestEmailNoUsernameCustomError(t *testing.T) {
}
a, err := email.auth("CRAM-MD5")
require.ErrorIs(t, err, errNoAuthUsernameConfigured)
require.Nil(t, a)
assert.Nil(t, a)
}
// TestEmailRejected simulates the failure of an otherwise valid message submission which fails at a later point than
@@ -720,7 +707,7 @@ func TestEmailRejected(t *testing.T) {
// Send the alert to mock SMTP server.
retry, err := e.Notify(context.Background(), firingAlert)
require.ErrorContains(t, err, "501 5.5.4 Rejected!")
require.True(t, retry)
assert.True(t, retry)
require.NoError(t, srv.Shutdown(ctx))
require.Eventuallyf(t, func() bool {
@@ -789,9 +776,7 @@ func TestEmailNotifyWithThreading(t *testing.T) {
}
c, err := loadEmailTestConfiguration(cfgFile)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
for _, tc := range []struct {
name string
@@ -836,22 +821,22 @@ func TestEmailNotifyWithThreading(t *testing.T) {
referencesValue := mail.Headers["references"]
inReplyToValue := mail.Headers["in-reply-to"]
require.NotEmpty(t, referencesValue, "References header not found in %v", mail.Headers)
require.NotEmpty(t, inReplyToValue, "In-Reply-To header not found in %v", mail.Headers)
assert.NotEmpty(t, referencesValue, "References header not found in %v", mail.Headers)
assert.NotEmpty(t, inReplyToValue, "In-Reply-To header not found in %v", mail.Headers)
require.Equal(t, referencesValue, inReplyToValue, "References and In-Reply-To should match")
assert.Equal(t, referencesValue, inReplyToValue, "References and In-Reply-To should match")
// Verify the format: <alert-HASH-DATE@alertmanager>
require.Contains(t, referencesValue, "<alert-")
require.Contains(t, referencesValue, "@alertmanager>")
assert.Contains(t, referencesValue, "<alert-")
assert.Contains(t, referencesValue, "@alertmanager>")
if tc.wantDatePart {
today := time.Now().Format("2006-01-02")
require.Contains(t, referencesValue, today, "threading header should contain today's date")
assert.Contains(t, referencesValue, today, "threading header should contain today's date")
} else {
// With thread_by_date: none, there should be no date
// (empty string between hash and @).
require.Contains(t, referencesValue, "-@alertmanager>", "threading header should have empty date part")
assert.Contains(t, referencesValue, "-@alertmanager>", "threading header should have empty date part")
}
})
}
@@ -904,14 +889,14 @@ func TestEmailGetPassword(t *testing.T) {
require.Error(t, err)
if errors.Asc(err, errors.CodeInternal) {
_, _, errMsg, _, _, _ := errors.Unwrapb(err)
require.Contains(t, errMsg, tc.errMsg)
assert.Contains(t, errMsg, tc.errMsg)
} else {
require.Contains(t, err.Error(), tc.errMsg)
assert.Contains(t, err.Error(), tc.errMsg)
}
require.Empty(t, password)
assert.Empty(t, password)
} else {
require.NoError(t, err)
require.Equal(t, "secret", password)
assert.Equal(t, "secret", password)
}
})
}
@@ -962,11 +947,11 @@ func TestEmailGetSecret(t *testing.T) {
secret, err := email.getAuthSecret()
if len(tc.errMsg) > 0 {
require.Error(t, err)
require.Contains(t, err.Error(), tc.errMsg)
require.Empty(t, secret)
assert.Contains(t, err.Error(), tc.errMsg)
assert.Empty(t, secret)
} else {
require.NoError(t, err)
require.Equal(t, "secret", secret)
assert.Equal(t, "secret", secret)
}
})
}
@@ -1032,7 +1017,7 @@ func TestEmailImplicitTLS(t *testing.T) {
useImplicitTLS = cfg.Smarthost.Port == "465"
}
require.Equal(t, tt.expectImplicit, useImplicitTLS,
assert.Equal(t, tt.expectImplicit, useImplicitTLS,
"Expected useImplicitTLS=%v for port=%s with forceImplicitTLS=%v",
tt.expectImplicit, tt.port, tt.forceImplicitTLS)
})
@@ -1074,8 +1059,8 @@ func TestPrepareContent(t *testing.T) {
ctx := context.Background()
subject, htmlBody, err := n.prepareContent(ctx, alerts)
require.NoError(t, err)
require.Equal(t, "subj", subject)
require.Equal(t, "<div><p>line one</p>\n</div><div><p>line two</p>\n</div>", htmlBody)
assert.Equal(t, "subj", subject)
assert.Equal(t, "<div><p>line one</p>\n</div><div><p>line two</p>\n</div>", htmlBody)
})
t.Run("custom title template; default body HTML template", func(t *testing.T) {
@@ -1103,8 +1088,8 @@ func TestPrepareContent(t *testing.T) {
ctx := context.Background()
subject, htmlBody, err := n.prepareContent(ctx, alerts)
require.NoError(t, err)
require.Equal(t, "Status: firing", htmlBody)
require.Equal(t, "fixed from firing", subject)
assert.Equal(t, "Status: firing", htmlBody)
assert.Equal(t, "fixed from firing", subject)
})
t.Run("default template without HTML", func(t *testing.T) {
@@ -1125,8 +1110,8 @@ func TestPrepareContent(t *testing.T) {
ctx := context.Background()
subject, htmlBody, err := n.prepareContent(ctx, alerts)
require.NoError(t, err)
require.Equal(t, "", htmlBody)
require.Equal(t, "the email subject", subject)
assert.Equal(t, "", htmlBody)
assert.Equal(t, "the email subject", subject)
})
t.Run("custom title template; custom body template", func(t *testing.T) {
@@ -1160,11 +1145,11 @@ func TestPrepareContent(t *testing.T) {
ctx := context.Background()
subject, htmlBody, err := n.prepareContent(ctx, alerts)
require.NoError(t, err)
require.Contains(t, htmlBody, "<!DOCTYPE html>")
require.Contains(t, htmlBody, "<p>line two</p>")
require.NotContains(t, htmlBody, "Well, what are you?")
require.Equal(t, subject, "fixed from firing")
require.NotContains(t, subject, "subject")
assert.Contains(t, htmlBody, "<!DOCTYPE html>")
assert.Contains(t, htmlBody, "<p>line two</p>")
assert.NotContains(t, htmlBody, "Well, what are you?")
assert.Equal(t, "fixed from firing", subject)
assert.NotContains(t, subject, "subject")
})
}

View File

@@ -22,6 +22,7 @@ import (
commoncfg "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
test "github.com/SigNoz/signoz/pkg/alertmanager/alertmanagernotify/alertmanagernotifytest"
@@ -54,7 +55,7 @@ func TestMSTeamsV2Retry(t *testing.T) {
for statusCode, expected := range test.RetryTests(test.DefaultRetryCodes()) {
actual, _ := notifier.retrier.Check(statusCode, nil)
require.Equal(t, expected, actual, "retry - error on status %d", statusCode)
assert.Equal(t, expected, actual, "retry - error on status %d", statusCode)
}
}
@@ -110,7 +111,7 @@ func TestNotifier_Notify_WithReason(t *testing.T) {
} else {
var reasonError *notify.ErrorWithReason
require.ErrorAs(t, err, &reasonError)
require.Equal(t, tt.expectedReason, reasonError.Reason)
assert.Equal(t, tt.expectedReason, reasonError.Reason)
}
})
}
@@ -133,7 +134,6 @@ func TestMSTeamsV2Templating(t *testing.T) {
cfg *config.MSTeamsV2Config
titleLink string
retry bool
errMsg string
}{
{
@@ -143,7 +143,6 @@ func TestMSTeamsV2Templating(t *testing.T) {
Text: `{{ template "msteams.default.text" . }}`,
},
titleLink: `{{ template "msteamsv2.default.titleLink" . }}`,
retry: false,
},
{
title: "title with templating errors",
@@ -185,12 +184,12 @@ func TestMSTeamsV2Templating(t *testing.T) {
},
}...)
if tc.errMsg == "" {
require.NoError(t, err)
assert.NoError(t, err)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), tc.errMsg)
assert.Contains(t, err.Error(), tc.errMsg)
}
require.Equal(t, tc.retry, ok)
assert.False(t, ok)
})
}
}
@@ -250,14 +249,14 @@ func TestPrepareContent(t *testing.T) {
}
blocks, err := notifier.prepareContent(ctx, alerts)
require.NoError(t, err)
require.NotEmpty(t, blocks)
require.Len(t, blocks, 2)
// First block should be the title with color (firing = red)
require.Equal(t, "Bolder", blocks[0].Weight)
require.Equal(t, colorRed, blocks[0].Color)
assert.Equal(t, "Bolder", blocks[0].Weight)
assert.Equal(t, colorRed, blocks[0].Color)
// verify title text
require.Equal(t, "Alertname: test", blocks[0].Text)
assert.Equal(t, "Alertname: test", blocks[0].Text)
// verify body text
require.Equal(t, "Firing alert: test", blocks[1].Text)
assert.Equal(t, "Firing alert: test", blocks[1].Text)
})
t.Run("custom template - per-alert color", func(t *testing.T) {
@@ -305,16 +304,15 @@ func TestPrepareContent(t *testing.T) {
}
blocks, err := notifier.prepareContent(ctx, alerts)
require.NoError(t, err)
require.NotEmpty(t, blocks)
// total 3 blocks: title and 2 body blocks
require.True(t, len(blocks) == 3)
require.Len(t, blocks, 3)
// First block: title color is overall color of the alerts
require.Equal(t, colorRed, blocks[0].Color)
assert.Equal(t, colorRed, blocks[0].Color)
// verify title text
require.Equal(t, "Custom Title", blocks[0].Text)
assert.Equal(t, "Custom Title", blocks[0].Text)
// Body blocks should have per-alert color
require.Equal(t, colorRed, blocks[1].Color) // firing
require.Equal(t, colorGreen, blocks[2].Color) // resolved
assert.Equal(t, colorRed, blocks[1].Color) // firing
assert.Equal(t, colorGreen, blocks[2].Color) // resolved
})
}

View File

@@ -21,6 +21,7 @@ import (
commoncfg "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/prometheus/alertmanager/config"
@@ -49,7 +50,7 @@ func TestOpsGenieRetry(t *testing.T) {
retryCodes := append(test.DefaultRetryCodes(), http.StatusTooManyRequests)
for statusCode, expected := range test.RetryTests(retryCodes) {
actual, _ := notifier.retrier.Check(statusCode, nil)
require.Equal(t, expected, actual, "error on status %d", statusCode)
assert.Equal(t, expected, actual, "error on status %d", statusCode)
}
}
@@ -103,9 +104,7 @@ func TestGettingOpsGegineApikeyFromFile(t *testing.T) {
func TestOpsGenie(t *testing.T) {
u, err := url.Parse("https://opsgenie/api")
if err != nil {
t.Fatalf("failed to parse URL: %v", err)
}
require.NoError(t, err)
logger := promslog.NewNopLogger()
tmpl := test.CreateTmpl(t)
@@ -236,10 +235,10 @@ func TestOpsGenie(t *testing.T) {
req, retry, err := notifier.createRequests(ctx, alert1)
require.NoError(t, err)
require.Len(t, req, 1)
require.True(t, retry)
require.Equal(t, expectedURL, req[0].URL)
require.Equal(t, "GenieKey http://am", req[0].Header.Get("Authorization"))
require.Equal(t, tc.expectedEmptyAlertBody, readBody(t, req[0]))
assert.True(t, retry)
assert.Equal(t, expectedURL, req[0].URL)
assert.Equal(t, "GenieKey http://am", req[0].Header.Get("Authorization"))
assert.Equal(t, tc.expectedEmptyAlertBody, readBody(t, req[0]))
// Fully defined alert.
alert2 := &types.Alert{
@@ -266,15 +265,15 @@ func TestOpsGenie(t *testing.T) {
}
req, retry, err = notifier.createRequests(ctx, alert2)
require.NoError(t, err)
require.True(t, retry)
assert.True(t, retry)
require.Len(t, req, 1)
require.Equal(t, tc.expectedBody, readBody(t, req[0]))
assert.Equal(t, tc.expectedBody, readBody(t, req[0]))
// Broken API Key Template.
tc.cfg.APIKey = "{{ kaput "
_, _, err = notifier.createRequests(ctx, alert2)
require.Error(t, err)
require.Equal(t, "template: :1: function \"kaput\" not defined", err.Error())
assert.Equal(t, "template: :1: function \"kaput\" not defined", err.Error())
})
}
}
@@ -307,7 +306,7 @@ func TestOpsGenieWithUpdate(t *testing.T) {
require.NoError(t, err)
requests, retry, err := notifierWithUpdate.createRequests(ctx, alert)
require.NoError(t, err)
require.True(t, retry)
assert.True(t, retry)
require.Len(t, requests, 3)
body0 := readBody(t, requests[0])
@@ -316,13 +315,13 @@ func TestOpsGenieWithUpdate(t *testing.T) {
key, _ := notify.ExtractGroupKey(ctx)
alias := key.Hash()
require.Equal(t, "https://test-opsgenie-url/v2/alerts", requests[0].URL.String())
require.NotEmpty(t, body0)
assert.Equal(t, "https://test-opsgenie-url/v2/alerts", requests[0].URL.String())
assert.NotEmpty(t, body0)
require.Equal(t, requests[1].URL.String(), fmt.Sprintf("https://test-opsgenie-url/v2/alerts/%s/message?identifierType=alias", alias))
require.JSONEq(t, `{"message":"new message"}`, body1)
require.Equal(t, requests[2].URL.String(), fmt.Sprintf("https://test-opsgenie-url/v2/alerts/%s/description?identifierType=alias", alias))
require.JSONEq(t, `{"description":"new description"}`, body2)
assert.Equal(t, requests[1].URL.String(), fmt.Sprintf("https://test-opsgenie-url/v2/alerts/%s/message?identifierType=alias", alias))
assert.JSONEq(t, `{"message":"new message"}`, body1)
assert.Equal(t, requests[2].URL.String(), fmt.Sprintf("https://test-opsgenie-url/v2/alerts/%s/description?identifierType=alias", alias))
assert.JSONEq(t, `{"description":"new description"}`, body2)
}
func TestOpsGenieApiKeyFile(t *testing.T) {
@@ -341,7 +340,8 @@ func TestOpsGenieApiKeyFile(t *testing.T) {
require.NoError(t, err)
requests, _, err := notifierWithUpdate.createRequests(ctx)
require.NoError(t, err)
require.Equal(t, "GenieKey my_secret_api_key", requests[0].Header.Get("Authorization"))
require.Len(t, requests, 1)
assert.Equal(t, "GenieKey my_secret_api_key", requests[0].Header.Get("Authorization"))
}
func TestPrepareContent(t *testing.T) {
@@ -377,8 +377,8 @@ func TestPrepareContent(t *testing.T) {
title, desc, prepErr := notifier.prepareContent(ctx, alerts)
require.NoError(t, prepErr)
require.Equal(t, "Firing alert: test", title)
require.Equal(t, "Check runbook for more details", desc)
assert.Equal(t, "Firing alert: test", title)
assert.Equal(t, "Check runbook for more details", desc)
})
t.Run("custom template", func(t *testing.T) {
@@ -431,9 +431,9 @@ func TestPrepareContent(t *testing.T) {
title, desc, err := notifier.prepareContent(ctx, alerts)
require.NoError(t, err)
require.Equal(t, "High request throughput for payment", title)
assert.Equal(t, "High request throughput for payment", title)
// Each alert body wrapped in <div>, separated by <hr>
require.Equal(t, "<div><p>Alert firing in NS: potter-the-harry</p>\n</div><hr><div><p>Alert firing in NS: smart-the-rat</p>\n</div>", desc)
assert.Equal(t, "<div><p>Alert firing in NS: potter-the-harry</p>\n</div><hr><div><p>Alert firing in NS: smart-the-rat</p>\n</div>", desc)
})
}

View File

@@ -25,6 +25,7 @@ import (
commoncfg "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/prometheus/alertmanager/config"
@@ -54,7 +55,7 @@ func TestPagerDutyRetryV1(t *testing.T) {
retryCodes := append(test.DefaultRetryCodes(), http.StatusForbidden)
for statusCode, expected := range test.RetryTests(retryCodes) {
actual, _ := notifier.retrier.Check(statusCode, nil)
require.Equal(t, expected, actual, "retryv1 - error on status %d", statusCode)
assert.Equal(t, expected, actual, "retryv1 - error on status %d", statusCode)
}
}
@@ -74,7 +75,7 @@ func TestPagerDutyRetryV2(t *testing.T) {
retryCodes := append(test.DefaultRetryCodes(), http.StatusTooManyRequests)
for statusCode, expected := range test.RetryTests(retryCodes) {
actual, _ := notifier.retrier.Check(statusCode, nil)
require.Equal(t, expected, actual, "retryv2 - error on status %d", statusCode)
assert.Equal(t, expected, actual, "retryv2 - error on status %d", statusCode)
}
}
@@ -349,12 +350,12 @@ func TestPagerDutyTemplating(t *testing.T) {
require.Error(t, err)
if errors.Asc(err, errors.CodeInternal) {
_, _, errMsg, _, _, _ := errors.Unwrapb(err)
require.Contains(t, errMsg, tc.errMsg)
assert.Contains(t, errMsg, tc.errMsg)
} else {
require.Contains(t, err.Error(), tc.errMsg)
assert.Contains(t, err.Error(), tc.errMsg)
}
}
require.Equal(t, tc.retry, ok)
assert.Equal(t, tc.retry, ok)
})
}
}
@@ -393,7 +394,7 @@ func TestErrDetails(t *testing.T) {
} {
t.Run("", func(t *testing.T) {
err := errDetails(tc.status, tc.body)
require.Contains(t, err, tc.exp)
assert.Contains(t, err, tc.exp)
})
}
}
@@ -427,7 +428,7 @@ func TestEventSizeEnforcement(t *testing.T) {
encodedV1, err := notifierV1.encodeMessage(context.Background(), msgV1)
require.NoError(t, err)
require.Contains(t, encodedV1.String(), `"details":{"error":"Custom details have been removed because the original event exceeds the maximum size of 512KB"}`)
assert.Contains(t, encodedV1.String(), `"details":{"error":"Custom details have been removed because the original event exceeds the maximum size of 512KB"}`)
// V2 Messages
msgV2 := &pagerDutyMessage{
@@ -451,7 +452,7 @@ func TestEventSizeEnforcement(t *testing.T) {
encodedV2, err := notifierV2.encodeMessage(context.Background(), msgV2)
require.NoError(t, err)
require.Contains(t, encodedV2.String(), `"custom_details":{"error":"Custom details have been removed because the original event exceeds the maximum size of 512KB"}`)
assert.Contains(t, encodedV2.String(), `"custom_details":{"error":"Custom details have been removed because the original event exceeds the maximum size of 512KB"}`)
}
func TestPagerDutyEmptySrcHref(t *testing.T) {
@@ -543,8 +544,9 @@ func TestPagerDutyEmptySrcHref(t *testing.T) {
}
}
require.Equal(t, expectedImages, event.Images)
require.Equal(t, expectedLinks, event.Links)
// Handler runs on the server's goroutine — require is illegal here.
assert.Equal(t, expectedImages, event.Images)
assert.Equal(t, expectedLinks, event.Links)
},
))
defer server.Close()
@@ -644,7 +646,7 @@ func TestPagerDutyTimeout(t *testing.T) {
},
}
_, err = pd.Notify(ctx, alert)
require.Equal(t, tt.wantErr, err != nil)
assert.Equal(t, tt.wantErr, err != nil)
})
}
}
@@ -899,11 +901,12 @@ func TestRenderDetails(t *testing.T) {
tmpl: test.CreateTmpl(t),
}
got, err := n.renderDetails(tt.args.data)
if (err != nil) != tt.wantErr {
t.Errorf("renderDetails() error = %v, wantErr %v", err, tt.wantErr)
return
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
require.Equal(t, tt.want, got)
assert.Equal(t, tt.want, got)
})
}
}
@@ -944,7 +947,7 @@ func TestPrepareContent(t *testing.T) {
title, err := notifier.prepareTitle(ctx, alerts)
require.NoError(t, err)
require.Equal(t, "HighCPU for Payment service (FIRING)", title)
assert.Equal(t, "HighCPU for Payment service (FIRING)", title)
})
t.Run("custom template uses $variable annotation for title", func(t *testing.T) {
@@ -980,6 +983,6 @@ func TestPrepareContent(t *testing.T) {
title, err := notifier.prepareTitle(ctx, alerts)
require.NoError(t, err)
require.Equal(t, "HighCPU on api-server is in resolved state", title)
assert.Equal(t, "HighCPU on api-server is in resolved state", title)
})
}

View File

@@ -23,6 +23,7 @@ import (
commoncfg "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/prometheus/alertmanager/config"
@@ -50,7 +51,7 @@ func TestSlackRetry(t *testing.T) {
for statusCode, expected := range test.RetryTests(test.DefaultRetryCodes()) {
actual, _ := notifier.retrier.Check(statusCode, nil)
require.Equal(t, expected, actual, "error on status %d", statusCode)
assert.Equal(t, expected, actual, "error on status %d", statusCode)
}
}
@@ -232,15 +233,15 @@ func TestNotifier_Notify_WithReason(t *testing.T) {
},
}
retry, err := notifier.Notify(ctx, alert1)
require.Equal(t, tt.expectedRetry, retry)
assert.Equal(t, tt.expectedRetry, retry)
if tt.noError {
require.NoError(t, err)
} else {
var reasonError *notify.ErrorWithReason
require.ErrorAs(t, err, &reasonError)
require.Equal(t, tt.expectedReason, reasonError.Reason)
require.Contains(t, err.Error(), tt.expectedErr)
require.Contains(t, err.Error(), "channelname")
assert.Equal(t, tt.expectedReason, reasonError.Reason)
assert.Contains(t, err.Error(), tt.expectedErr)
assert.Contains(t, err.Error(), "channelname")
}
})
}
@@ -296,7 +297,7 @@ func TestSlackTimeout(t *testing.T) {
},
}
_, err = notifier.Notify(ctx, alert)
require.Equal(t, tt.wantErr, err != nil)
assert.Equal(t, tt.wantErr, err != nil)
})
}
}
@@ -350,14 +351,14 @@ func TestPrepareContent(t *testing.T) {
require.NoError(t, err)
require.Len(t, atts, 1)
require.Equal(t, "HighCPU (FIRING)", atts[0].Title)
require.Equal(t, "Alert: HighCPU - severity critical", atts[0].Text)
assert.Equal(t, "HighCPU (FIRING)", atts[0].Title)
assert.Equal(t, "Alert: HighCPU - severity critical", atts[0].Text)
// Color is templated — firing alert should be "danger"
require.Equal(t, "danger", atts[0].Color)
assert.Equal(t, "danger", atts[0].Color)
// No BlockKit blocks for default template
require.Nil(t, atts[0].Blocks)
assert.Nil(t, atts[0].Blocks)
// Default markdownIn when config has none
require.Equal(t, []string{"fallback", "pretext", "text"}, atts[0].MrkdwnIn)
assert.Equal(t, []string{"fallback", "pretext", "text"}, atts[0].MrkdwnIn)
})
t.Run("custom template produces 1+N attachments with per-alert color", func(t *testing.T) {
@@ -428,10 +429,10 @@ func TestPrepareContent(t *testing.T) {
require.Len(t, atts, 3)
// First attachment: title-only, no color, no blocks
require.Equal(t, "[firing] HighCPU — api-server", atts[0].Title)
require.Empty(t, atts[0].Color)
require.Nil(t, atts[0].Blocks)
require.Equal(t, "https://alertmanager.signoz.com", atts[0].TitleLink)
assert.Equal(t, "[firing] HighCPU — api-server", atts[0].Title)
assert.Empty(t, atts[0].Color)
assert.Nil(t, atts[0].Blocks)
assert.Equal(t, "https://alertmanager.signoz.com", atts[0].TitleLink)
expectedFiringBody := "*HighCPU*\n\n" +
"*Service:* _api-server_\n*Instance:* _i-0abc123_\n*Region:* _us-east-1_\n*Method:* _GET_\n\n" +
@@ -446,16 +447,16 @@ func TestPrepareContent(t *testing.T) {
"*Status:* resolved | *Severity:* critical\n\n"
// Second attachment: firing alert body rendered as slack mrkdwn text, red color
require.Nil(t, atts[1].Blocks)
require.Equal(t, "#FF0000", atts[1].Color)
require.Equal(t, []string{"text"}, atts[1].MrkdwnIn)
require.Equal(t, expectedFiringBody, atts[1].Text)
assert.Nil(t, atts[1].Blocks)
assert.Equal(t, "#FF0000", atts[1].Color)
assert.Equal(t, []string{"text"}, atts[1].MrkdwnIn)
assert.Equal(t, expectedFiringBody, atts[1].Text)
// Third attachment: resolved alert body rendered as slack mrkdwn text, green color
require.Nil(t, atts[2].Blocks)
require.Equal(t, "#00FF00", atts[2].Color)
require.Equal(t, []string{"text"}, atts[2].MrkdwnIn)
require.Equal(t, expectedResolvedBody, atts[2].Text)
assert.Nil(t, atts[2].Blocks)
assert.Equal(t, "#00FF00", atts[2].Color)
assert.Equal(t, []string{"text"}, atts[2].MrkdwnIn)
assert.Equal(t, expectedResolvedBody, atts[2].Text)
})
t.Run("default template with fields and actions", func(t *testing.T) {
@@ -498,49 +499,45 @@ func TestPrepareContent(t *testing.T) {
// prepareContent does not populate fields/actions — that's done by
// addFieldsAndActions which is called from Notify.
require.Nil(t, atts[0].Fields)
require.Nil(t, atts[0].Actions)
assert.Nil(t, atts[0].Fields)
assert.Nil(t, atts[0].Actions)
// Simulate what Notify does after prepareContent
notifier.addFieldsAndActions(&atts[0], tmplText)
// Verify fields
require.Len(t, atts[0].Fields, 2)
require.Equal(t, "Severity", atts[0].Fields[0].Title)
require.Equal(t, "critical", atts[0].Fields[0].Value)
require.True(t, *atts[0].Fields[0].Short)
require.Equal(t, "Service", atts[0].Fields[1].Title)
require.Equal(t, "api-server", atts[0].Fields[1].Value)
assert.Equal(t, "Severity", atts[0].Fields[0].Title)
assert.Equal(t, "critical", atts[0].Fields[0].Value)
require.NotNil(t, atts[0].Fields[0].Short)
assert.True(t, *atts[0].Fields[0].Short)
assert.Equal(t, "Service", atts[0].Fields[1].Title)
assert.Equal(t, "api-server", atts[0].Fields[1].Value)
// Verify actions
require.Len(t, atts[0].Actions, 1)
require.Equal(t, "button", atts[0].Actions[0].Type)
require.Equal(t, "View Alert", atts[0].Actions[0].Text)
require.Equal(t, "https://alertmanager.signoz.com", atts[0].Actions[0].URL)
assert.Equal(t, "button", atts[0].Actions[0].Type)
assert.Equal(t, "View Alert", atts[0].Actions[0].Text)
assert.Equal(t, "https://alertmanager.signoz.com", atts[0].Actions[0].URL)
})
}
func TestSlackMessageField(t *testing.T) {
// 1. Setup a fake Slack server
// 1. Setup a fake Slack server. The handler runs on the server's
// goroutine, so only assert (never require) is safe here.
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var body map[string]any
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Fatal(err)
}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&body))
// 2. VERIFY: Top-level text exists
if body["text"] != "My Top Level Message" {
t.Errorf("Expected top-level 'text' to be 'My Top Level Message', got %v", body["text"])
}
assert.Equal(t, "My Top Level Message", body["text"])
// 3. VERIFY: Old attachments still exist
attachments, ok := body["attachments"].([]any)
if !ok || len(attachments) == 0 {
t.Errorf("Expected attachments to exist")
} else {
first := attachments[0].(map[string]any)
if first["title"] != "Old Attachment Title" {
t.Errorf("Expected attachment title 'Old Attachment Title', got %v", first["title"])
if assert.True(t, ok, "expected attachments to exist") && assert.NotEmpty(t, attachments) {
first, ok := attachments[0].(map[string]any)
if assert.True(t, ok, "expected attachment to be an object") {
assert.Equal(t, "Old Attachment Title", first["title"])
}
}
@@ -561,21 +558,16 @@ func TestSlackMessageField(t *testing.T) {
}
tmpl, err := template.FromGlobs([]string{})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
tmpl.ExternalURL = u
logger := slog.New(slog.DiscardHandler)
notifier, err := New(conf, tmpl, logger, newTestTemplater(tmpl))
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
ctx := context.Background()
ctx = notify.WithGroupKey(ctx, "test-group-key")
if _, err := notifier.Notify(ctx); err != nil {
t.Fatal("Notify failed:", err)
}
_, err = notifier.Notify(ctx)
require.NoError(t, err, "Notify failed")
}

View File

@@ -19,6 +19,7 @@ import (
commoncfg "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/SigNoz/signoz/pkg/alertmanager/alertmanagertemplate"
@@ -39,14 +40,12 @@ func TestWebhookRetry(t *testing.T) {
promslog.NewNopLogger(),
alertmanagertemplate.New(tmpl, slog.Default()),
)
if err != nil {
require.NoError(t, err)
}
require.NoError(t, err)
t.Run("test retry status code", func(t *testing.T) {
for statusCode, expected := range test.RetryTests(test.DefaultRetryCodes()) {
actual, _ := notifier.retrier.Check(statusCode, nil)
require.Equal(t, expected, actual, "error on status %d", statusCode)
assert.Equal(t, expected, actual, "error on status %d", statusCode)
}
})
@@ -73,7 +72,8 @@ func TestWebhookRetry(t *testing.T) {
} {
t.Run("", func(t *testing.T) {
_, err = notifier.retrier.Check(tc.status, tc.body)
require.Equal(t, tc.exp, err.Error())
require.Error(t, err)
assert.Equal(t, tc.exp, err.Error())
})
}
})
@@ -83,16 +83,16 @@ func TestWebhookTruncateAlerts(t *testing.T) {
alerts := make([]*types.Alert, 10)
truncatedAlerts, numTruncated := truncateAlerts(0, alerts)
require.Len(t, truncatedAlerts, 10)
require.EqualValues(t, 0, numTruncated)
assert.Len(t, truncatedAlerts, 10)
assert.EqualValues(t, 0, numTruncated)
truncatedAlerts, numTruncated = truncateAlerts(4, alerts)
require.Len(t, truncatedAlerts, 4)
require.EqualValues(t, 6, numTruncated)
assert.Len(t, truncatedAlerts, 4)
assert.EqualValues(t, 6, numTruncated)
truncatedAlerts, numTruncated = truncateAlerts(100, alerts)
require.Len(t, truncatedAlerts, 10)
require.EqualValues(t, 0, numTruncated)
assert.Len(t, truncatedAlerts, 10)
assert.EqualValues(t, 0, numTruncated)
}
func TestWebhookRedactedURL(t *testing.T) {
@@ -219,10 +219,10 @@ func TestWebhookURLTemplating(t *testing.T) {
if tc.expectError {
require.Error(t, err)
require.Contains(t, err.Error(), tc.expectedErrMsg)
assert.Contains(t, err.Error(), tc.expectedErrMsg)
} else {
require.NoError(t, err)
require.Equal(t, tc.expectedPath, calledURL)
assert.Equal(t, tc.expectedPath, calledURL)
}
})
}

View File

@@ -23,11 +23,7 @@ func (q *querier) Collect(ctx context.Context, _ valuer.UUID) (map[string]any, e
traces uint64
tracesLastSeenAt time.Time
)
tracesLastSeenExpr := "max(timestamp)"
if q.hasColumn(ctx, tracestelemetryschema.DBName, tracestelemetryschema.SpanIndexV3TableName, "inserted_at") {
tracesLastSeenExpr = "max(inserted_at)"
}
if err := q.telemetryStore.ClickhouseDB().QueryRow(ctx, fmt.Sprintf("SELECT COUNT(*), %s FROM %s", tracesLastSeenExpr, tracesTable)).Scan(&traces, &tracesLastSeenAt); err == nil {
if err := q.telemetryStore.ClickhouseDB().QueryRow(ctx, fmt.Sprintf("SELECT COUNT(*), max(timestamp) FROM %s", tracesTable)).Scan(&traces, &tracesLastSeenAt); err == nil {
stats["telemetry.traces.count"] = traces
if tracesLastSeenAt.Unix() != 0 {
stats["telemetry.traces.last_observed.time"] = tracesLastSeenAt.UTC()
@@ -41,11 +37,7 @@ func (q *querier) Collect(ctx context.Context, _ valuer.UUID) (map[string]any, e
logs uint64
logsLastSeenAt time.Time
)
logsLastSeenExpr := "fromUnixTimestamp64Nano(max(timestamp))"
if q.hasColumn(ctx, logstelemetryschema.DBName, logstelemetryschema.LogsV2TableName, "inserted_at") {
logsLastSeenExpr = "max(inserted_at)"
}
if err := q.telemetryStore.ClickhouseDB().QueryRow(ctx, fmt.Sprintf("SELECT COUNT(*), %s FROM %s", logsLastSeenExpr, logsTable)).Scan(&logs, &logsLastSeenAt); err == nil {
if err := q.telemetryStore.ClickhouseDB().QueryRow(ctx, fmt.Sprintf("SELECT COUNT(*), fromUnixTimestamp64Nano(max(timestamp)) FROM %s", logsTable)).Scan(&logs, &logsLastSeenAt); err == nil {
stats["telemetry.logs.count"] = logs
if logsLastSeenAt.Unix() != 0 {
stats["telemetry.logs.last_observed.time"] = logsLastSeenAt.UTC()
@@ -59,11 +51,7 @@ func (q *querier) Collect(ctx context.Context, _ valuer.UUID) (map[string]any, e
metrics uint64
metricsLastSeenAt time.Time
)
metricsLastSeenExpr := "toDateTime(max(unix_milli) / 1000)"
if q.hasColumn(ctx, metricstelemetryschema.DBName, metricstelemetryschema.SamplesV4TableName, "inserted_at_unix_milli") {
metricsLastSeenExpr = "fromUnixTimestamp64Milli(max(inserted_at_unix_milli))"
}
if err := q.telemetryStore.ClickhouseDB().QueryRow(ctx, fmt.Sprintf("SELECT COUNT(*), %s FROM %s", metricsLastSeenExpr, metricsTable)).Scan(&metrics, &metricsLastSeenAt); err == nil {
if err := q.telemetryStore.ClickhouseDB().QueryRow(ctx, fmt.Sprintf("SELECT COUNT(*), toDateTime(max(unix_milli) / 1000) FROM %s", metricsTable)).Scan(&metrics, &metricsLastSeenAt); err == nil {
stats["telemetry.metrics.count"] = metrics
if metricsLastSeenAt.Unix() != 0 {
stats["telemetry.metrics.last_observed.time"] = metricsLastSeenAt.UTC()
@@ -75,12 +63,3 @@ func (q *querier) Collect(ctx context.Context, _ valuer.UUID) (map[string]any, e
return stats, nil
}
func (q *querier) hasColumn(ctx context.Context, database, table, column string) bool {
var exists bool
if err := q.telemetryStore.ClickhouseDB().QueryRow(ctx, "SELECT hasColumnInTable(?, ?, ?)", database, table, column).Scan(&exists); err != nil {
q.logger.DebugContext(ctx, "failed to check column existence", errors.Attr(err))
return false
}
return exists
}

View File

@@ -240,6 +240,7 @@ func NewSQLMigrationProviderFactories(
sqlmigration.NewFixSavedViewSelectedFieldsFactory(sqlstore),
sqlmigration.NewBackfillSavedViewRequestTypeFactory(sqlstore),
sqlmigration.NewRestructureAuthDomainConfigFactory(sqlstore),
sqlmigration.NewFixSavedViewSelectFieldsFactory(sqlstore),
)
}

View File

@@ -0,0 +1,224 @@
package sqlmigration
import (
"bytes"
"context"
"encoding/json"
"log/slog"
"github.com/uptrace/bun"
"github.com/uptrace/bun/migrate"
"github.com/SigNoz/signoz/pkg/factory"
"github.com/SigNoz/signoz/pkg/sqlstore"
)
// storableSavedViewSelectFieldsRow is the shape of the `saved_view` table this migration repairs.
type storableSavedViewSelectFieldsRow struct {
bun.BaseModel `bun:"table:saved_view"`
ID string `bun:"id,pk,type:text"`
Data string `bun:"data,type:text"`
}
// selectedField is a superset of the current shape (name/signal/fieldContext/
// fieldDataType) and the legacy v1 shape (key/dataType/type) it replaced.
type selectedField struct {
Name string `json:"name"`
Signal string `json:"signal"`
FieldContext string `json:"fieldContext"`
FieldDataType string `json:"fieldDataType"`
Key string `json:"key"`
DataType string `json:"dataType"`
Type string `json:"type"`
}
// telemetryFieldKeyOutput is the current shape only.
type telemetryFieldKeyOutput struct {
Name string `json:"name"`
Signal string `json:"signal"`
FieldContext string `json:"fieldContext"`
FieldDataType string `json:"fieldDataType"`
}
// legacyTypeToFieldContext holds the legacy AttributeKeyType values whose current
// spelling differs. "tag" resolves to attribute through a telemetrytypes alias kept
// only for old DB entries, so store the current spelling rather than rely on it.
var legacyTypeToFieldContext = map[string]string{
"tag": "attribute",
"spanSearchScope": "span",
}
// legacyDataTypeToFieldDataType holds the legacy AttributeKeyDataType values with no
// matching telemetrytypes.FieldDataType alias.
var legacyDataTypeToFieldDataType = map[string]string{
"array(string)": "[]string",
"array(int64)": "[]int64",
"array(float64)": "[]float64",
"array(bool)": "[]bool",
}
type fixSavedViewSelectFields struct {
sqlstore sqlstore.SQLStore
settings factory.ProviderSettings
}
func NewFixSavedViewSelectFieldsFactory(sqlstore sqlstore.SQLStore) factory.ProviderFactory[SQLMigration, Config] {
return factory.NewProviderFactory(factory.MustNewName("fix_saved_view_select_fields"), func(ctx context.Context, ps factory.ProviderSettings, c Config) (SQLMigration, error) {
return &fixSavedViewSelectFields{sqlstore: sqlstore, settings: ps}, nil
})
}
func (migration *fixSavedViewSelectFields) Register(migrations *migrate.Migrations) error {
return migrations.Register(migration.Up, migration.Down)
}
func (migration *fixSavedViewSelectFields) Up(ctx context.Context, db *bun.DB) error {
tx, err := db.BeginTx(ctx, nil)
if err != nil {
return err
}
defer func() { _ = tx.Rollback() }()
var rows []*storableSavedViewSelectFieldsRow
if err := tx.NewSelect().Model(&rows).Scan(ctx); err != nil {
return err
}
var fixed, skipped int
for _, row := range rows {
fixedData, changed, ok := fixSelectFields(row.Data)
if !ok {
migration.settings.Logger.WarnContext(ctx, "saved view data could not be parsed, leaving it untouched", slog.String("saved_view_id", row.ID), slog.String("raw_data", row.Data))
skipped++
continue
}
if !changed {
continue
}
fixed++
if _, err := tx.NewUpdate().Model((*storableSavedViewSelectFieldsRow)(nil)).Set("data = ?", fixedData).Where("id = ?", row.ID).Exec(ctx); err != nil {
return err
}
}
migration.settings.Logger.InfoContext(ctx, "fixed invalid saved view selectedFields entries", slog.Int("total", len(rows)), slog.Int("fixed", fixed), slog.Int("skipped", skipped))
return tx.Commit()
}
func (migration *fixSavedViewSelectFields) Down(context.Context, *bun.DB) error {
return nil
}
// marshalUnescaped encodes without json.Marshal's HTML escaping, which would
// otherwise rewrite <, > and & as \u003c, \u003e and \u0026 throughout the row --
// json.RawMessage included, so it reaches query expressions this migration only
// carries through.
func marshalUnescaped(v any) ([]byte, error) {
var buf bytes.Buffer
encoder := json.NewEncoder(&buf)
encoder.SetEscapeHTML(false)
if err := encoder.Encode(v); err != nil {
return nil, err
}
return bytes.TrimRight(buf.Bytes(), "\n"), nil
}
func fieldContextFromLegacyType(legacyType string) string {
if mapped, ok := legacyTypeToFieldContext[legacyType]; ok {
return mapped
}
return legacyType
}
func fieldDataTypeFromLegacyDataType(legacyDataType string) string {
if mapped, ok := legacyDataTypeToFieldDataType[legacyDataType]; ok {
return mapped
}
return legacyDataType
}
// fixSelectFields recovers or drops entries in spec.selectedFields that never got
// mapped from the legacy key/dataType/type shape to the current
// name/fieldContext/fieldDataType shape. Entries that still carry a legacy key are
// recovered by renaming the fields; entries with neither a name nor a key are dropped
// as unrecoverable. Returns ok=false if data can't be parsed at all, and changed=false
// if there was nothing to fix.
func fixSelectFields(data string) (fixed string, changed bool, ok bool) {
var raw map[string]json.RawMessage
if err := json.Unmarshal([]byte(data), &raw); err != nil {
return "", false, false
}
var spec map[string]json.RawMessage
if err := json.Unmarshal(raw["spec"], &spec); err != nil {
return "", false, false
}
selectedFieldsRaw, ok := spec["selectedFields"]
if !ok {
return "", false, true
}
var fieldsRaw []json.RawMessage
if err := json.Unmarshal(selectedFieldsRaw, &fieldsRaw); err != nil {
return "", false, false
}
fixedFields := make([]json.RawMessage, 0, len(fieldsRaw))
for _, rawField := range fieldsRaw {
var field selectedField
if err := json.Unmarshal(rawField, &field); err != nil {
return "", false, false
}
switch {
case field.Name != "":
// already valid -- keep the original bytes untouched, e.g. to preserve
// description/unit rather than dropping them by re-deriving the entry.
fixedFields = append(fixedFields, rawField)
case field.Key != "":
// legacy shape -- recover by renaming the fields.
recoveredJSON, err := marshalUnescaped(telemetryFieldKeyOutput{
Name: field.Key,
FieldContext: fieldContextFromLegacyType(field.Type),
FieldDataType: fieldDataTypeFromLegacyDataType(field.DataType),
})
if err != nil {
return "", false, false
}
fixedFields = append(fixedFields, recoveredJSON)
changed = true
default:
// neither name nor key -- unrecoverable, drop it.
changed = true
}
}
if !changed {
return "", false, true
}
fixedFieldsJSON, err := marshalUnescaped(fixedFields)
if err != nil {
return "", false, false
}
spec["selectedFields"] = fixedFieldsJSON
fixedSpec, err := marshalUnescaped(spec)
if err != nil {
return "", false, false
}
raw["spec"] = fixedSpec
fixedData, err := marshalUnescaped(raw)
if err != nil {
return "", false, false
}
return string(fixedData), true, true
}

View File

@@ -92,6 +92,11 @@ func (s *SavedViewSpec) Validate() error {
if s.RequestType.IsZero() {
return errors.NewInvalidInputf(ErrCodeSavedViewInvalidInput, "requestType is required")
}
for i, field := range s.SelectedFields {
if field.Name == "" {
return errors.NewInvalidInputf(ErrCodeSavedViewInvalidInput, "selectedFields[%d].name is required", i)
}
}
return (&qbtypes.CompositeQuery{Queries: s.Queries}).Validate(qbtypes.GetValidationOptions(s.RequestType)...)
}

View File

@@ -99,6 +99,17 @@ func TestSavedViewSpecValidate(t *testing.T) {
},
expectError: false,
},
{
name: "selectedFields entry with no name is rejected",
spec: SavedViewSpec{
DisplayName: "My View",
PanelType: PanelTypeTable,
RequestType: qbtypes.RequestTypeScalar,
Queries: validQueries(),
SelectedFields: []telemetrytypes.TelemetryFieldKey{{Name: "service.name"}, {}},
},
expectError: true,
},
{
name: "nil selectedFields is valid -- selectedFields itself is not required",
spec: SavedViewSpec{