Skip to content

Commit 1385def

Browse files
hanksHan Zhou
hanks
authored and
Han Zhou
committed
fix: preserve extra model fields in AlertQueryModel for non-PromQL datasources
AlertQueryModel only had typed fields for PromQL-style queries (expr, type, expression, reducer, conditions). Fields specific to other datasources like Graphite (target, datasource, textEditor) and classic_conditions fields (operator, query, reducer) were silently dropped during JSON round-tripping in convertAlertQueries(). Changes: - Add Extra map[string]any to AlertQueryModel with custom MarshalJSON/ UnmarshalJSON to preserve all datasource-specific fields - Use knownConsumed tracking so known keys with unexpected types (null, number, object) fall through to Extra instead of being silently lost - Add missing Operator, Query, Reducer fields to AlertCondition for classic_conditions support - Add unit tests for Graphite model round-tripping, classic_conditions field preservation, and JSON round-trip correctness Fixes #731
1 parent fd83688 commit 1385def

2 files changed

Lines changed: 270 additions & 3 deletions

File tree

‎tools/alerting_manage_rules_types.go‎

Lines changed: 119 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,134 @@ type AlertQuery struct {
7171
Model AlertQueryModel `json:"model" jsonschema:"required,description=Query model. For data sources: set expr. For expressions: set type and expression."`
7272
}
7373

74+
// AlertQueryModel represents the query model for an alert query.
75+
// It has typed fields for common PromQL/expression properties, plus an Extra
76+
// map that preserves all additional datasource-specific fields (e.g. Graphite
77+
// "target", "datasource", "textEditor") that would otherwise be silently
78+
// dropped during JSON round-tripping.
7479
type AlertQueryModel struct {
7580
Expr string `json:"expr,omitempty" jsonschema:"description=Query expression (PromQL\\, LogQL\\, etc.)"`
7681

7782
// Server-side expressions only (when datasourceUid is __expr__).
78-
Type string `json:"type,omitempty" jsonschema:"description=Expression type (only for __expr__ datasource): math\\, reduce\\, threshold"`
83+
Type string `json:"type,omitempty" jsonschema:"description=Expression type (only for __expr__ datasource): math\\, reduce\\, threshold\\, classic_conditions"`
7984
Expression string `json:"expression,omitempty" jsonschema:"description=Expression ref or formula. For reduce/threshold: ref like 'A'. For math: '$A > 0'."`
8085
Reducer string `json:"reducer,omitempty" jsonschema:"description=Reducer for reduce expressions: last\\, mean\\, min\\, max\\, sum\\, count"`
81-
Conditions []AlertCondition `json:"conditions,omitempty" jsonschema:"description=Conditions for threshold expressions"`
86+
Conditions []AlertCondition `json:"conditions,omitempty" jsonschema:"description=Conditions for threshold or classic_conditions expressions"`
87+
88+
// Extra captures all additional model fields not explicitly typed above
89+
// (e.g. Graphite "target", "datasource", "textEditor", "intervalMs",
90+
// "maxDataPoints", "refId", etc.). This ensures datasource-specific
91+
// fields survive JSON round-tripping through convertAlertQueries.
92+
Extra map[string]any `json:"-"`
8293
}
8394

