From d4916207a0239fa90e3b38e1ae4698f72f0d7329 Mon Sep 17 00:00:00 2001 From: Jeff Levin Date: Mon, 5 Aug 2024 17:17:39 -0800 Subject: [PATCH] chore(tracing): add tracing for frontend and db session (#91509) This PR adds instrumentation for loading frontend SPA along with select methods in the dashboard service, and cleans up span handling in sqlstore. --------- Co-authored-by: Dave Henderson --- pkg/api/dashboard.go | 5 ++++- pkg/api/frontendsettings.go | 15 +++++++++++++++ pkg/api/frontendsettings_test.go | 2 ++ pkg/api/index.go | 6 ++++++ pkg/api/search.go | 3 +++ pkg/api/utils.go | 7 +++++++ pkg/services/search/service.go | 6 ++++++ pkg/services/sqlstore/database_wrapper.go | 1 + pkg/services/sqlstore/session.go | 20 ++++++++++++-------- 9 files changed, 56 insertions(+), 9 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index cb650138f1a..63f5230fdb6 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -82,7 +82,7 @@ func dashboardGuardianResponse(err error) response.Response { // 404: notFoundError // 500: internalServerError func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response { - ctx, span := hs.tracer.Start(c.Req.Context(), "httpserver.GetDashboard") + ctx, span := hs.tracer.Start(c.Req.Context(), "api.GetDashboard") defer span.End() uid := web.Params(c.Req)[":uid"] @@ -262,6 +262,9 @@ func (hs *HTTPServer) getUserLogin(ctx context.Context, userID int64) string { } func (hs *HTTPServer) getDashboardHelper(ctx context.Context, orgID int64, id int64, uid string) (*dashboards.Dashboard, response.Response) { + ctx, span := hs.tracer.Start(ctx, "api.getDashboardHelper") + defer span.End() + var query dashboards.GetDashboardQuery if len(uid) > 0 { diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 148f916c4ac..b57bb201022 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -31,6 +31,9 @@ import ( // Returns a file that is easy to check for changes // Any changes to the file means we should refresh the frontend func (hs *HTTPServer) GetFrontendAssets(c *contextmodel.ReqContext) { + c, span := hs.injectSpan(c, "api.GetFrontendAssets") + defer span.End() + hash := sha256.New() keys := map[string]any{} @@ -97,6 +100,9 @@ func (hs *HTTPServer) GetFrontendSettings(c *contextmodel.ReqContext) { // //nolint:gocyclo func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.FrontendSettingsDTO, error) { + c, span := hs.injectSpan(c, "api.getFrontendSettings") + defer span.End() + availablePlugins, err := hs.availablePlugins(c.Req.Context(), c.SignedInUser.GetOrgID()) if err != nil { return nil, err @@ -388,6 +394,9 @@ func getShortCommitHash(commitHash string, maxLength int) string { } func (hs *HTTPServer) getFSDataSources(c *contextmodel.ReqContext, availablePlugins AvailablePlugins) (map[string]plugins.DataSourceDTO, error) { + c, span := hs.injectSpan(c, "api.getFSDataSources") + defer span.End() + orgDataSources := make([]*datasources.DataSource, 0) if c.SignedInUser.GetOrgID() != 0 { query := datasources.GetDataSourcesQuery{OrgID: c.SignedInUser.GetOrgID(), DataSourceLimit: hs.Cfg.DataSourceLimit} @@ -620,6 +629,9 @@ func (ap AvailablePlugins) Get(pluginType plugins.Type, pluginID string) (*avail } func (hs *HTTPServer) availablePlugins(ctx context.Context, orgID int64) (AvailablePlugins, error) { + ctx, span := hs.tracer.Start(ctx, "api.availablePlugins") + defer span.End() + ap := make(AvailablePlugins) pluginSettingMap, err := hs.pluginSettings(ctx, orgID) @@ -665,6 +677,9 @@ func (hs *HTTPServer) availablePlugins(ctx context.Context, orgID int64) (Availa } func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[string]*pluginsettings.InfoDTO, error) { + ctx, span := hs.tracer.Start(ctx, "api.pluginSettings") + defer span.End() + pluginSettings := make(map[string]*pluginsettings.InfoDTO) // fill settings from database diff --git a/pkg/api/frontendsettings_test.go b/pkg/api/frontendsettings_test.go index 6f3f9ad5c68..ad5a753d7a7 100644 --- a/pkg/api/frontendsettings_test.go +++ b/pkg/api/frontendsettings_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/remotecache" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/login/social/socialimpl" "github.com/grafana/grafana/pkg/plugins" @@ -81,6 +82,7 @@ func setupTestEnvironment(t *testing.T, cfg *setting.Cfg, features featuremgmt.F namespacer: request.GetNamespaceMapper(cfg), SocialService: socialimpl.ProvideService(cfg, features, &usagestats.UsageStatsMock{}, supportbundlestest.NewFakeBundleService(), remotecache.NewFakeCacheStorage(), nil, &ssosettingstests.MockService{}), managedPluginsService: managedplugins.NewNoop(), + tracer: tracing.InitializeTracerForTest(), } m := web.New() diff --git a/pkg/api/index.go b/pkg/api/index.go index 09bef43890d..e09c0adcaf6 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -23,6 +23,9 @@ import ( ) func (hs *HTTPServer) setIndexViewData(c *contextmodel.ReqContext) (*dtos.IndexViewData, error) { + c, span := hs.injectSpan(c, "api.setIndexViewData") + defer span.End() + settings, err := hs.getFrontendSettings(c) if err != nil { return nil, err @@ -215,6 +218,9 @@ func hashUserIdentifier(identifier string, secret string) string { } func (hs *HTTPServer) Index(c *contextmodel.ReqContext) { + c, span := hs.injectSpan(c, "api.Index") + defer span.End() + data, err := hs.setIndexViewData(c) if err != nil { c.Handle(hs.Cfg, http.StatusInternalServerError, "Failed to get settings", err) diff --git a/pkg/api/search.go b/pkg/api/search.go index 2cd77a7d157..13c76fabf4f 100644 --- a/pkg/api/search.go +++ b/pkg/api/search.go @@ -23,6 +23,9 @@ import ( // 422: unprocessableEntityError // 500: internalServerError func (hs *HTTPServer) Search(c *contextmodel.ReqContext) response.Response { + c, span := hs.injectSpan(c, "api.Search") + defer span.End() + query := c.Query("query") tags := c.QueryStrings("tag") starred := c.Query("starred") diff --git a/pkg/api/utils.go b/pkg/api/utils.go index dae4c2da137..f9c648cc2fa 100644 --- a/pkg/api/utils.go +++ b/pkg/api/utils.go @@ -11,6 +11,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/user" + "go.opentelemetry.io/otel/trace" ) func (hs *HTTPServer) GetRedirectURL(c *contextmodel.ReqContext) string { @@ -63,3 +64,9 @@ func ValidateAndNormalizeEmail(email string) (string, error) { return e.Address, nil } + +func (hs *HTTPServer) injectSpan(c *contextmodel.ReqContext, name string) (*contextmodel.ReqContext, trace.Span) { + ctx, span := hs.tracer.Start(c.Req.Context(), name) + c.Req = c.Req.WithContext(ctx) + return c, span +} diff --git a/pkg/services/search/service.go b/pkg/services/search/service.go index c59ab22e5a5..74c04f525cf 100644 --- a/pkg/services/search/service.go +++ b/pkg/services/search/service.go @@ -12,8 +12,11 @@ import ( "github.com/grafana/grafana/pkg/services/star" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" + "go.opentelemetry.io/otel" ) +var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/search") + func ProvideService(cfg *setting.Cfg, sqlstore db.DB, starService star.Service, dashboardService dashboards.DashboardService) *SearchService { s := &SearchService{ Cfg: cfg, @@ -61,6 +64,9 @@ type SearchService struct { } func (s *SearchService) SearchHandler(ctx context.Context, query *Query) (model.HitList, error) { + ctx, span := tracer.Start(ctx, "search.SearchHandler") + defer span.End() + starredQuery := star.GetUserStarsQuery{ UserID: query.SignedInUser.UserID, } diff --git a/pkg/services/sqlstore/database_wrapper.go b/pkg/services/sqlstore/database_wrapper.go index 1457a5a28e7..b6358e73002 100644 --- a/pkg/services/sqlstore/database_wrapper.go +++ b/pkg/services/sqlstore/database_wrapper.go @@ -119,6 +119,7 @@ func (h *databaseQueryWrapper) instrument(ctx context.Context, status string, qu ctx = log.IncDBCallCounter(ctx) + // timestamp overridden and recorded AFTER query is run _, span := h.tracer.Start(ctx, "database query", trace.WithTimestamp(begin)) defer span.End() diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go index 1b933514815..6abb32484a1 100644 --- a/pkg/services/sqlstore/session.go +++ b/pkg/services/sqlstore/session.go @@ -10,6 +10,7 @@ import ( "github.com/mattn/go-sqlite3" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.opentelemetry.io/otel/trace/noop" "xorm.io/xorm" "github.com/grafana/grafana/pkg/apimachinery/errutil" @@ -47,10 +48,15 @@ func startSessionOrUseExisting(ctx context.Context, engine *xorm.Engine, beginTr ctxLogger := sessionLogger.FromContext(ctx) ctxLogger.Debug("reusing existing session", "transaction", sess.transactionOpen) sess.Session = sess.Session.Context(ctx) - return sess, false, nil, nil + + // This is a noop span to simplify later operations. purposefully not using existing context + _, span := noop.NewTracerProvider().Tracer("integrationtests").Start(ctx, "sqlstore.startSessionOrUseExisting") + + return sess, false, span, nil } tctx, span := tracer.Start(ctx, "open session") + span.SetAttributes(attribute.Bool("transaction", beginTran)) newSess := &DBSession{Session: engine.NewSession(), transactionOpen: beginTran} @@ -103,16 +109,14 @@ func (ss *SQLStore) retryOnLocks(ctx context.Context, callback DBTransactionFunc func (ss *SQLStore) withDbSession(ctx context.Context, engine *xorm.Engine, callback DBTransactionFunc) error { sess, isNew, span, err := startSessionOrUseExisting(ctx, engine, false, ss.tracer) + defer span.End() + if err != nil { - return err + return tracing.Errorf(span, "start session failed: %s", err) } + if isNew { - defer func() { - if span != nil { - span.End() - } - sess.Close() - }() + defer sess.Close() } retry := 0 return retryer.Retry(ss.retryOnLocks(ctx, callback, sess, retry), ss.dbCfg.QueryRetries, time.Millisecond*time.Duration(10), time.Second)