fix(logql): remove __error_details__ label on drop __error__ or other…

… way around (grafana#8150)
owen-d · Jan 18, 2023 · cf23194 · cf23194
1 parent 5cfd6a3
commit cf23194
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 21 deletions.
diff --git a/docs/sources/logql/log_queries.md b/docs/sources/logql/log_queries.md
@@ -593,6 +593,8 @@ the result will be
 {} INFO GET / loki.net 200
 ```
 
+**Note:** `|drop __error__` or `|drop __error__="some error"` will also drop `__error__details__` label. Similarly, `|drop __error_details__` or `|drop __error_details__="some error details"` will also drop `__error__` label.
+
 Example with regex and multiple names
 
 For the query `{job="varlogs"}|json|drop level, path, app=~"some-api.*"`, with below log lines

diff --git a/pkg/logql/log/drop_labels.go b/pkg/logql/log/drop_labels.go
@@ -47,13 +47,14 @@ func isErrorDetailsLabel(name string) bool {
 	return name == logqlmodel.ErrorDetailsLabel
 }
 
+func resetError(lbls *LabelsBuilder) {
+	lbls.ResetError()
+	lbls.ResetErrorDetails()
+}
+
 func dropLabelNames(name string, lbls *LabelsBuilder) {
-	if isErrorLabel(name) {
-		lbls.ResetError()
-		return
-	}
-	if isErrorDetailsLabel(name) {
-		lbls.ResetErrorDetails()
+	if isErrorLabel(name) || isErrorDetailsLabel(name) {
+		resetError(lbls)
 		return
 	}
 	if _, ok := lbls.Get(name); ok {
@@ -67,14 +68,14 @@ func dropLabelMatches(matcher *labels.Matcher, lbls *LabelsBuilder) {
 	if isErrorLabel(name) {
 		value = lbls.GetErr()
 		if matcher.Matches(value) {
-			lbls.ResetError()
+			resetError(lbls)
 		}
 		return
 	}
 	if isErrorDetailsLabel(name) {
 		value = lbls.GetErrorDetails()
 		if matcher.Matches(value) {
-			lbls.ResetErrorDetails()
+			resetError(lbls)
 		}
 		return
 	}

diff --git a/pkg/logql/log/drop_labels_test.go b/pkg/logql/log/drop_labels_test.go
@@ -49,10 +49,6 @@ func Test_DropLabels(t *testing.T) {
 					labels.MustNewMatcher(labels.MatchEqual, logqlmodel.ErrorLabel, errJSON),
 					"",
 				},
-				{
-					nil,
-					"__error_details__",
-				},
 			},
 			errJSON,
 			"json error",
@@ -92,6 +88,27 @@ func Test_DropLabels(t *testing.T) {
 		},
 		{
 			"drop by __error_details__",
+			[]DropLabel{
+				{
+					labels.MustNewMatcher(labels.MatchRegexp, logqlmodel.ErrorDetailsLabel, "expecting json.*"),
+					"",
+				},
+			},
+			errJSON,
+			"expecting json object but it is not",
+			labels.Labels{
+				{Name: "app", Value: "foo"},
+				{Name: "namespace", Value: "prod"},
+				{Name: "pod_uuid", Value: "foo"},
+			},
+			labels.Labels{
+				{Name: "app", Value: "foo"},
+				{Name: "namespace", Value: "prod"},
+				{Name: "pod_uuid", Value: "foo"},
+			},
+		},
+		{
+			"using both __error_details__ and __error__ should have expected labels ",
 			[]DropLabel{
 				{
 					labels.MustNewMatcher(labels.MatchRegexp, logqlmodel.ErrorDetailsLabel, "expecting json.*"),
@@ -122,10 +139,6 @@ func Test_DropLabels(t *testing.T) {
 					labels.MustNewMatcher(labels.MatchEqual, logqlmodel.ErrorLabel, errJSON),
 					"",
 				},
-				{
-					nil,
-					"__error_details__",
-				},
 				{
 					nil,
 					"app",

diff --git a/pkg/logql/log/pipeline_test.go b/pkg/logql/log/pipeline_test.go
@@ -153,10 +153,6 @@ func TestDropLabelsPipeline(t *testing.T) {
 						nil,
 						"__error__",
 					},
-					{
-						nil,
-						"__error_details__",
-					},
 				}),
 			},
 			[][]byte{
@@ -222,7 +218,6 @@ func TestDropLabelsPipeline(t *testing.T) {
 					{Name: "namespace", Value: "prod"},
 					{Name: "pod_uuid", Value: "foo"},
 					{Name: "pod_deployment_ref", Value: "foobar"},
-					{Name: logqlmodel.ErrorDetailsLabel, Value: "logfmt syntax error at pos 2 : unexpected '\"'"},
 				},
 			},
 		},