95+
// MarshalJSON implements custom JSON marshaling that merges typed fields with
96+
// Extra overflow fields, ensuring datasource-specific properties are preserved.
97+
func (m AlertQueryModel) MarshalJSON() ([]byte, error) {
98+
result := make(map[string]any)
99+
100+
// Start with extra fields as the base
101+
for k, v := range m.Extra {
102+
result[k] = v
103+
}
104+
105+
// Overlay typed fields (they take precedence over Extra)
106+
if m.Expr != "" {
107+
result["expr"] = m.Expr
108+
}
109+
if m.Type != "" {
110+
result["type"] = m.Type
111+
}
112+
if m.Expression != "" {
113+
result["expression"] = m.Expression
114+
}
115+
if m.Reducer != "" {
116+
result["reducer"] = m.Reducer
117+
}
118+
if len(m.Conditions) > 0 {
119+
result["conditions"] = m.Conditions
120+
}
121+
122+
return json.Marshal(result)
123+
}
124+
125+
// UnmarshalJSON implements custom JSON unmarshaling that populates typed fields
126+
// and stores all remaining fields in Extra.
127+
func (m *AlertQueryModel) UnmarshalJSON(data []byte) error {
128+
// Unmarshal everything into a generic map first
129+
var raw map[string]any
130+
if err := json.Unmarshal(data, &raw); err != nil {
131+
return err
132+
}
133+
134+
// Extract known typed fields. If a known key exists but has an unexpected
135+
// type (e.g. null, number, object instead of string), fall through to
136+
// Extra so the value is preserved in the round-trip rather than silently
137+
// dropped.
138+
knownConsumed := make(map[string]bool)
139+
140+
if v, ok := raw["expr"].(string); ok {
141+
m.Expr = v
142+
knownConsumed["expr"] = true
143+
}
144+
if v, ok := raw["type"].(string); ok {
145+
m.Type = v
146+
knownConsumed["type"] = true
147+
}
148+
if v, ok := raw["expression"].(string); ok {
149+
m.Expression = v
150+
knownConsumed["expression"] = true
151+
}
152+
if v, ok := raw["reducer"].(string); ok {
153+
m.Reducer = v
154+
knownConsumed["reducer"] = true
155+
}
156+
if v, ok := raw["conditions"]; ok {
157+
condBytes, err := json.Marshal(v)
158+
if err != nil {
159+
return fmt.Errorf("marshal conditions: %w", err)
160+
}
161+
if err := json.Unmarshal(condBytes, &m.Conditions); err != nil {
162+
return fmt.Errorf("unmarshal conditions: %w", err)
163+
}
164+
knownConsumed["conditions"] = true
165+
}
166+
167+
// Store all remaining fields in Extra — including known keys whose
168+
// type assertion failed above, so no value is silently lost.
169+
m.Extra = make(map[string]any)
170+
for k, v := range raw {
171+
if !knownConsumed[k] {
172+
m.Extra[k] = v
173+
}
174+
}
175+
176+
return nil
177+
}
178+
179+
// AlertCondition represents a condition within a classic_conditions or threshold expression.
180+
// It includes the full set of fields used by classic_conditions (evaluator, operator, query, reducer).
84181
type AlertCondition struct {
85-
Evaluator ConditionEvaluator `json:"evaluator" jsonschema:"description=Threshold evaluator"`
182+
Evaluator ConditionEvaluator `json:"evaluator" jsonschema:"description=Threshold evaluator"`
183+
Operator *ConditionOperator `json:"operator,omitempty" jsonschema:"description=Logical operator (and/or) for combining conditions"`
184+
Query *ConditionQuery `json:"query,omitempty" jsonschema:"description=Query reference for classic_conditions (contains params with RefID)"`
185+
Reducer *ConditionReducer `json:"reducer,omitempty" jsonschema:"description=Reducer for classic_conditions (avg\\, sum\\, min\\, max\\, count\\, last\\, median)"`
186+
}
187+
188+
// ConditionOperator represents the logical operator in a classic_conditions condition.
189+
type ConditionOperator struct {
190+
Type string `json:"type" jsonschema:"description=Logical operator type: and\\, or"`
191+
}
192+
193+
// ConditionQuery represents the query reference in a classic_conditions condition.
194+
type ConditionQuery struct {
195+
Params []string `json:"params" jsonschema:"description=Query parameters\\, typically contains the RefID of the referenced query (e.g. ['C'])"`
196+
}
197+
198+
// ConditionReducer represents the reducer in a classic_conditions condition.
199+
type ConditionReducer struct {
200+
Type string `json:"type" jsonschema:"description=Reducer type: avg\\, sum\\, min\\, max\\, count\\, last\\, median"`
201+
Params []string `json:"params,omitempty" jsonschema:"description=Optional reducer parameters"`
86202
}
87203

