From 4b234c47957ffe97f53565aa7983cb2ba4691f0a Mon Sep 17 00:00:00 2001 From: Sarah Zinger Date: Wed, 27 Nov 2024 14:32:06 -0500 Subject: [PATCH] Add logging to legacy datasource look up path (#97065) --- pkg/registry/apis/query/parser.go | 10 +++++++++- pkg/registry/apis/query/parser_test.go | 3 ++- pkg/registry/apis/query/query_test.go | 2 +- pkg/registry/apis/query/register.go | 2 +- pkg/services/datasources/service/legacy.go | 8 ++++++++ 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/pkg/registry/apis/query/parser.go b/pkg/registry/apis/query/parser.go index dabd8d46566..f36642c33d8 100644 --- a/pkg/registry/apis/query/parser.go +++ b/pkg/registry/apis/query/parser.go @@ -12,6 +12,7 @@ import ( query "github.com/grafana/grafana/pkg/apis/query/v0alpha1" "github.com/grafana/grafana/pkg/expr" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/datasources/service" ) @@ -48,13 +49,15 @@ type queryParser struct { legacy service.LegacyDataSourceLookup reader *expr.ExpressionQueryReader tracer tracing.Tracer + logger log.Logger } -func newQueryParser(reader *expr.ExpressionQueryReader, legacy service.LegacyDataSourceLookup, tracer tracing.Tracer) *queryParser { +func newQueryParser(reader *expr.ExpressionQueryReader, legacy service.LegacyDataSourceLookup, tracer tracing.Tracer, logger log.Logger) *queryParser { return &queryParser{ reader: reader, legacy: legacy, tracer: tracer, + logger: logger, } } @@ -82,6 +85,7 @@ func (p *queryParser) parseRequest(ctx context.Context, input *query.QueryDataRe ds, err := p.getValidDataSourceRef(ctx, q.Datasource, q.DatasourceID) if err != nil { + p.logger.Error("Failed to get valid datasource ref", "error", err) return rsp, err } @@ -93,14 +97,17 @@ func (p *queryParser) parseRequest(ctx context.Context, input *query.QueryDataRe // but this approach lets us focus on well typed behavior first raw, err := json.Marshal(q) if err != nil { + p.logger.Error("Failed to marshal query for expression", "error", err) return rsp, err } iter, err := jsoniter.ParseBytes(jsoniter.ConfigDefault, raw) if err != nil { + p.logger.Error("Failed to parse bytes for expression", "error", err) return rsp, err } exp, err := p.reader.ReadQuery(q, iter) if err != nil { + p.logger.Error("Failed to read query for expression", "error", err) return rsp, NewErrorWithRefID(q.RefID, err) } exp.GraphID = int64(len(expressions) + 1) @@ -170,6 +177,7 @@ func (p *queryParser) parseRequest(ctx context.Context, input *query.QueryDataRe // Add the sorted expressions sortedNodes, err := topo.SortStabilized(dg, nil) if err != nil { + p.logger.Error("Error when sorting nodes", "error", err) return rsp, makeCyclicError("") } for _, v := range sortedNodes { diff --git a/pkg/registry/apis/query/parser_test.go b/pkg/registry/apis/query/parser_test.go index 3476fae74a7..d775cbf55fe 100644 --- a/pkg/registry/apis/query/parser_test.go +++ b/pkg/registry/apis/query/parser_test.go @@ -15,6 +15,7 @@ import ( query "github.com/grafana/grafana/pkg/apis/query/v0alpha1" "github.com/grafana/grafana/pkg/expr" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/featuremgmt" ) @@ -29,7 +30,7 @@ type parserTestObject struct { func TestQuerySplitting(t *testing.T) { ctx := context.Background() parser := newQueryParser(expr.NewExpressionQueryReader(featuremgmt.WithFeatures()), - &legacyDataSourceRetriever{}, tracing.InitializeTracerForTest()) + &legacyDataSourceRetriever{}, tracing.InitializeTracerForTest(), log.NewNopLogger()) t.Run("missing datasource flavors", func(t *testing.T) { split, err := parser.parseRequest(ctx, &query.QueryDataRequest{ diff --git a/pkg/registry/apis/query/query_test.go b/pkg/registry/apis/query/query_test.go index 75e37cb8174..711b9da8ed5 100644 --- a/pkg/registry/apis/query/query_test.go +++ b/pkg/registry/apis/query/query_test.go @@ -26,7 +26,7 @@ func TestQueryRestConnectHandler(t *testing.T) { }, tracer: tracing.InitializeTracerForTest(), parser: newQueryParser(expr.NewExpressionQueryReader(featuremgmt.WithFeatures()), - &legacyDataSourceRetriever{}, tracing.InitializeTracerForTest()), + &legacyDataSourceRetriever{}, tracing.InitializeTracerForTest(), nil), log: log.New("test"), } qr := newQueryREST(b) diff --git a/pkg/registry/apis/query/register.go b/pkg/registry/apis/query/register.go index a08af9909cc..2354bede53a 100644 --- a/pkg/registry/apis/query/register.go +++ b/pkg/registry/apis/query/register.go @@ -75,7 +75,7 @@ func NewQueryAPIBuilder(features featuremgmt.FeatureToggles, log: log.New("query_apiserver"), client: client, registry: registry, - parser: newQueryParser(reader, legacy, tracer), + parser: newQueryParser(reader, legacy, tracer, log.New("query_parser")), metrics: newQueryMetrics(registerer), tracer: tracer, features: features, diff --git a/pkg/services/datasources/service/legacy.go b/pkg/services/datasources/service/legacy.go index b138a906cd6..8705e1e9dc3 100644 --- a/pkg/services/datasources/service/legacy.go +++ b/pkg/services/datasources/service/legacy.go @@ -9,6 +9,7 @@ import ( data "github.com/grafana/grafana-plugin-sdk-go/experimental/apis/data/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/datasources" ) @@ -38,6 +39,7 @@ type cachingLegacyDataSourceLookup struct { retriever DataSourceRetriever cache map[string]cachedValue cacheMu sync.Mutex + log log.Logger } type cachedValue struct { @@ -49,15 +51,18 @@ func ProvideLegacyDataSourceLookup(p *Service) LegacyDataSourceLookup { return &cachingLegacyDataSourceLookup{ retriever: p, cache: make(map[string]cachedValue), + log: log.New("legacy-datasource-lookup"), } } func (s *cachingLegacyDataSourceLookup) GetDataSourceFromDeprecatedFields(ctx context.Context, name string, id int64) (*data.DataSourceRef, error) { if id == 0 && name == "" { + s.log.Error("missing id and name in GetDataSourceFromDeprecatedFields") return nil, fmt.Errorf("either name or ID must be set") } user, err := identity.GetRequester(ctx) if err != nil { + s.log.Error("failed to get user from context after getRequester", "error", err) return nil, err } key := fmt.Sprintf("%d/%s/%d", user.GetOrgID(), name, id) @@ -74,6 +79,9 @@ func (s *cachingLegacyDataSourceLookup) GetDataSourceFromDeprecatedFields(ctx co Name: name, ID: id, }) + if err != nil { + s.log.Error("failed to get datasource from retriever", "error", err) + } if errors.Is(err, datasources.ErrDataSourceNotFound) && name != "" { ds, err = s.retriever.GetDataSource(ctx, &datasources.GetDataSourceQuery{ OrgID: user.GetOrgID(),