review suggestions

grafana · Jun 25, 2024 · 436249e · 436249e
1 parent 0ea06a5
commit 436249e
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -94,7 +94,6 @@ Starting with the 3.0 release we began using [conventional commits](https://www.
 * [11587](https://github.com/grafana/loki/pull/11587) **trevorwhitney** Fix semantics of label parsing logic of metrics and logs queries. Both only parse the first label if multiple extractions into the same label are requested.
 * [11776](https://github.com/grafana/loki/pull/11776) **ashwanthgoli** Background Cache: Fixes a bug that is causing the background queue size to be incremented twice for each enqueued item.
 * [11921](https://github.com/grafana/loki/pull/11921) **paul1r**: Parsing: String array elements were not being parsed correctly in JSON processing
-* [12102](https://github.com/grafana/loki/pull/12102) **ashwanthgoli**: Report correct status code for metric and logs queries in `metrics.go`
 
 * All lifecycler cfgs ref a valid IPv6 addr and port combination ([#11121](https://github.com/grafana/loki/issues/11121)) ([6385b19](https://github.com/grafana/loki/commit/6385b195739bd7d4e9706faddd0de663d8e5331a))
 * **deps:** update github.com/c2h5oh/datasize digest to 859f65c (main) ([#10820](https://github.com/grafana/loki/issues/10820)) ([c66ffd1](https://github.com/grafana/loki/commit/c66ffd125cd89f5845a75a1751186fa46d003f70))

diff --git a/pkg/querier/http.go b/pkg/querier/http.go
@@ -116,11 +116,7 @@ func (q *QuerierAPI) LabelHandler(ctx context.Context, req *logproto.LabelReques
 		sp.LogKV(statResult.KVList()...)
 	}
 
-	status := 200
-	if err != nil {
-		status, _ = serverutil.ClientHTTPStatusAndError(err)
-	}
-
+	status, _ := serverutil.ClientHTTPStatusAndError(err)
 	logql.RecordLabelQueryMetrics(ctx, util_log.Logger, *req.Start, *req.End, req.Name, req.Query, strconv.Itoa(status), statResult)
 
 	return resp, err
@@ -271,11 +267,7 @@ func (q *QuerierAPI) SeriesHandler(ctx context.Context, req *logproto.SeriesRequ
 		sp.LogKV(statResult.KVList()...)
 	}
 
-	status := 200
-	if err != nil {
-		status, _ = serverutil.ClientHTTPStatusAndError(err)
-	}
-
+	status, _ := serverutil.ClientHTTPStatusAndError(err)
 	logql.RecordSeriesQueryMetrics(ctx, util_log.Logger, req.Start, req.End, req.Groups, strconv.Itoa(status), req.GetShards(), statResult)
 
 	return resp, statResult, err
@@ -302,11 +294,7 @@ func (q *QuerierAPI) IndexStatsHandler(ctx context.Context, req *loghttp.RangeQu
 		sp.LogKV(statResult.KVList()...)
 	}
 
-	status := 200
-	if err != nil {
-		status, _ = serverutil.ClientHTTPStatusAndError(err)
-	}
-
+	status, _ := serverutil.ClientHTTPStatusAndError(err)
 	logql.RecordStatsQueryMetrics(ctx, util_log.Logger, req.Start, req.End, req.Query, strconv.Itoa(status), statResult)
 
 	return resp, err
@@ -334,11 +322,7 @@ func (q *QuerierAPI) IndexShardsHandler(ctx context.Context, req *loghttp.RangeQ
 		sp.LogKV(statResult.KVList()...)
 	}
 
-	status := 200
-	if err != nil {
-		status, _ = serverutil.ClientHTTPStatusAndError(err)
-	}
-
+	status, _ := serverutil.ClientHTTPStatusAndError(err)
 	logql.RecordShardsQueryMetrics(
 		ctx, util_log.Logger, req.Start, req.End, req.Query, targetBytesPerShard, strconv.Itoa(status), resLength, statResult,
 	)
@@ -371,11 +355,7 @@ func (q *QuerierAPI) VolumeHandler(ctx context.Context, req *logproto.VolumeRequ
 		sp.LogKV(statResult.KVList()...)
 	}
 
-	status := 200
-	if err != nil {
-		status, _ = serverutil.ClientHTTPStatusAndError(err)
-	}
-
+	status, _ := serverutil.ClientHTTPStatusAndError(err)
 	logql.RecordVolumeQueryMetrics(ctx, util_log.Logger, req.From.Time(), req.Through.Time(), req.GetQuery(), uint32(req.GetLimit()), time.Duration(req.GetStep()), strconv.Itoa(status), statResult)
 
 	return resp, nil

diff --git a/pkg/util/server/error.go b/pkg/util/server/error.go
@@ -48,6 +48,10 @@ func WriteError(err error, w http.ResponseWriter) {
 // ClientHTTPStatusAndError returns error and http status that is "safe" to return to client without
 // exposing any implementation details.
 func ClientHTTPStatusAndError(err error) (int, error) {
+	if err == nil {
+		return http.StatusOK, nil
+	}
+
 	var (
 		queryErr storage_errors.QueryError
 		promErr  promql.ErrStorage