fix(drilldown): remove unknown_service from non-service metrics (#232)

* fix(drilldown): suppress high-cardinality timestamp terminal fields

* fix(drilldown): suppress unstable fields and unknown service injection

* docs(changelog): add unreleased drilldown and metrics fixes

Author: szibis

CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Bug Fixes
+
+- metrics/query-range: stop injecting synthetic `service_name="unknown_service"` into metric `query`/`query_range` responses when the result labelset has no real service signal (for example `sum by(cluster)(rate(...))`).
+- drilldown/fields: suppress high-cardinality terminal timestamp fields (`timestamp_end`, `observed_timestamp_end`) from `detected_fields` responses to keep Drilldown field discovery stable under repeated refreshes.
+
+### Tests
+
+- drilldown/compat: add unit and e2e regressions for non-service metric aggregations (no synthetic `unknown_service`) and detected-field suppression of terminal timestamp keys.
+
 ## [1.12.0] - 2026-04-21
 
 ### Bug Fixes

docs/compatibility-drilldown.md

@@ -89,6 +89,7 @@ Promotion criteria for a new family:
 - `index/volume_range` must expose non-empty `detected_level` series names
 - `detected_fields` must show parsed fields like `method`, `path`, `status`, `duration_ms`
 - `detected_fields` must not leak indexed labels like `app`, `cluster`, or `namespace`
+- `detected_fields` must suppress high-cardinality terminal timestamp fields (`timestamp_end`, `observed_timestamp_end`) so Drilldown field discovery does not trigger expensive backend stats paths that can flap into intermittent no-data responses
 - In hybrid field mode, `detected_fields` may expose both native dotted fields and translated aliases such as `service.name` and `service_name`
 - `labels` and `label/{name}/values` should stay stream-shaped; they should prefer VictoriaLogs stream metadata endpoints and only fall back to generic field endpoints for older backend versions
 - `detected_fields`, `detected_labels`, and `detected_field/{name}/values` should prefer native VictoriaLogs metadata lookups where they map cleanly, then fall back to bounded raw-log sampling for parsed and derived fields
@@ -103,6 +104,7 @@ Promotion criteria for a new family:
 - Mixed parser query path: `| json ... | logfmt | drop __error__, __error_details__`
 - Labels object parsing in returned log frames
 - App-level field suppression for `detected_level`, `level`, and `level_extracted`
+- High-cardinality terminal timestamp keys (`timestamp_end`, `observed_timestamp_end`) are excluded from Drilldown detected-field responses while regular parsed fields stay visible
 - `1.x` service-selection buckets, detected-fields filtering, and labels field parsing stay explicit in the source-contract checks
 - `2.x` detected-level default columns, field-values breakdown scenes, and additional label-tab wiring stay explicit in the source-contract checks
 - Grafana runtime `11.x` explicitly asserts `1.x`-style service buckets, filtered detected fields, and extra label values at runtime

docs/compatibility-loki.md

@@ -120,6 +120,7 @@ The required matrix is intentionally not limited to happy-path selectors. It now
 - `pattern` parser extraction semantics
 - set-style binary operators such as `or` and `unless`
 - `bool` comparison semantics on metric expressions
+- metric aggregations that do not group by service labels must not receive synthetic `service_name="unknown_service"` in query/query_range responses
 - invalid log/metric shape rejections that must fail with the same class of error as Loki
 
 When a new LogQL family is implemented or fixed in the proxy, the expectation is to add:

internal/proxy/drilldown.go

@@ -18,6 +18,11 @@ import (
 const unknownServiceName = "unknown_service"
 const detectedFieldsSampleLimit = 500
 
+var suppressedDetectedFieldNames = map[string]struct{}{
+	"timestamp_end":          {},
+	"observed_timestamp_end": {},
+}
+
 var serviceNameSourceFields = []string{
 	"service_name",
 	"service.name",
@@ -52,6 +57,27 @@ type detectedLabelSummary struct {
 	values map[string]struct{}
 }
 
+func shouldSuppressDetectedField(label string) bool {
+	normalized := strings.ToLower(strings.TrimSpace(label))
+	if normalized == "" {
+		return false
+	}
+	_, suppressed := suppressedDetectedFieldNames[normalized]
+	return suppressed
+}
+
+func hasServiceSignal(labels map[string]string) bool {
+	if len(labels) == 0 {
+		return false
+	}
+	for _, key := range serviceNameSourceFields {
+		if strings.TrimSpace(labels[key]) != "" {
+			return true
+		}
+	}
+	return false
+}
+
 func deriveServiceName(labels map[string]string) string {
 	for _, key := range serviceNameSourceFields {
 		if value := strings.TrimSpace(labels[key]); value != "" {
@@ -177,6 +203,9 @@ func addDetectedField(fields map[string]*detectedFieldSummary, label, parser, ty
 	if label == "" || strings.TrimSpace(value) == "" {
 		return
 	}
+	if shouldSuppressDetectedField(label) {
+		return
+	}
 	summary := fields[label]
 	if summary == nil {
 		summary = &detectedFieldSummary{
@@ -1411,6 +1440,9 @@ func (p *Proxy) detectNativeFields(ctx context.Context, query, start, end string
 	out := make(map[string]*detectedFieldSummary, len(fieldNames))
 	for _, field := range fieldNames {
 		for _, exposure := range p.metadataFieldExposures(field) {
+			if shouldSuppressDetectedField(exposure.name) {
+				continue
+			}
 			out[exposure.name] = &detectedFieldSummary{
 				label:       exposure.name,
 				typ:         "string",
@@ -1443,6 +1475,9 @@ func mergeNativeDetectedFields(scanned []map[string]interface{}, scannedValues m
 		merged[label] = copied
 	}
 	for label, summary := range native {
+		if shouldSuppressDetectedField(label) {
+			continue
+		}
 		if existing, ok := merged[label]; ok {
 			if card, ok := existing["cardinality"].(int); ok && card > 0 {
 				continue

internal/proxy/drilldown_compat_test.go

@@ -925,6 +925,47 @@ func TestDrilldown_DetectedFields_ParseStructuredLogsInsteadOfIndexedLabels(t *t
 	}
 }
 
+func TestDrilldown_DetectedFields_SuppressHighCardinalityTimestampTerminals(t *testing.T) {
+	vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/select/logsql/field_names":
+			w.Header().Set("Content-Type", "application/json")
+			w.Write([]byte(`{"values":[{"value":"app","hits":1},{"value":"cluster","hits":1},{"value":"timestamp_end","hits":1},{"value":"observed_timestamp_end","hits":1}]}`))
+		case "/select/logsql/query":
+			w.Header().Set("Content-Type", "application/x-ndjson")
+			w.Write([]byte(`{"_time":"2026-04-04T17:18:49.971082Z","_msg":"{\"method\":\"GET\",\"path\":\"/api/v1/users\",\"timestamp_end\":\"2026-04-04T17:18:49.971082Z\",\"observed_timestamp_end\":\"2026-04-04T17:18:49.971082Z\"}","_stream":"{app=\"api-gateway\",cluster=\"us-east-1\"}","app":"api-gateway","cluster":"us-east-1","timestamp_end":"2026-04-04T17:18:49.971082Z","observed_timestamp_end":"2026-04-04T17:18:49.971082Z","level":"info"}` + "\n"))
+		default:
+			t.Fatalf("unexpected backend path %s", r.URL.Path)
+		}
+	}))
+	defer vlBackend.Close()
+
+	resp := doGet(t, vlBackend.URL, "/loki/api/v1/detected_fields?query=%7Bapp%3D%22api-gateway%22%7D")
+	fields, ok := resp["fields"].([]interface{})
+	if !ok {
+		t.Fatalf("expected fields array, got %v", resp)
+	}
+
+	got := make(map[string]struct{}, len(fields))
+	for _, field := range fields {
+		obj := field.(map[string]interface{})
+		got[obj["label"].(string)] = struct{}{}
+	}
+
+	if _, exists := got["timestamp_end"]; exists {
+		t.Fatalf("timestamp_end must be suppressed from detected_fields: %v", got)
+	}
+	if _, exists := got["observed_timestamp_end"]; exists {
+		t.Fatalf("observed_timestamp_end must be suppressed from detected_fields: %v", got)
+	}
+	if _, exists := got["method"]; !exists {
+		t.Fatalf("expected parsed field method to remain discoverable, got %v", got)
+	}
+	if _, exists := got["detected_level"]; !exists {
+		t.Fatalf("expected detected_level summary to remain discoverable, got %v", got)
+	}
+}
+
 func TestDrilldown_DetectedFields_ExposeStructuredMetadataWithDottedNames(t *testing.T) {
 	vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch r.URL.Path {

internal/proxy/drilldown_coverage_test.go

@@ -54,6 +54,8 @@ func TestDrilldownHelpers_AdditionalCoverage(t *testing.T) {
 		addDetectedField(fields, "", "", "string", nil, "ignored")
 		addDetectedField(fields, "duration", "json", "int", []string{"duration"}, "10")
 		addDetectedField(fields, "duration", "logfmt", "string", nil, "ten")
+		addDetectedField(fields, "timestamp_end", "json", "string", nil, "2026-04-21T10:00:00Z")
+		addDetectedField(fields, "observed_timestamp_end", "json", "string", nil, "2026-04-21T10:00:00Z")
 
 		summary := fields["duration"]
 		if summary == nil {
@@ -68,6 +70,15 @@ func TestDrilldownHelpers_AdditionalCoverage(t *testing.T) {
 		if len(summary.jsonPath) != 1 || summary.jsonPath[0] != "duration" {
 			t.Fatalf("expected original json path to be preserved, got %#v", summary.jsonPath)
 		}
+		if _, exists := fields["timestamp_end"]; exists {
+			t.Fatal("expected suppressed high-cardinality timestamp_end field to be ignored")
+		}
+		if _, exists := fields["observed_timestamp_end"]; exists {
+			t.Fatal("expected suppressed high-cardinality observed_timestamp_end field to be ignored")
+		}
+		if !shouldSuppressDetectedField("timestamp_end") {
+			t.Fatal("expected timestamp_end to be part of suppression list")
+		}
 		if got := asString(map[string]any{"line": "hello\nworld"}); !strings.Contains(got, "hello") || strings.Contains(got, "\n") {
 			t.Fatalf("expected asString to flatten JSON, got %q", got)
 		}

internal/proxy/proxy.go

@@ -10249,13 +10249,7 @@ func (p *Proxy) translateVolumeMetric(fields map[string]string) map[string]strin
 	if translated == nil {
 		return nil
 	}
-	serviceSignal := false
-	for _, key := range serviceNameSourceFields {
-		if strings.TrimSpace(translated[key]) != "" {
-			serviceSignal = true
-			break
-		}
-	}
+	serviceSignal := hasServiceSignal(translated)
 	ensureSyntheticServiceName(translated)
 	if !serviceSignal && strings.TrimSpace(translated["service_name"]) == unknownServiceName {
 		delete(translated, "service_name")
@@ -13095,9 +13089,13 @@ func (p *Proxy) translateStatsResponseLabelsWithContext(ctx context.Context, bod
 						syntheticLabels[key] = s
 					}
 				}
+				serviceSignal := hasServiceSignal(syntheticLabels)
 				beforeSyntheticCount := len(syntheticLabels)
 				ensureDetectedLevel(syntheticLabels)
 				ensureSyntheticServiceName(syntheticLabels)
+				if !serviceSignal && strings.TrimSpace(syntheticLabels["service_name"]) == unknownServiceName {
+					delete(syntheticLabels, "service_name")
+				}
 				if len(syntheticLabels) != beforeSyntheticCount {
 					changed = true
 				}

internal/proxy/request_helpers_coverage_test.go

@@ -105,4 +105,23 @@ func TestTranslateStatsResponseLabelsWithContext_Coverage(t *testing.T) {
 	if metric["service_name"] != "api" || metric["detected_level"] != "warn" {
 		t.Fatalf("unexpected translated metric labels %#v", metric)
 	}
+
+	noServiceBody := []byte(`{"result":[{"metric":{"k8s_cluster_name":"ops-sand"}}]}`)
+	noServiceGot := p.translateStatsResponseLabelsWithContext(context.Background(), noServiceBody, `sum by(k8s_cluster_name) (rate({deployment_environment="dev"}[1m]))`)
+
+	var noServiceResp struct {
+		Result []struct {
+			Metric map[string]interface{} `json:"metric"`
+		} `json:"result"`
+	}
+	if err := json.Unmarshal(noServiceGot, &noServiceResp); err != nil {
+		t.Fatalf("decode no-service stats response: %v", err)
+	}
+	noServiceMetric := noServiceResp.Result[0].Metric
+	if _, exists := noServiceMetric["service_name"]; exists {
+		t.Fatalf("unexpected synthetic service_name for metric without service signal: %#v", noServiceMetric)
+	}
+	if noServiceMetric["k8s_cluster_name"] != "ops-sand" {
+		t.Fatalf("expected original metric labels to remain intact, got %#v", noServiceMetric)
+	}
 }

test/e2e-compat/drilldown_compat_test.go

@@ -203,6 +203,77 @@ func TestDrilldown_DetectedFieldsMatchStructuredLogs(t *testing.T) {
 	}
 }
 
+func TestDrilldown_DetectedFieldsSuppressTimestampTerminalFields(t *testing.T) {
+	ensureDataIngested(t)
+
+	now := time.Now()
+	pushStream(t, now, streamDef{
+		Labels: map[string]string{
+			"app": "timestamp-heavy-service", "namespace": "prod", "env": "production",
+			"cluster": "us-east-1", "pod": "timestamp-heavy-service-abc123",
+			"container": "timestamp-heavy-service", "level": "info",
+		},
+		Lines: []string{
+			`{"method":"GET","path":"/api/v1/slow","status":200,"timestamp_end":"2026-04-21T08:34:59.661518472Z","observed_timestamp_end":"2026-04-21T08:34:59.661518472Z"}`,
+			`{"method":"POST","path":"/api/v1/slow","status":201,"timestamp_end":"2026-04-21T08:35:00.139166515Z","observed_timestamp_end":"2026-04-21T08:35:00.139166515Z"}`,
+		},
+	})
+	time.Sleep(3 * time.Second)
+
+	params := url.Values{}
+	params.Set("query", `{service_name="timestamp-heavy-service"}`)
+	params.Set("start", fmt.Sprintf("%d", now.Add(-15*time.Minute).UnixNano()))
+	params.Set("end", fmt.Sprintf("%d", now.Add(15*time.Minute).UnixNano()))
+
+	proxyResp := getJSON(t, proxyURL+"/loki/api/v1/detected_fields?"+params.Encode())
+	proxyFields, _ := proxyResp["fields"].([]interface{})
+	if len(proxyFields) == 0 {
+		t.Fatalf("proxy detected_fields empty for timestamp-heavy service query: %v", proxyResp)
+	}
+
+	proxyLabels := make(map[string]bool, len(proxyFields))
+	for _, field := range proxyFields {
+		proxyLabels[field.(map[string]interface{})["label"].(string)] = true
+	}
+
+	if !proxyLabels["method"] {
+		t.Fatalf("proxy detected_fields missing parsed method field: %v", proxyResp)
+	}
+	for _, suppressed := range []string{"timestamp_end", "observed_timestamp_end"} {
+		if proxyLabels[suppressed] {
+			t.Fatalf("proxy detected_fields must suppress high-cardinality field %q: %v", suppressed, proxyResp)
+		}
+	}
+}
+
+func TestDrilldown_StatsQueryDoesNotInjectUnknownServiceName(t *testing.T) {
+	ensureDataIngested(t)
+	now := time.Now()
+
+	params := url.Values{}
+	params.Set("query", `sum by(cluster) (rate({env="production",cluster="us-east-1"}[1m]))`)
+	params.Set("start", fmt.Sprintf("%d", now.Add(-15*time.Minute).UnixNano()))
+	params.Set("end", fmt.Sprintf("%d", now.UnixNano()))
+	params.Set("step", "60")
+
+	proxyResp := getJSON(t, proxyURL+"/loki/api/v1/query_range?"+params.Encode())
+	data := extractMap(proxyResp, "data")
+	if data == nil {
+		t.Fatalf("expected query_range data envelope, got %v", proxyResp)
+	}
+	result := extractArray(data, "result")
+	if len(result) == 0 {
+		t.Fatalf("expected non-empty stats result for cluster aggregation query, got %v", proxyResp)
+	}
+
+	for _, item := range result {
+		metric := item.(map[string]interface{})["metric"].(map[string]interface{})
+		if _, exists := metric["service_name"]; exists {
+			t.Fatalf("stats metric without service grouping must not inject synthetic service_name: %v", metric)
+		}
+	}
+}
+
 func TestDrilldown_GrafanaResourceContracts(t *testing.T) {
 	ensureDataIngested(t)
 	ingestPatternData(t)