From 2a2ff482a5ca2d771a5ffc32b02fad46914fa071 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 2 Aug 2024 15:05:13 +0530 Subject: [PATCH 1/3] chore: improve error message readability --- pkg/query-service/app/clickhouseReader/reader.go | 4 ++-- pkg/query-service/app/http_handler.go | 4 ++-- pkg/query-service/app/querier/querier.go | 15 ++++++++++++--- pkg/query-service/app/querier/v2/querier.go | 15 ++++++++++++--- pkg/query-service/interfaces/interface.go | 2 +- pkg/query-service/rules/thresholdRule.go | 2 +- 6 files changed, 30 insertions(+), 12 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 6bfa1839ef..6fb5a4f917 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4585,7 +4585,7 @@ func (r *ClickHouseReader) GetTimeSeriesResultV3(ctx context.Context, query stri if err != nil { zap.L().Error("error while reading time series result", zap.Error(err)) - return nil, err + return nil, errors.New(err.Error()) } defer rows.Close() @@ -4632,7 +4632,7 @@ func (r *ClickHouseReader) GetListResultV3(ctx context.Context, query string) ([ if err != nil { zap.L().Error("error while reading time series result", zap.Error(err)) - return nil, err + return nil, errors.New(err.Error()) } defer rows.Close() diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index 4eff84d50c..ae5b4fb9a5 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -3189,7 +3189,7 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que var result []*v3.Result var err error - var errQuriesByName map[string]error + var errQuriesByName map[string]string var spanKeys map[string]v3.AttributeKey if queryRangeParams.CompositeQuery.QueryType == v3.QueryTypeBuilder { // check if any enrichment is required for logs if yes then enrich them @@ -3474,7 +3474,7 @@ func (aH *APIHandler) queryRangeV4(ctx context.Context, queryRangeParams *v3.Que var result []*v3.Result var err error - var errQuriesByName map[string]error + var errQuriesByName map[string]string var spanKeys map[string]v3.AttributeKey if queryRangeParams.CompositeQuery.QueryType == v3.QueryTypeBuilder { // check if any enrichment is required for logs if yes then enrich them diff --git a/pkg/query-service/app/querier/querier.go b/pkg/query-service/app/querier/querier.go index 95cfc7cc73..b99ae2f43c 100644 --- a/pkg/query-service/app/querier/querier.go +++ b/pkg/query-service/app/querier/querier.go @@ -475,7 +475,7 @@ func (q *querier) runBuilderListQueries(ctx context.Context, params *v3.QueryRan rowList, err := q.reader.GetListResultV3(ctx, query) if err != nil { - ch <- channelResult{Err: fmt.Errorf("error in query-%s: %v", name, err), Name: name, Query: query} + ch <- channelResult{Err: err, Name: name, Query: query} return } ch <- channelResult{List: rowList, Name: name, Query: query} @@ -506,7 +506,11 @@ func (q *querier) runBuilderListQueries(ctx context.Context, params *v3.QueryRan return res, nil, nil } -func (q *querier) QueryRange(ctx context.Context, params *v3.QueryRangeParamsV3, keys map[string]v3.AttributeKey) ([]*v3.Result, map[string]error, error) { +func (q *querier) QueryRange( + ctx context.Context, + params *v3.QueryRangeParamsV3, + keys map[string]v3.AttributeKey, +) ([]*v3.Result, map[string]string, error) { var results []*v3.Result var err error var errQueriesByName map[string]error @@ -543,7 +547,12 @@ func (q *querier) QueryRange(ctx context.Context, params *v3.QueryRangeParamsV3, } } - return results, errQueriesByName, err + queryErrors := make(map[string]string) + for name, err := range errQueriesByName { + queryErrors[fmt.Sprintf("Query-%s", name)] = err.Error() + } + + return results, queryErrors, err } func (q *querier) QueriesExecuted() []string { diff --git a/pkg/query-service/app/querier/v2/querier.go b/pkg/query-service/app/querier/v2/querier.go index b8a8a9e92e..741be10fe4 100644 --- a/pkg/query-service/app/querier/v2/querier.go +++ b/pkg/query-service/app/querier/v2/querier.go @@ -468,7 +468,7 @@ func (q *querier) runBuilderListQueries(ctx context.Context, params *v3.QueryRan rowList, err := q.reader.GetListResultV3(ctx, query) if err != nil { - ch <- channelResult{Err: fmt.Errorf("error in query-%s: %v", name, err), Name: name, Query: query} + ch <- channelResult{Err: err, Name: name, Query: query} return } ch <- channelResult{List: rowList, Name: name, Query: query} @@ -499,7 +499,11 @@ func (q *querier) runBuilderListQueries(ctx context.Context, params *v3.QueryRan return res, nil, nil } -func (q *querier) QueryRange(ctx context.Context, params *v3.QueryRangeParamsV3, keys map[string]v3.AttributeKey) ([]*v3.Result, map[string]error, error) { +func (q *querier) QueryRange( + ctx context.Context, + params *v3.QueryRangeParamsV3, + keys map[string]v3.AttributeKey, +) ([]*v3.Result, map[string]string, error) { var results []*v3.Result var err error var errQueriesByName map[string]error @@ -536,7 +540,12 @@ func (q *querier) QueryRange(ctx context.Context, params *v3.QueryRangeParamsV3, } } - return results, errQueriesByName, err + queryErrors := make(map[string]string) + for name, err := range errQueriesByName { + queryErrors[fmt.Sprintf("Query-%s", name)] = err.Error() + } + + return results, queryErrors, err } func (q *querier) QueriesExecuted() []string { diff --git a/pkg/query-service/interfaces/interface.go b/pkg/query-service/interfaces/interface.go index fea923ac27..835055a8c7 100644 --- a/pkg/query-service/interfaces/interface.go +++ b/pkg/query-service/interfaces/interface.go @@ -108,7 +108,7 @@ type Reader interface { } type Querier interface { - QueryRange(context.Context, *v3.QueryRangeParamsV3, map[string]v3.AttributeKey) ([]*v3.Result, map[string]error, error) + QueryRange(context.Context, *v3.QueryRangeParamsV3, map[string]v3.AttributeKey) ([]*v3.Result, map[string]string, error) // test helpers QueriesExecuted() []string diff --git a/pkg/query-service/rules/thresholdRule.go b/pkg/query-service/rules/thresholdRule.go index e642fb3a0a..2f658c4d26 100644 --- a/pkg/query-service/rules/thresholdRule.go +++ b/pkg/query-service/rules/thresholdRule.go @@ -779,7 +779,7 @@ func (r *ThresholdRule) buildAndRunQuery(ctx context.Context, ts time.Time, ch c } var results []*v3.Result - var errQuriesByName map[string]error + var errQuriesByName map[string]string if r.version == "v4" { results, errQuriesByName, err = r.querierV2.QueryRange(ctx, params, map[string]v3.AttributeKey{}) From 35a60424a2c70790a312db0c44cc176b8967e2ae Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 2 Aug 2024 15:14:44 +0530 Subject: [PATCH 2/3] chore: update error type --- pkg/query-service/app/http_handler.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index ae5b4fb9a5..aafbef9e97 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -3217,7 +3217,7 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que if queryRangeParams.CompositeQuery.QueryType == v3.QueryTypeBuilder { // check if traceID is used as filter (with equal/similar operator) in traces query if yes add timestamp filter to queryRange params isUsed, traceIDs := tracesV3.TraceIdFilterUsedWithEqual(queryRangeParams) - if isUsed == true && len(traceIDs) > 0 { + if isUsed && len(traceIDs) > 0 { zap.L().Debug("traceID used as filter in traces query") // query signoz_spans table with traceID to get min and max timestamp min, max, err := aH.reader.GetMinAndMaxTimestampForTraceID(ctx, traceIDs) @@ -3232,7 +3232,7 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que result, errQuriesByName, err = aH.querier.QueryRange(ctx, queryRangeParams, spanKeys) if err != nil { - apiErrObj := &model.ApiError{Typ: model.ErrorBadData, Err: err} + apiErrObj := &model.ApiError{Typ: model.ErrorInternal, Err: err} RespondError(w, apiErrObj, errQuriesByName) return } @@ -3502,7 +3502,7 @@ func (aH *APIHandler) queryRangeV4(ctx context.Context, queryRangeParams *v3.Que if queryRangeParams.CompositeQuery.QueryType == v3.QueryTypeBuilder { // check if traceID is used as filter (with equal/similar operator) in traces query if yes add timestamp filter to queryRange params isUsed, traceIDs := tracesV3.TraceIdFilterUsedWithEqual(queryRangeParams) - if isUsed == true && len(traceIDs) > 0 { + if isUsed && len(traceIDs) > 0 { zap.L().Debug("traceID used as filter in traces query") // query signoz_spans table with traceID to get min and max timestamp min, max, err := aH.reader.GetMinAndMaxTimestampForTraceID(ctx, traceIDs) @@ -3517,7 +3517,7 @@ func (aH *APIHandler) queryRangeV4(ctx context.Context, queryRangeParams *v3.Que result, errQuriesByName, err = aH.querierV2.QueryRange(ctx, queryRangeParams, spanKeys) if err != nil { - apiErrObj := &model.ApiError{Typ: model.ErrorBadData, Err: err} + apiErrObj := &model.ApiError{Typ: model.ErrorInternal, Err: err} RespondError(w, apiErrObj, errQuriesByName) return } From 9d85054c45f6b70c677e2dadd7954bdfe5e05fc6 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 2 Aug 2024 15:21:29 +0530 Subject: [PATCH 3/3] chore: keep the interface unchanged --- pkg/query-service/app/http_handler.go | 16 ++++++++++++---- pkg/query-service/app/querier/querier.go | 9 ++------- pkg/query-service/app/querier/v2/querier.go | 9 ++------- pkg/query-service/interfaces/interface.go | 2 +- pkg/query-service/rules/thresholdRule.go | 2 +- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index aafbef9e97..c67f691122 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -3189,7 +3189,7 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que var result []*v3.Result var err error - var errQuriesByName map[string]string + var errQuriesByName map[string]error var spanKeys map[string]v3.AttributeKey if queryRangeParams.CompositeQuery.QueryType == v3.QueryTypeBuilder { // check if any enrichment is required for logs if yes then enrich them @@ -3232,8 +3232,12 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que result, errQuriesByName, err = aH.querier.QueryRange(ctx, queryRangeParams, spanKeys) if err != nil { + queryErrors := map[string]string{} + for name, err := range errQuriesByName { + queryErrors[fmt.Sprintf("Query-%s", name)] = err.Error() + } apiErrObj := &model.ApiError{Typ: model.ErrorInternal, Err: err} - RespondError(w, apiErrObj, errQuriesByName) + RespondError(w, apiErrObj, queryErrors) return } @@ -3474,7 +3478,7 @@ func (aH *APIHandler) queryRangeV4(ctx context.Context, queryRangeParams *v3.Que var result []*v3.Result var err error - var errQuriesByName map[string]string + var errQuriesByName map[string]error var spanKeys map[string]v3.AttributeKey if queryRangeParams.CompositeQuery.QueryType == v3.QueryTypeBuilder { // check if any enrichment is required for logs if yes then enrich them @@ -3517,8 +3521,12 @@ func (aH *APIHandler) queryRangeV4(ctx context.Context, queryRangeParams *v3.Que result, errQuriesByName, err = aH.querierV2.QueryRange(ctx, queryRangeParams, spanKeys) if err != nil { + queryErrors := map[string]string{} + for name, err := range errQuriesByName { + queryErrors[fmt.Sprintf("Query-%s", name)] = err.Error() + } apiErrObj := &model.ApiError{Typ: model.ErrorInternal, Err: err} - RespondError(w, apiErrObj, errQuriesByName) + RespondError(w, apiErrObj, queryErrors) return } diff --git a/pkg/query-service/app/querier/querier.go b/pkg/query-service/app/querier/querier.go index b99ae2f43c..81abf8b9a1 100644 --- a/pkg/query-service/app/querier/querier.go +++ b/pkg/query-service/app/querier/querier.go @@ -510,7 +510,7 @@ func (q *querier) QueryRange( ctx context.Context, params *v3.QueryRangeParamsV3, keys map[string]v3.AttributeKey, -) ([]*v3.Result, map[string]string, error) { +) ([]*v3.Result, map[string]error, error) { var results []*v3.Result var err error var errQueriesByName map[string]error @@ -547,12 +547,7 @@ func (q *querier) QueryRange( } } - queryErrors := make(map[string]string) - for name, err := range errQueriesByName { - queryErrors[fmt.Sprintf("Query-%s", name)] = err.Error() - } - - return results, queryErrors, err + return results, errQueriesByName, err } func (q *querier) QueriesExecuted() []string { diff --git a/pkg/query-service/app/querier/v2/querier.go b/pkg/query-service/app/querier/v2/querier.go index 741be10fe4..750d02e211 100644 --- a/pkg/query-service/app/querier/v2/querier.go +++ b/pkg/query-service/app/querier/v2/querier.go @@ -503,7 +503,7 @@ func (q *querier) QueryRange( ctx context.Context, params *v3.QueryRangeParamsV3, keys map[string]v3.AttributeKey, -) ([]*v3.Result, map[string]string, error) { +) ([]*v3.Result, map[string]error, error) { var results []*v3.Result var err error var errQueriesByName map[string]error @@ -540,12 +540,7 @@ func (q *querier) QueryRange( } } - queryErrors := make(map[string]string) - for name, err := range errQueriesByName { - queryErrors[fmt.Sprintf("Query-%s", name)] = err.Error() - } - - return results, queryErrors, err + return results, errQueriesByName, err } func (q *querier) QueriesExecuted() []string { diff --git a/pkg/query-service/interfaces/interface.go b/pkg/query-service/interfaces/interface.go index 835055a8c7..fea923ac27 100644 --- a/pkg/query-service/interfaces/interface.go +++ b/pkg/query-service/interfaces/interface.go @@ -108,7 +108,7 @@ type Reader interface { } type Querier interface { - QueryRange(context.Context, *v3.QueryRangeParamsV3, map[string]v3.AttributeKey) ([]*v3.Result, map[string]string, error) + QueryRange(context.Context, *v3.QueryRangeParamsV3, map[string]v3.AttributeKey) ([]*v3.Result, map[string]error, error) // test helpers QueriesExecuted() []string diff --git a/pkg/query-service/rules/thresholdRule.go b/pkg/query-service/rules/thresholdRule.go index 2f658c4d26..e642fb3a0a 100644 --- a/pkg/query-service/rules/thresholdRule.go +++ b/pkg/query-service/rules/thresholdRule.go @@ -779,7 +779,7 @@ func (r *ThresholdRule) buildAndRunQuery(ctx context.Context, ts time.Time, ch c } var results []*v3.Result - var errQuriesByName map[string]string + var errQuriesByName map[string]error if r.version == "v4" { results, errQuriesByName, err = r.querierV2.QueryRange(ctx, params, map[string]v3.AttributeKey{})