public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Optimize memory allocations in permissions cache (#89645)

Browse Source 
This PR reduces the number of allocations made while caching permissions from the database, fixes the hierarchy of spans and adds new spans for tracing.

---------

Signed-off-by: Dave Henderson <dave.henderson@grafana.com>
Co-authored-by: Dave Henderson <dave.henderson@grafana.com>
This commit is contained in:
Jeff Levin
2024-06-26 23:03:13 +03:00
committed by GitHub
co-authored by Dave Henderson
parent f894c22b21
commit ed13959e33
5 changed files with 60 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/common_test.go
+2
View File
@@ -274,6 +274,7 @@ func setupSimpleHTTPServer(features featuremgmt.FeatureToggles) *HTTPServer {
authInfoService: &authinfotest.FakeService{
ExpectedLabels: map[int64]string{int64(1): login.GetAuthProviderLabel(login.LDAPAuthModule)},
},
tracer: tracing.InitializeTracerForTest(),
}
}
@@ -299,6 +300,7 @@ func SetupAPITestServer(t *testing.T, opts ...APITestServerOption) *webtest.Serv
Features: featuremgmt.WithFeatures(),
QuotaService: quotatest.New(false, nil),
searchUsersService: &searchusers.OSSService{},
tracer: tracing.InitializeTracerForTest(),
}
for _, opt := range opts {
pkg/api/dashboard.go
+11 -10
View File
@@ -50,7 +50,6 @@ func (hs *HTTPServer) isDashboardStarredByUser(c *contextmodel.ReqContext, dashI
}
userID, err := identity.IntIdentifier(namespaceID, userIDstr)
if err != nil {
return false, err
}
@@ -83,8 +82,11 @@ 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")
defer span.End()
uid := web.Params(c.Req)[":uid"]
dash, rsp := hs.getDashboardHelper(c.Req.Context(), c.SignedInUser.GetOrgID(), 0, uid)
dash, rsp := hs.getDashboardHelper(ctx, c.SignedInUser.GetOrgID(), 0, uid)
if rsp != nil {
return rsp
}
@@ -96,7 +98,7 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response
// If public dashboards is enabled and we have a public dashboard, update meta values
if hs.Features.IsEnabledGlobally(featuremgmt.FlagPublicDashboards) && hs.Cfg.PublicDashboardsEnabled {
publicDashboard, err := hs.PublicDashboardsApi.PublicDashboardService.FindByDashboardUid(c.Req.Context(), c.SignedInUser.GetOrgID(), dash.UID)
publicDashboard, err := hs.PublicDashboardsApi.PublicDashboardService.FindByDashboardUid(ctx, c.SignedInUser.GetOrgID(), dash.UID)
if err != nil && !errors.Is(err, publicdashboardModels.ErrPublicDashboardNotFound) {
return response.Error(http.StatusInternalServerError, "Error while retrieving public dashboards", err)
}
@@ -119,7 +121,7 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response
return response.Error(http.StatusInternalServerError, "Error while loading dashboard, dashboard data is invalid", nil)
}
}
guardian, err := guardian.NewByDashboard(c.Req.Context(), dash, c.SignedInUser.GetOrgID(), c.SignedInUser)
guardian, err := guardian.NewByDashboard(ctx, dash, c.SignedInUser.GetOrgID(), c.SignedInUser)
if err != nil {
return response.Err(err)
}
@@ -139,14 +141,14 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response
// Finding creator and last updater of the dashboard
updater, creator := anonString, anonString
if dash.UpdatedBy > 0 {
updater = hs.getUserLogin(c.Req.Context(), dash.UpdatedBy)
updater = hs.getUserLogin(ctx, dash.UpdatedBy)
}
if dash.CreatedBy > 0 {
creator = hs.getUserLogin(c.Req.Context(), dash.CreatedBy)
creator = hs.getUserLogin(ctx, dash.CreatedBy)
}
annotationPermissions := &dashboardsV0.AnnotationPermission{}
if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) {
if hs.Features.IsEnabled(ctx, featuremgmt.FlagAnnotationPermissionUpdate) {
hs.getAnnotationPermissionsByScope(c, &annotationPermissions.Dashboard, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dash.UID))
} else {
hs.getAnnotationPermissionsByScope(c, &annotationPermissions.Dashboard, accesscontrol.ScopeAnnotationsTypeDashboard)
@@ -182,7 +184,7 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response
// nolint:staticcheck
query := dashboards.GetDashboardQuery{ID: dash.FolderID, OrgID: c.SignedInUser.GetOrgID()}
metrics.MFolderIDsAPICount.WithLabelValues(metrics.GetDashboard).Inc()
queryResult, err := hs.DashboardService.GetDashboard(c.Req.Context(), &query)
queryResult, err := hs.DashboardService.GetDashboard(ctx, &query)
if err != nil {
if errors.Is(err, dashboards.ErrFolderNotFound) {
return response.Error(http.StatusNotFound, "Folder not found", err)
@@ -194,7 +196,7 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response
meta.FolderUrl = queryResult.GetURL()
}
provisioningData, err := hs.dashboardProvisioningService.GetProvisionedDashboardDataByDashboardID(c.Req.Context(), dash.ID)
provisioningData, err := hs.dashboardProvisioningService.GetProvisionedDashboardDataByDashboardID(ctx, dash.ID)
if err != nil {
return response.Error(http.StatusInternalServerError, "Error while checking if dashboard is provisioned", err)
}
@@ -1004,7 +1006,6 @@ func (hs *HTTPServer) CalculateDashboardDiff(c *contextmodel.ReqContext) respons
newData := newVersionRes.Data
result, err := dashdiffs.CalculateDiff(c.Req.Context(), &options, baseData, newData)
if err != nil {
if errors.Is(err, dashver.ErrDashboardVersionNotFound) {
return response.Error(http.StatusNotFound, "Dashboard version not found", err)
pkg/api/dashboard_test.go
+19 -9
View File
@@ -81,6 +81,7 @@ func TestGetHomeDashboard(t *testing.T) {
preferenceService: prefService,
dashboardVersionService: dashboardVersionService,
log: log.New("test-logger"),
tracer: tracing.InitializeTracerForTest(),
}
tests := []struct {
@@ -585,7 +586,8 @@ func TestDashboardAPIEndpoint(t *testing.T) {
DashboardID: 2,
Version: 1,
Data: fakeDash.Data,
}}
},
}
mockSQLStore := dbtest.NewFakeDB()
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
@@ -622,7 +624,8 @@ func TestDashboardAPIEndpoint(t *testing.T) {
DashboardID: 2,
Version: 1,
Data: fakeDash.Data,
}}
},
}
cmd := dtos.RestoreDashboardVersionCommand{
Version: 1,
@@ -680,6 +683,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
DashboardService: dashboardService,
Features: featuremgmt.WithFeatures(),
starService: startest.NewStarServiceFake(),
tracer: tracing.InitializeTracerForTest(),
}
hs.callGetDashboard(sc)
@@ -718,6 +722,7 @@ func TestDashboardVersionsAPIEndpoint(t *testing.T) {
userService: userSvc,
CacheService: localcache.New(5*time.Minute, 10*time.Minute),
log: log.New(),
tracer: tracing.InitializeTracerForTest(),
}
}
@@ -855,6 +860,7 @@ func getDashboardShouldReturn200WithConfig(t *testing.T, sc *scenarioContext, pr
DashboardService: dashboardService,
Features: featuremgmt.WithFeatures(),
starService: startest.NewStarServiceFake(),
tracer: tracing.InitializeTracerForTest(),
}
hs.callGetDashboard(sc)
@@ -908,6 +914,7 @@ func postDashboardScenario(t *testing.T, desc string, url string, routePattern s
Features: featuremgmt.WithFeatures(),
accesscontrolService: actest.FakeService{},
log: log.New("test-logger"),
tracer: tracing.InitializeTracerForTest(),
}
sc := setupScenarioContext(t, url)
@@ -927,7 +934,8 @@ func postDashboardScenario(t *testing.T, desc string, url string, routePattern s
}
func postDiffScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.CalculateDiffOptions,
role org.RoleType, fn scenarioFunc, sqlmock db.DB, fakeDashboardVersionService *dashvertest.FakeDashboardVersionService) {
role org.RoleType, fn scenarioFunc, sqlmock db.DB, fakeDashboardVersionService *dashvertest.FakeDashboardVersionService,
) {
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
cfg := setting.NewCfg()
@@ -943,6 +951,7 @@ func postDiffScenario(t *testing.T, desc string, url string, routePattern string
dashboardVersionService: fakeDashboardVersionService,
Features: featuremgmt.WithFeatures(),
DashboardService: dashSvc,
tracer: tracing.InitializeTracerForTest(),
}
sc := setupScenarioContext(t, url)
@@ -967,7 +976,8 @@ func postDiffScenario(t *testing.T, desc string, url string, routePattern string
func restoreDashboardVersionScenario(t *testing.T, desc string, url string, routePattern string,
mock *dashboards.FakeDashboardService, fakeDashboardVersionService *dashvertest.FakeDashboardVersionService,
cmd dtos.RestoreDashboardVersionCommand, fn scenarioFunc, sqlStore db.DB) {
cmd dtos.RestoreDashboardVersionCommand, fn scenarioFunc, sqlStore db.DB,
) {
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
cfg := setting.NewCfg()
folderSvc := foldertest.NewFakeService()
@@ -986,6 +996,7 @@ func restoreDashboardVersionScenario(t *testing.T, desc string, url string, rout
dashboardVersionService: fakeDashboardVersionService,
accesscontrolService: actest.FakeService{},
folderService: folderSvc,
tracer: tracing.InitializeTracerForTest(),
}
sc := setupScenarioContext(t, url)
@@ -1022,12 +1033,12 @@ type mockDashboardProvisioningService struct {
}
func (s mockDashboardProvisioningService) GetProvisionedDashboardDataByDashboardID(ctx context.Context, dashboardID int64) (
*dashboards.DashboardProvisioning, error) {
*dashboards.DashboardProvisioning, error,
) {
return nil, nil
}
type mockLibraryPanelService struct {
}
type mockLibraryPanelService struct{}
var _ librarypanels.Service = (*mockLibraryPanelService)(nil)
@@ -1039,8 +1050,7 @@ func (m *mockLibraryPanelService) ImportLibraryPanelsForDashboard(c context.Cont
return nil
}
type mockLibraryElementService struct {
}
type mockLibraryElementService struct{}
func (l *mockLibraryElementService) CreateElement(c context.Context, signedInUser identity.Requester, cmd model.CreateLibraryElementCommand) (model.LibraryElementDTO, error) {
return model.LibraryElementDTO{}, nil