88204
type ConditionEvaluator struct {

‎tools/alerting_manage_rules_unit_test.go‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,6 +1329,157 @@ func TestConvertAlertQueries(t *testing.T) {
13291329
require.Equal(t, models.Duration(3600), result[0].RelativeTimeRange.From)
13301330
require.Equal(t, models.Duration(1800), result[0].RelativeTimeRange.To)
13311331
})
1332+
1333+
t.Run("preserves extra model fields for Graphite queries", func(t *testing.T) {
1334+
queries := []*AlertQuery{
1335+
{
1336+
RefID: "C",
1337+
DatasourceUID: "000000004",
1338+
RelativeTimeRange: &RelativeTimeRange{From: 300, To: 0},
1339+
Model: AlertQueryModel{
1340+
Extra: map[string]any{
1341+
"datasource": map[string]any{"type": "graphite", "uid": "000000004"},
1342+
"target": "alias(asPercent(sumSeries(a), sumSeries(b)), 'error rate')",
1343+
"textEditor": true,
1344+
"refId": "C",
1345+
},
1346+
},
1347+
},
1348+
}
1349+
result, err := convertAlertQueries(queries)
1350+
require.NoError(t, err)
1351+
require.Len(t, result, 1)
1352+
require.Equal(t, "C", result[0].RefID)
1353+
1354+
model := result[0].Model.(map[string]any)
1355+
require.Equal(t, "alias(asPercent(sumSeries(a), sumSeries(b)), 'error rate')", model["target"])
1356+
require.Equal(t, true, model["textEditor"])
1357+
ds, ok := model["datasource"].(map[string]any)
1358+
require.True(t, ok)
1359+
require.Equal(t, "graphite", ds["type"])
1360+
})
1361+
1362+
t.Run("preserves classic_conditions with operator, query, and reducer", func(t *testing.T) {
1363+
queries := []*AlertQuery{
1364+
{
1365+
RefID: "A",
1366+
DatasourceUID: "__expr__",
1367+
Model: AlertQueryModel{
1368+
Type: "classic_conditions",
1369+
Conditions: []AlertCondition{
1370+
{
1371+
Evaluator: ConditionEvaluator{Type: "gt", Params: []float64{0.1}},
1372+
Operator: &ConditionOperator{Type: "and"},
1373+
Query: &ConditionQuery{Params: []string{"C"}},
1374+
Reducer: &ConditionReducer{Type: "avg"},
1375+
},
1376+
},
1377+
},
1378+
},
1379+
}
1380+
result, err := convertAlertQueries(queries)
1381+
require.NoError(t, err)
1382+
require.Len(t, result, 1)
1383+
1384+
model := result[0].Model.(map[string]any)
1385+
conditions, ok := model["conditions"].([]any)
1386+
require.True(t, ok)
1387+
require.Len(t, conditions, 1)
1388+
1389+
cond := conditions[0].(map[string]any)
1390+
// Verify evaluator
1391+
evaluator := cond["evaluator"].(map[string]any)
1392+
require.Equal(t, "gt", evaluator["type"])
1393+
// Verify operator is preserved
1394+
operator := cond["operator"].(map[string]any)
1395+
require.Equal(t, "and", operator["type"])
1396+
// Verify query params (RefID reference) is preserved
1397+
query := cond["query"].(map[string]any)
1398+
params := query["params"].([]any)
1399+
require.Equal(t, "C", params[0])
1400+
// Verify reducer is preserved
1401+
reducer := cond["reducer"].(map[string]any)
1402+
require.Equal(t, "avg", reducer["type"])
1403+
})
1404+
}
1405+
1406+
func TestAlertQueryModel_JSONRoundTrip(t *testing.T) {
1407+
t.Run("round-trips Graphite model without data loss", func(t *testing.T) {
1408+
input := `{
1409+
"datasource": {"type": "graphite", "uid": "000000004"},
1410+
"target": "alias(asPercent(a, b), 'rate')",
1411+
"textEditor": true,
1412+
"intervalMs": 1000,
1413+
"maxDataPoints": 43200,
1414+
"refCount": 0,
1415+
"refId": "C"
1416+
}`
1417+
1418+
var model AlertQueryModel
1419+
err := json.Unmarshal([]byte(input), &model)
1420+
require.NoError(t, err)
1421+
1422+
// Typed fields should be empty (Graphite doesn't use them)
1423+
require.Empty(t, model.Expr)
1424+
require.Empty(t, model.Type)
1425+
1426+
// Extra should have all Graphite-specific fields
1427+
require.Equal(t, "alias(asPercent(a, b), 'rate')", model.Extra["target"])
1428+
require.Equal(t, true, model.Extra["textEditor"])
1429+
require.Equal(t, float64(1000), model.Extra["intervalMs"])
1430+
1431+
// Re-marshal and verify no data loss
1432+
output, err := json.Marshal(model)
1433+
require.NoError(t, err)
1434+
1435+
var roundTripped map[string]any
1436+
err = json.Unmarshal(output, &roundTripped)
1437+
require.NoError(t, err)
1438+
1439+
require.Equal(t, "alias(asPercent(a, b), 'rate')", roundTripped["target"])
1440+
require.Equal(t, true, roundTripped["textEditor"])
1441+
require.Equal(t, float64(1000), roundTripped["intervalMs"])
1442+
})
1443+
1444+
t.Run("round-trips classic_conditions with all fields", func(t *testing.T) {
1445+
input := `{
1446+
"type": "classic_conditions",
1447+
"conditions": [
1448+
{
1449+
"evaluator": {"type": "gt", "params": [0.1]},
1450+
"operator": {"type": "and"},
1451+
"query": {"params": ["C"]},
1452+
"reducer": {"type": "avg"}
1453+
}
1454+
]
1455+
}`
1456+
1457+
var model AlertQueryModel
1458+
err := json.Unmarshal([]byte(input), &model)
1459+
require.NoError(t, err)
1460+
require.Equal(t, "classic_conditions", model.Type)
1461+
require.Len(t, model.Conditions, 1)
1462+
require.NotNil(t, model.Conditions[0].Operator)
1463+
require.Equal(t, "and", model.Conditions[0].Operator.Type)
1464+
require.NotNil(t, model.Conditions[0].Query)
1465+
require.Equal(t, []string{"C"}, model.Conditions[0].Query.Params)
1466+
require.NotNil(t, model.Conditions[0].Reducer)
1467+
require.Equal(t, "avg", model.Conditions[0].Reducer.Type)
1468+
1469+
// Re-marshal
1470+
output, err := json.Marshal(model)
1471+
require.NoError(t, err)
1472+
1473+
var roundTripped map[string]any
1474+
err = json.Unmarshal(output, &roundTripped)
1475+
require.NoError(t, err)
1476+
1477+
conditions := roundTripped["conditions"].([]any)
1478+
cond := conditions[0].(map[string]any)
1479+
query := cond["query"].(map[string]any)
1480+
params := query["params"].([]any)
1481+
require.Equal(t, "C", params[0])
1482+
})
13321483
}
13331484

13341485
func TestExtractQuerySummaries(t *testing.T) {

0 commit comments

Comments
 (0)