Skip to content

Commit 77a7cb4

Browse files
authored
fix: relax drilldown metadata fallback and extract literals (#225)
1 parent 95fd59d commit 77a7cb4

6 files changed

Lines changed: 145 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- release/metadata: synchronized release metadata for v1.10.2.
1313

14+
### Bug Fixes
15+
16+
- drilldown/metadata: retry relaxed metadata candidates after successful-empty strict scans for native streams, native field names, detected fields, and detected labels so Drilldown labels/fields stay populated when strict parser filters over-constrain the sample.
17+
- translator/patterns: translate `pattern`/`extract` stages without named captures into line filters instead of emitting invalid VictoriaLogs `extract` pipes such as `extract "Metrics"`.
18+
19+
### Tests
20+
21+
- drilldown/translator: update fallback coverage for empty strict scans and add regression cases for literal `extract`/`pattern` stage translation.
22+
1423
## [1.10.2] - 2026-04-20
1524

1625
### Bug Fixes

internal/proxy/coverage_gaps_test.go

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ func TestDetectedFieldValues_FieldFilterFallbackKeepsValuesVisible(t *testing.T)
520520
}
521521
}
522522

523-
func TestDetectedFields_EmptyStrictQueryDoesNotRelaxCandidates(t *testing.T) {
523+
func TestDetectedFields_EmptyStrictQueryRelaxesCandidates(t *testing.T) {
524524
const strictToken = "strict-only"
525525

526526
var fieldNameQueries []string
@@ -565,22 +565,43 @@ func TestDetectedFields_EmptyStrictQueryDoesNotRelaxCandidates(t *testing.T) {
565565
if w.Code != http.StatusOK {
566566
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
567567
}
568-
if len(fieldNameQueries) != 1 {
569-
t.Fatalf("expected only the strict native field-name lookup, got %v", fieldNameQueries)
568+
if len(fieldNameQueries) < 2 {
569+
t.Fatalf("expected strict+relaxed native field-name lookups, got %v", fieldNameQueries)
570570
}
571-
for _, got := range scanQueries {
572-
if !strings.Contains(got, strictToken) {
573-
t.Fatalf("expected scan lookup to preserve strict filter, got %q", got)
574-
}
571+
if !strings.Contains(fieldNameQueries[0], strictToken) {
572+
t.Fatalf("expected first native field-name lookup to stay strict, got %v", fieldNameQueries)
573+
}
574+
if strings.Contains(fieldNameQueries[len(fieldNameQueries)-1], strictToken) {
575+
t.Fatalf("expected final native field-name lookup to relax whole-query filters, got %v", fieldNameQueries)
576+
}
577+
if len(scanQueries) < 2 {
578+
t.Fatalf("expected strict+relaxed field scans, got %v", scanQueries)
579+
}
580+
if !strings.Contains(scanQueries[0], strictToken) {
581+
t.Fatalf("expected first field scan to stay strict, got %v", scanQueries)
582+
}
583+
if strings.Contains(scanQueries[len(scanQueries)-1], strictToken) {
584+
t.Fatalf("expected final field scan to relax whole-query filters, got %v", scanQueries)
575585
}
576586

577587
var resp map[string]interface{}
578588
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
579589
t.Fatalf("unmarshal response: %v", err)
580590
}
581591
fields, _ := resp["fields"].([]interface{})
582-
if len(fields) != 0 {
583-
t.Fatalf("expected empty detected_fields payload for strict empty query, got %v", resp)
592+
if len(fields) == 0 {
593+
t.Fatalf("expected detected_fields payload after relaxed fallback, got %v", resp)
594+
}
595+
foundStatus := false
596+
for _, raw := range fields {
597+
item, _ := raw.(map[string]interface{})
598+
if item["label"] == "status" {
599+
foundStatus = true
600+
break
601+
}
602+
}
603+
if !foundStatus {
604+
t.Fatalf("expected relaxed detected_fields payload to include status, got %v", resp)
584605
}
585606
}
586607

internal/proxy/drilldown.go

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,7 @@ func (p *Proxy) detectFields(ctx context.Context, query, start, end string, line
11331133
candidates := fieldDetectionQueryCandidates(query)
11341134
hadScanFailure := false
11351135
var lastErr error
1136-
for _, candidate := range candidates {
1136+
for i, candidate := range candidates {
11371137
logsqlQuery, err := p.translateQuery(candidate)
11381138
if err != nil {
11391139
lastErr = err
@@ -1183,13 +1183,10 @@ func (p *Proxy) detectFields(ctx context.Context, query, start, end string, line
11831183
p.setCachedDetectedFields(ctx, query, start, end, lineLimit, fieldList, fieldValues)
11841184
return fieldList, fieldValues, nil
11851185
}
1186-
if len(candidates) > 1 && !hadScanFailure {
1187-
emptyFields := []map[string]interface{}{}
1188-
emptyValues := map[string][]string{}
1189-
p.setCachedDetectedFields(ctx, query, start, end, lineLimit, emptyFields, emptyValues)
1190-
return emptyFields, emptyValues, nil
1186+
if i+1 < len(candidates) {
1187+
p.observeInternalOperation(ctx, "discovery_fallback", "detected_fields_empty_primary", 0)
1188+
continue
11911189
}
1192-
break
11931190
}
11941191

11951192
if len(nativeFields) > 0 {
@@ -1203,6 +1200,12 @@ func (p *Proxy) detectFields(ctx context.Context, query, start, end string, line
12031200
p.setCachedDetectedFields(ctx, query, start, end, lineLimit, fieldList, fieldValues)
12041201
return fieldList, fieldValues, nil
12051202
}
1203+
if !hadScanFailure && lastErr == nil {
1204+
emptyFields := []map[string]interface{}{}
1205+
emptyValues := map[string][]string{}
1206+
p.setCachedDetectedFields(ctx, query, start, end, lineLimit, emptyFields, emptyValues)
1207+
return emptyFields, emptyValues, nil
1208+
}
12061209
return nil, nil, lastErr
12071210
}
12081211

@@ -1503,14 +1506,22 @@ func (p *Proxy) fetchNativeFieldNamesForCandidate(ctx context.Context, candidate
15031506

15041507
func (p *Proxy) fetchNativeFieldNames(ctx context.Context, query, start, end string) ([]string, error) {
15051508
var lastErr error
1506-
for _, candidate := range fieldDetectionQueryCandidates(query) {
1509+
candidates := fieldDetectionQueryCandidates(query)
1510+
for i, candidate := range candidates {
15071511
fields, err := p.fetchNativeFieldNamesForCandidate(ctx, candidate, start, end)
15081512
if err != nil {
15091513
lastErr = err
15101514
continue
15111515
}
1516+
if len(fields) == 0 && i+1 < len(candidates) {
1517+
p.observeInternalOperation(ctx, "discovery_fallback", "native_field_names_empty_primary", 0)
1518+
continue
1519+
}
15121520
return fields, nil
15131521
}
1522+
if lastErr == nil {
1523+
return []string{}, nil
1524+
}
15141525
return nil, lastErr
15151526
}
15161527

@@ -1615,6 +1626,7 @@ func (p *Proxy) resolveNativeDetectedField(ctx context.Context, query, start, en
16151626
}
16161627
if i+1 < len(queryCandidates) {
16171628
p.observeInternalOperation(ctx, "discovery_fallback", "native_detected_field_empty_primary", 0)
1629+
continue
16181630
}
16191631
return "", false, nil
16201632
}
@@ -1654,7 +1666,8 @@ func (p *Proxy) detectNativeLabels(ctx context.Context, query, start, end string
16541666

16551667
func (p *Proxy) fetchNativeStreams(ctx context.Context, query, start, end string) (*vlStreamsResponse, error) {
16561668
var lastErr error
1657-
for _, candidate := range fieldDetectionQueryCandidates(query) {
1669+
candidates := fieldDetectionQueryCandidates(query)
1670+
for i, candidate := range candidates {
16581671
logsqlQuery, err := p.translateQuery(candidate)
16591672
if err != nil {
16601673
lastErr = err
@@ -1678,8 +1691,15 @@ func (p *Proxy) fetchNativeStreams(ctx context.Context, query, start, end string
16781691
lastErr = err
16791692
continue
16801693
}
1694+
if len(parsed.Values) == 0 && i+1 < len(candidates) {
1695+
p.observeInternalOperation(ctx, "discovery_fallback", "native_streams_empty_primary", 0)
1696+
continue
1697+
}
16811698
return &parsed, nil
16821699
}
1700+
if lastErr == nil {
1701+
return &vlStreamsResponse{}, nil
1702+
}
16831703
return &vlStreamsResponse{}, lastErr
16841704
}
16851705

@@ -1754,9 +1774,13 @@ func (p *Proxy) detectScannedLabels(ctx context.Context, query, start, end strin
17541774
summaries := scanDetectedLabelSummaries(body, p.labelTranslator)
17551775
if len(summaries) == 0 && i+1 < len(candidates) {
17561776
p.observeInternalOperation(ctx, "discovery_fallback", "detected_labels_empty_primary", 0)
1777+
continue
17571778
}
17581779
return summaries, nil
17591780
}
1781+
if lastErr == nil {
1782+
return map[string]*detectedLabelSummary{}, nil
1783+
}
17601784
return nil, lastErr
17611785
}
17621786

internal/proxy/drilldown_coverage_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func TestFetchNativeFieldValues_RelaxesAfterSuccessfulEmptyPrimary(t *testing.T)
132132
}
133133
}
134134

135-
func TestDetectScannedLabels_DoesNotRelaxOnSuccessfulEmptyPrimary(t *testing.T) {
135+
func TestDetectScannedLabels_RelaxesOnSuccessfulEmptyPrimary(t *testing.T) {
136136
var calls atomic.Int32
137137
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
138138
if r.URL.Path != "/select/logsql/query" {
@@ -153,11 +153,11 @@ func TestDetectScannedLabels_DoesNotRelaxOnSuccessfulEmptyPrimary(t *testing.T)
153153
if err != nil {
154154
t.Fatalf("detectScannedLabels returned error: %v", err)
155155
}
156-
if len(labels) != 0 {
157-
t.Fatalf("expected empty primary response to stop without relaxed fallback, got %#v", labels)
156+
if labels["service_name"] == nil {
157+
t.Fatalf("expected relaxed fallback labels, got %#v", labels)
158158
}
159-
if got := calls.Load(); got != 1 {
160-
t.Fatalf("expected exactly one backend request, got %d", got)
159+
if got := calls.Load(); got != 2 {
160+
t.Fatalf("expected strict+relaxed backend requests, got %d", got)
161161
}
162162
}
163163

internal/translator/translator.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,12 @@ func translatePipelineStage(stage string, labelFn LabelTranslateFunc) string {
322322
// rejects these, so treat them as a no-op stage.
323323
return ""
324324
}
325+
if !hasNamedExtractPlaceholder(patternExpr) {
326+
if filter, ok := patternExpressionToLineFilter(patternExpr); ok {
327+
return filter
328+
}
329+
return ""
330+
}
325331
return "| extract " + patternExpr
326332
}
327333
if strings.HasPrefix(stage, "regexp ") {
@@ -334,6 +340,12 @@ func translatePipelineStage(stage string, labelFn LabelTranslateFunc) string {
334340
if isNoopPatternExpression(patternExpr) {
335341
return ""
336342
}
343+
if !hasNamedExtractPlaceholder(patternExpr) {
344+
if filter, ok := patternExpressionToLineFilter(patternExpr); ok {
345+
return filter
346+
}
347+
return ""
348+
}
337349
return "| extract " + patternExpr
338350
}
339351

@@ -400,6 +412,52 @@ func isNoopPatternExpression(expr string) bool {
400412
}
401413
}
402414

415+
func hasNamedExtractPlaceholder(expr string) bool {
416+
pattern, ok := extractPatternExpressionValue(expr)
417+
if !ok {
418+
return false
419+
}
420+
for {
421+
start := strings.IndexByte(pattern, '<')
422+
if start < 0 {
423+
return false
424+
}
425+
pattern = pattern[start+1:]
426+
end := strings.IndexByte(pattern, '>')
427+
if end < 0 {
428+
return false
429+
}
430+
token := strings.TrimSpace(pattern[:end])
431+
if token != "" && token != "_" {
432+
return true
433+
}
434+
pattern = pattern[end+1:]
435+
}
436+
}
437+
438+
func patternExpressionToLineFilter(expr string) (string, bool) {
439+
pattern, ok := extractPatternExpressionValue(expr)
440+
if !ok {
441+
return "", false
442+
}
443+
return "~" + strconv.Quote(patternFilterValueToRegex(pattern)), true
444+
}
445+
446+
func extractPatternExpressionValue(expr string) (string, bool) {
447+
expr = strings.TrimSpace(expr)
448+
if expr == "" {
449+
return "", false
450+
}
451+
if strings.HasPrefix(expr, "`") && strings.HasSuffix(expr, "`") && len(expr) >= 2 {
452+
return expr[1 : len(expr)-1], true
453+
}
454+
unquoted, err := strconv.Unquote(expr)
455+
if err != nil {
456+
return "", false
457+
}
458+
return unquoted, true
459+
}
460+
403461
// translateLabelFilter handles label comparison filters.
404462
func translateLabelFilter(stage string, labelFn LabelTranslateFunc) string {
405463
if chained, ok := translateLogicalLabelFilterChain(stage, labelFn); ok {

internal/translator/translator_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,16 @@ func TestTranslateLogQL(t *testing.T) {
9696
logql: `{app="nginx"} | pattern "<ip> - - <_>"`,
9797
want: `app:=nginx | extract "<ip> - - <_>"`,
9898
},
99+
{
100+
name: "pattern parser without named field falls back to line filter",
101+
logql: `{app="nginx"} | pattern "Metrics"`,
102+
want: `app:=nginx ~"Metrics"`,
103+
},
104+
{
105+
name: "extract parser without named field falls back to line filter",
106+
logql: `{app="nginx"} | extract "Metrics"`,
107+
want: `app:=nginx ~"Metrics"`,
108+
},
99109
{
100110
name: "pattern wildcard no-op is dropped",
101111
logql: `{app="nginx"} | pattern "(.*)" | status="500"`,

0 commit comments

Comments
 (0)