query: handle query.Analyze returning nil gracefully (#8199)

* fix: handle analyze returning nil gracefully

Signed-off-by: Saumya Shah <[email protected]>

* update CHANGELOG.md

Signed-off-by: Saumya Shah <[email protected]>

* fix format

Signed-off-by: Saumya Shah <[email protected]>

---------

Signed-off-by: Saumya Shah <[email protected]>

Author: Saumya40-codes

CHANGELOG.md

@@ -28,6 +28,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 ### Removed
 
 ### Fixed
+- [#8199](https://github.com/thanos-io/thanos/pull/8199) Query: handle panics or nil pointer dereference in querier gracefully when query analyze returns nil
 
 - [#8211](https://github.com/thanos-io/thanos/pull/8211) Query: fix panic on nested partial response in distributed instant query
 - [#8216](https://github.com/thanos-io/thanos/pull/8216) Query/Receive: fix iter race between `next()` and `stop()` introduced in https://github.com/thanos-io/thanos/pull/7821.

pkg/api/query/grpc.go

@@ -273,6 +273,9 @@ func extractQueryStats(qry promql.Query) *querypb.QueryStats {
 	}
 	if explQry, ok := qry.(engine.ExplainableQuery); ok {
 		analyze := explQry.Analyze()
+		if analyze == nil {
+			return stats
+		}
 		stats.SamplesTotal = analyze.TotalSamples()
 		stats.PeakSamples = analyze.PeakSamples()
 	}

pkg/api/query/v1.go

@@ -407,7 +407,6 @@ func (qapi *QueryAPI) getQueryExplain(query promql.Query) (*engine.ExplainOutput
 		return eq.Explain(), nil
 	}
 	return nil, &api.ApiError{Typ: api.ErrorBadData, Err: errors.Errorf("Query not explainable")}
-
 }
 
 func (qapi *QueryAPI) parseQueryAnalyzeParam(r *http.Request) bool {
@@ -416,7 +415,11 @@ func (qapi *QueryAPI) parseQueryAnalyzeParam(r *http.Request) bool {
 
 func analyzeQueryOutput(query promql.Query, engineType PromqlEngineType) (queryTelemetry, error) {
 	if eq, ok := query.(engine.ExplainableQuery); ok {
-		return processAnalysis(eq.Analyze()), nil
+		if analyze := eq.Analyze(); analyze != nil {
+			return processAnalysis(analyze), nil
+		} else {
+			return queryTelemetry{}, errors.Errorf("Query: %v not analyzable", query)
+		}
 	}
 
 	var warning error
@@ -539,7 +542,6 @@ func (qapi *QueryAPI) queryExplain(r *http.Request) (interface{}, []error, *api.
 		var qErr error
 		qry, qErr = qapi.queryCreate.makeInstantQuery(ctx, engineParam, queryable, remoteEndpoints, planOrQuery{query: queryStr}, queryOpts, ts)
 		return qErr
-
 	}); err != nil {
 		return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err}, func() {}
 	}
@@ -648,7 +650,6 @@ func (qapi *QueryAPI) query(r *http.Request) (interface{}, []error, *api.ApiErro
 		var qErr error
 		qry, qErr = qapi.queryCreate.makeInstantQuery(ctx, engineParam, queryable, remoteEndpoints, planOrQuery{query: queryStr}, queryOpts, ts)
 		return qErr
-
 	}); err != nil {
 		return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err}, func() {}
 	}
@@ -827,7 +828,6 @@ func (qapi *QueryAPI) queryRangeExplain(r *http.Request) (interface{}, []error,
 		var qErr error
 		qry, qErr = qapi.queryCreate.makeRangeQuery(ctx, engineParam, queryable, remoteEndpoints, planOrQuery{query: queryStr}, queryOpts, start, end, step)
 		return qErr
-
 	}); err != nil {
 		return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err}, func() {}
 	}
@@ -961,7 +961,6 @@ func (qapi *QueryAPI) queryRange(r *http.Request) (interface{}, []error, *api.Ap
 		var qErr error
 		qry, qErr = qapi.queryCreate.makeRangeQuery(ctx, engineParam, queryable, remoteEndpoints, planOrQuery{query: queryStr}, queryOpts, start, end, step)
 		return qErr
-
 	}); err != nil {
 		return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err}, func() {}
 	}
@@ -1164,7 +1163,6 @@ func (qapi *QueryAPI) series(r *http.Request) (interface{}, []error, *api.ApiErr
 		nil,
 		query.NoopSeriesStatsReporter,
 	).Querier(timestamp.FromTime(start), timestamp.FromTime(end))
-
 	if err != nil {
 		return nil, nil, &api.ApiError{Typ: api.ErrorExec, Err: err}, func() {}
 	}