Skip to content

Commit ea74000

Browse files
committed
Merge branch 'main' into fix/curate-escalate-rule-list
Conflict in internal/cli/fieldproject_test.go: both sides appended test functions after TestBoundProjectedListErrorNamesLargestFields (this branch: the escalate-rule-list projection suite; main: the identifier-exemption suite). Kept both blocks.
2 parents 3c735e1 + 7b4f15a commit ea74000

16 files changed

Lines changed: 171 additions & 449 deletions

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.25.1
44

55
require (
66
github.com/flashcatcloud/go-flashduty v0.14.2
7-
github.com/mattn/go-runewidth v0.0.27
7+
github.com/mattn/go-runewidth v0.0.28
88
github.com/spf13/cobra v1.10.2
99
github.com/spf13/pflag v1.0.10
1010
github.com/toon-format/toon-go v0.0.0-20251202084852-7ca0e27c4e8c

go.sum

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
github.com/clipperhouse/uax29/v2 v2.2.0 h1:ChwIKnQN3kcZteTXMgb1wztSgaU+ZemkgWdohwgs8tY=
22
github.com/clipperhouse/uax29/v2 v2.2.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM=
33
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
4-
github.com/flashcatcloud/go-flashduty v0.14.1 h1:yrpKPuMJuC1z8bhIaZfGV66lb+sAsBUcqOZZsFZjsiI=
5-
github.com/flashcatcloud/go-flashduty v0.14.1/go.mod h1:aA0RtZEs0AYOwwdNKdtVeD8YMOdnmVY1zAlVD+9Ovx8=
64
github.com/flashcatcloud/go-flashduty v0.14.2 h1:SIF8AMoaF67eEGAiiq8e4y5Dvv/0OOA/VzoxYpDB1CE=
75
github.com/flashcatcloud/go-flashduty v0.14.2/go.mod h1:aA0RtZEs0AYOwwdNKdtVeD8YMOdnmVY1zAlVD+9Ovx8=
86
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
97
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
10-
github.com/mattn/go-runewidth v0.0.27 h1:Feg/Oou5zI/wnpgDF6omIU0OokC9GxLC/WRknhVlIR0=
11-
github.com/mattn/go-runewidth v0.0.27/go.mod h1:3qAiGCV4Koz/yuveO58qUefmUTRm8r0IGEXZ9jeHp/8=
8+
github.com/mattn/go-runewidth v0.0.28 h1:rPyg2ybwEKPebvpzVWe1gKBkH8EQFkxO4Y0hjBeLaBU=
9+
github.com/mattn/go-runewidth v0.0.28/go.mod h1:3qAiGCV4Koz/yuveO58qUefmUTRm8r0IGEXZ9jeHp/8=
1210
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
1311
github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU=
1412
github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4=

internal/cli/coverage_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ var curatedOperationIDs = map[string]bool{
2424
"automation-trigger-write-fire": true,
2525
}
2626

27+
// retiredOperationIDs bridges the release window where the server-side route
28+
// is gone but the CLI still compiles against the previous released SDK spec.
29+
var retiredOperationIDs = map[string]bool{
30+
"monit-preview-sync": true,
31+
"monit-read-query-rows": true,
32+
"monit-rule-write-status": true,
33+
}
34+
2735
// loadSpecOps reads every public GET/POST operation from the openapi spec
2836
// shipped in the linked go-flashduty module — the same spec cligen generates
2937
// against — recording each op's id, path, and whether its 200 response is a
@@ -80,6 +88,9 @@ func loadSpecPaths(t *testing.T) map[string]string {
8088
t.Helper()
8189
ids := map[string]string{}
8290
for _, op := range loadSpecOps(t) {
91+
if retiredOperationIDs[op.id] {
92+
continue
93+
}
8394
ids[op.id] = op.path
8495
}
8596
return ids
@@ -158,6 +169,9 @@ func TestGeneratorTargetsFullSpec(t *testing.T) {
158169
curated := map[string]bool{}
159170
wantGenerated := map[string]bool{}
160171
for _, op := range ops {
172+
if retiredOperationIDs[op.id] {
173+
continue
174+
}
161175
if op.streaming {
162176
streaming[op.id] = true
163177
continue

internal/cli/fieldproject.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,25 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error {
180180
len(encoded), maxBytes, largest)
181181
}
182182

183+
// isIdentifierField reports whether a projected field is an identifier:
184+
// keys ending in _id or _key (incident_id, alert_key, …). Identifier values
185+
// are matched, filtered, and passed back verbatim by the consumer — a jq
186+
// exact-match over --json output, or a follow-up detail call — so clipping
187+
// one silently defeats that consumer: an identifier either survives a
188+
// projection intact or the projection errors out.
189+
func isIdentifierField(key string) bool {
190+
return strings.HasSuffix(key, "_id") || strings.HasSuffix(key, "_key")
191+
}
192+
183193
// boundProjectedList shortens a list projection's string values fairly when
184194
// the compact rows themselves overflow the budget: it finds the largest
185195
// per-field byte cap that still makes everything fit, then applies that one
186-
// cap to every string value across every row. A field already shorter than
187-
// the cap is left completely untouched — only the field(s) actually
188-
// responsible for the overflow (typically a long title) get shortened, each
189-
// marked with "...". The cap never drops low enough to make the "..."
196+
// cap to every shortenable string value across every row. A field already
197+
// shorter than the cap is left completely untouched — only the field(s)
198+
// actually responsible for the overflow (typically a long title) get
199+
// shortened, each marked with "...". Identifier fields (keys ending in _id
200+
// or _key) are exempt at every step — sizing, fitting, and applying — so
201+
// they survive byte-intact. The cap never drops low enough to make the "..."
190202
// marker itself disappear, so a shortened value is always distinguishable
191203
// from a genuinely short one; if no cap at or above that floor fits, the
192204
// command fails with a small error instead of emitting values that look
@@ -213,7 +225,10 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
213225

214226
maxLen := 0
215227
for _, row := range rows {
216-
for _, value := range row {
228+
for key, value := range row {
229+
if isIdentifierField(key) {
230+
continue
231+
}
217232
if text, ok := value.(string); ok && len(text) > maxLen {
218233
maxLen = len(text)
219234
}
@@ -228,7 +243,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
228243
for i, row := range rows {
229244
trialRow := make(map[string]any, len(row))
230245
for key, value := range row {
231-
if text, ok := value.(string); ok {
246+
if text, ok := value.(string); ok && !isIdentifierField(key) {
232247
trialRow[key] = truncateUTF8Bytes(text, limit)
233248
} else {
234249
trialRow[key] = value
@@ -280,6 +295,9 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
280295
fields := map[string]bool{}
281296
for _, row := range rows {
282297
for key, value := range row {
298+
if isIdentifierField(key) {
299+
continue
300+
}
283301
text, ok := value.(string)
284302
if !ok {
285303
continue

internal/cli/fieldproject_test.go

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -781,12 +781,16 @@ func TestAlertEventListFieldsProjectionUnchanged(t *testing.T) {
781781
// dropped to 3 bytes or below, truncateUTF8Bytes's no-room-for-a-marker
782782
// fallback returned raw, unmarked bytes indistinguishable from a genuinely
783783
// short value. Even under extreme row/field pressure that forces every
784-
// string field to shrink, every shortened value must carry the "..." marker.
784+
// shortenable string field to shrink, every shortened value must carry the
785+
// "..." marker. (Row count is sized so the exempt _id fields and the JSON
786+
// envelope still fit at the truncation floor — more rows would tip the page
787+
// into the identifier-overflow error pinned by
788+
// TestBoundProjectedListIdentifierOnlyOverflowErrors.)
785789
func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) {
786790
saveAndResetGlobals(t)
787791
flagOutputFormat = "json"
788792

789-
rows := make([]map[string]any, 100)
793+
rows := make([]map[string]any, 90)
790794
for i := range rows {
791795
rows[i] = map[string]any{
792796
"event_id": fmt.Sprintf("%024x", i),
@@ -1098,3 +1102,99 @@ func TestChannelEscalateRuleListStructuredProjection(t *testing.T) {
10981102
}
10991103
})
11001104
}
1105+
1106+
// TestBoundProjectedListNeverShortensIdentifierFields pins the identifier
1107+
// exemption: keys ending in _id/_key carry values a consumer matches,
1108+
// filters, or passes back verbatim (a jq exact-match over --json output, a
1109+
// follow-up detail call), so shortening one silently defeats that consumer.
1110+
// Even when a page overflows badly enough that the fair cap lands below an
1111+
// identifier's own length, identifiers must come back byte-identical and
1112+
// only free-text fields shorten; the note must name only the clipped fields.
1113+
func TestBoundProjectedListNeverShortensIdentifierFields(t *testing.T) {
1114+
for _, format := range []string{"json", "toon"} {
1115+
t.Run(format, func(t *testing.T) {
1116+
saveAndResetGlobals(t)
1117+
flagOutputFormat = format
1118+
1119+
rows := make([]map[string]any, 10)
1120+
wantIDs := make([]map[string]string, len(rows))
1121+
for i := range rows {
1122+
eventID := fmt.Sprintf("%024x", i)
1123+
alertKey := fmt.Sprintf("%032x", i)
1124+
rows[i] = map[string]any{
1125+
"event_id": eventID,
1126+
"alert_key": alertKey,
1127+
"title": strings.Repeat("a", 500),
1128+
}
1129+
wantIDs[i] = map[string]string{"event_id": eventID, "alert_key": alertKey}
1130+
}
1131+
1132+
const budget = 1400
1133+
note, err := boundProjectedOutput(rows, budget)
1134+
if err != nil {
1135+
t.Fatalf("bound projected output: %v", err)
1136+
}
1137+
1138+
for i, row := range rows {
1139+
for _, field := range []string{"event_id", "alert_key"} {
1140+
if got := row[field].(string); got != wantIDs[i][field] {
1141+
t.Errorf("row %d %s was shortened: got %q, want byte-identical %q", i, field, got, wantIDs[i][field])
1142+
}
1143+
}
1144+
if title := row["title"].(string); !strings.HasSuffix(title, "...") {
1145+
t.Errorf("row %d title should be shortened with the \"...\" marker, got %q", i, title)
1146+
}
1147+
}
1148+
1149+
if note == "" {
1150+
t.Fatal("shortened projection returned no note; caller cannot tell values were clipped")
1151+
}
1152+
if !strings.Contains(note, "title") {
1153+
t.Errorf("note = %q, want it to name the shortened field (title)", note)
1154+
}
1155+
if strings.Contains(note, "event_id") || strings.Contains(note, "alert_key") {
1156+
t.Errorf("note = %q, want it to name only shortened fields, never exempt identifiers", note)
1157+
}
1158+
1159+
encoded, err := marshalStructured(rows)
1160+
if err != nil {
1161+
t.Fatalf("marshal bounded output: %v", err)
1162+
}
1163+
if len(encoded)+1 >= budget {
1164+
t.Errorf("bounded %s output is %d bytes, want <%d", format, len(encoded)+1, budget)
1165+
}
1166+
})
1167+
}
1168+
}
1169+
1170+
// TestBoundProjectedListIdentifierOnlyOverflowErrors pins the other half of
1171+
// the identifier exemption: when a page carries nothing shortenable and its
1172+
// identifier content alone overflows the budget, the command must fail with
1173+
// the narrowing error instead of clipping identifiers to fit — and the rows
1174+
// must come back untouched.
1175+
func TestBoundProjectedListIdentifierOnlyOverflowErrors(t *testing.T) {
1176+
saveAndResetGlobals(t)
1177+
flagOutputFormat = "json"
1178+
1179+
// Sized so the full rows overflow 512 bytes while rows with ids clipped
1180+
// to the truncation floor would still fit: the old fair cap "succeeded"
1181+
// by shipping mangled ids, the exemption must instead refuse.
1182+
rows := make([]map[string]any, 12)
1183+
for i := range rows {
1184+
rows[i] = map[string]any{"incident_id": fmt.Sprintf("%024x", i)}
1185+
}
1186+
originals := make([]string, len(rows))
1187+
for i, row := range rows {
1188+
originals[i] = row["incident_id"].(string)
1189+
}
1190+
1191+
_, err := boundProjectedOutput(rows, 512)
1192+
if err == nil || !strings.Contains(err.Error(), "request fewer rows") {
1193+
t.Fatalf("identifier-only overflow error = %v, want bounded guidance", err)
1194+
}
1195+
for i, row := range rows {
1196+
if got := row["incident_id"].(string); got != originals[i] {
1197+
t.Errorf("row %d incident_id was mutated despite the error: got %q, want %q", i, got, originals[i])
1198+
}
1199+
}
1200+
}

internal/cli/monit_query.go

Lines changed: 1 addition & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cli
22

33
import (
4-
"encoding/json"
54
"fmt"
65
"strconv"
76

@@ -15,7 +14,6 @@ func newMonitQueryCmd() *cobra.Command {
1514
cmd := newGroupCmd("monit-query", "Probe monit-backed datasources (9 types via data; diagnose: loki|victorialogs log patterns, prometheus metric trends)")
1615
cmd.AddCommand(newMonitQueryDiagnoseCmd())
1716
cmd.AddCommand(newMonitQueryDataCmd())
18-
cmd.AddCommand(newMonitQueryRowsCmd())
1917
return cmd
2018
}
2119

@@ -135,74 +133,8 @@ func newMonitQueryDataCmd() *cobra.Command {
135133
return cmd
136134
}
137135

138-
func newMonitQueryRowsCmd() *cobra.Command {
139-
var (
140-
dsType, dsName, expr string
141-
argsKV []string
142-
)
143-
144-
cmd := &cobra.Command{
145-
Use: "rows",
146-
Short: "Raw datasource passthrough (returns values/rows as the datasource itself would). Deprecated — prefer 'monit-query data'",
147-
Deprecated: "use 'monit-query data' instead",
148-
Long: curatedLong("Deprecated. Raw datasource passthrough returning values/rows as the datasource itself would. Migrate to 'monit-query data', which preserves frames/records/samples without forcing results into legacy rows.", "Diagnostics", "QueryRows"),
149-
RunE: func(cmd *cobra.Command, args []string) error {
150-
if dsType == "" || dsName == "" || expr == "" {
151-
return fmt.Errorf("--ds-type, --ds-name, --expr are required")
152-
}
153-
argsMap, err := parseKVSlice(argsKV)
154-
if err != nil {
155-
return fmt.Errorf("invalid --args: %w", err)
156-
}
157-
if err := normalizeRawTimeArgs(dsType, argsMap); err != nil {
158-
return err
159-
}
160-
161-
return runCommand(cmd, args, func(ctx *RunContext) error {
162-
input := &flashduty.QueryRowsRequest{
163-
DsType: dsType,
164-
DsName: dsName,
165-
Expr: expr,
166-
Args: argsMap,
167-
}
168-
result, _, err := ctx.Client.Diagnostics.QueryRows(cmdContext(ctx.Cmd), input)
169-
if err != nil {
170-
return err
171-
}
172-
// This command is a raw datasource passthrough. The legacy SDK
173-
// captured the response body (a JSON array of {fields,values}
174-
// objects) as a RawMessage and wrote it through verbatim,
175-
// independent of the --json/--toon output format. go-flashduty
176-
// decodes that same array into []QueryRow, so re-marshal it to
177-
// the equivalent JSON array and write it through unchanged to
178-
// preserve the legacy single-blob output shape.
179-
if result == nil {
180-
_, err = fmt.Fprintln(ctx.Writer, "{}")
181-
return err
182-
}
183-
body, err := json.Marshal(*result)
184-
if err != nil {
185-
return fmt.Errorf("failed to marshal query rows: %w", err)
186-
}
187-
_, err = fmt.Fprintln(ctx.Writer, string(body))
188-
return err
189-
})
190-
},
191-
}
192-
193-
cmd.Flags().StringVar(&dsType, "ds-type", "", "Datasource type (required)")
194-
cmd.Flags().StringVar(&dsName, "ds-name", "", "Datasource name (required)")
195-
registerEnumFlag(cmd, "ds-type", "prometheus", "victorialogs", "loki", "mysql")
196-
cmd.Flags().StringVar(&expr, "expr", "", "Query expression (required)")
197-
cmd.Flags().StringSliceVar(&argsKV, "args", nil, "Arg entries KEY=VALUE (repeatable; values must be strings per monit-query contract). "+
198-
"For loki/victorialogs raw mode, <ds-type>.start/<ds-type>.end accept a relative duration ('15m'), 'now', a date/RFC3339 timestamp, "+
199-
"or a unix epoch in seconds or milliseconds — normalized to the form the datasource requires before sending")
200-
201-
return cmd
202-
}
203-
204136
// normalizeRawTimeArgs rewrites the raw-mode time-window args of a
205-
// monit-query rows call (<ds-type>.start / <ds-type>.end) into the unix-
137+
// monit-query data call (<ds-type>.start / <ds-type>.end) into the unix-
206138
// seconds form the server requires, accepting any format timeutil.Parse
207139
// understands (RFC3339, date/datetime, relative duration, unix seconds or
208140
// milliseconds). Loki and VictoriaLogs are the only ds-types whose raw mode

0 commit comments

Comments
 (0)