Chore: Add tracing to team service (#86999)

* add tracing to team service

* another test fix

* pass in context for team creation and membership checking
This commit is contained in:
Ieva 2024-04-29 11:32:03 +01:00 committed by GitHub
parent fbaa847a3c
commit cee713e34c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 107 additions and 36 deletions

View File

@ -208,7 +208,7 @@ func setupDB(b testing.TB) benchScenario {
quotaService := quotatest.New(false, nil)
teamSvc, err := teamimpl.ProvideService(db, cfg)
teamSvc, err := teamimpl.ProvideService(db, cfg, tracing.InitializeTracerForTest())
require.NoError(b, err)
orgService, err := orgimpl.ProvideService(db, cfg, quotaService)
require.NoError(b, err)

View File

@ -12,6 +12,7 @@ import (
"github.com/urfave/cli/v2"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
@ -636,9 +637,9 @@ func TestIntegrationMergeUser(t *testing.T) {
t.Run("should be able to merge user", func(t *testing.T) {
// Restore after destructive operation
sqlStore := db.InitTestDB(t)
teamSvc, err := teamimpl.ProvideService(sqlStore, setting.NewCfg())
teamSvc, err := teamimpl.ProvideService(sqlStore, setting.NewCfg(), tracing.InitializeTracerForTest())
require.NoError(t, err)
team1, err := teamSvc.CreateTeam("team1 name", "", 1)
team1, err := teamSvc.CreateTeam(context.Background(), "team1 name", "", 1)
require.NoError(t, err)
const testOrgID int64 = 1

View File

@ -11,6 +11,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
rs "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
"github.com/grafana/grafana/pkg/services/auth/identity"
@ -323,7 +324,7 @@ func createUserAndTeam(t *testing.T, store db.DB, userSrv user.Service, teamSvc
})
require.NoError(t, err)
team, err := teamSvc.CreateTeam("team", "", orgID)
team, err := teamSvc.CreateTeam(context.Background(), "team", "", orgID)
require.NoError(t, err)
err = store.WithDbSession(context.Background(), func(sess *db.Session) error {
@ -373,7 +374,7 @@ func createUsersAndTeams(t *testing.T, store db.DB, svcs helperServices, orgID i
continue
}
team, err := svcs.teamSvc.CreateTeam(fmt.Sprintf("team%v", i+1), "", orgID)
team, err := svcs.teamSvc.CreateTeam(context.Background(), fmt.Sprintf("team%v", i+1), "", orgID)
require.NoError(t, err)
err = store.WithDbSession(context.Background(), func(sess *db.Session) error {
@ -399,7 +400,7 @@ func setupTestEnv(t testing.TB) (*AccessControlStore, rs.Store, user.Service, te
acstore := ProvideService(sql)
asService := rs.NewActionSetService()
permissionStore := rs.NewStore(sql, featuremgmt.WithFeatures(), &asService)
teamService, err := teamimpl.ProvideService(sql, cfg)
teamService, err := teamimpl.ProvideService(sql, cfg, tracing.InitializeTracerForTest())
require.NoError(t, err)
orgService, err := orgimpl.ProvideService(sql, cfg, quotatest.New(false, nil))
require.NoError(t, err)

View File

@ -15,6 +15,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
@ -322,7 +323,7 @@ func TestApi_setTeamPermission(t *testing.T) {
server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
// seed team
_, err := teamSvc.CreateTeam("test", "test@test.com", 1)
_, err := teamSvc.CreateTeam(context.Background(), "test", "test@test.com", 1)
require.NoError(t, err)
recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "teams", strconv.Itoa(int(tt.teamID)))
@ -509,9 +510,9 @@ func checkSeededPermissions(t *testing.T, permissions []resourcePermissionDTO) {
func seedPermissions(t *testing.T, resourceID string, sql db.DB, cfg *setting.Cfg, service *Service) {
t.Helper()
// seed team 1 with "Edit" permission on dashboard 1
teamSvc, err := teamimpl.ProvideService(sql, cfg)
teamSvc, err := teamimpl.ProvideService(sql, cfg, tracing.InitializeTracerForTest())
require.NoError(t, err)
team, err := teamSvc.CreateTeam("test", "test@test.com", 1)
team, err := teamSvc.CreateTeam(context.Background(), "test", "test@test.com", 1)
require.NoError(t, err)
_, err = service.SetTeamPermission(context.Background(), team.OrgID, team.ID, resourceID, "Edit")
require.NoError(t, err)

View File

@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
@ -98,7 +99,7 @@ func TestService_SetTeamPermission(t *testing.T) {
})
// seed team
team, err := teamSvc.CreateTeam("test", "test@test.com", 1)
team, err := teamSvc.CreateTeam(context.Background(), "test", "test@test.com", 1)
require.NoError(t, err)
var hookCalled bool
@ -217,7 +218,7 @@ func TestService_SetPermissions(t *testing.T) {
require.NoError(t, err)
_, err = usrSvc.Create(context.Background(), &user.CreateUserCommand{Login: "user", OrgID: 1})
require.NoError(t, err)
_, err = teamSvc.CreateTeam("team", "", 1)
_, err = teamSvc.CreateTeam(context.Background(), "team", "", 1)
require.NoError(t, err)
permissions, err := service.SetPermissions(context.Background(), 1, "1", tt.commands...)
@ -236,7 +237,7 @@ func setupTestEnvironment(t *testing.T, ops Options) (*Service, db.DB, *setting.
sql := db.InitTestDB(t)
cfg := setting.NewCfg()
teamSvc, err := teamimpl.ProvideService(sql, cfg)
teamSvc, err := teamimpl.ProvideService(sql, cfg, tracing.InitializeTracerForTest())
require.NoError(t, err)
userSvc, err := userimpl.ProvideService(sql, nil, cfg, teamSvc, nil, quotatest.New(false, nil), supportbundlestest.NewFakeBundleService())
require.NoError(t, err)

View File

@ -12,6 +12,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/datasources"
datasourcesService "github.com/grafana/grafana/pkg/services/datasources/service"
@ -139,7 +140,7 @@ func GenerateDatasourcePermissions(b *testing.B, db db.DB, cfg *setting.Cfg, ac
}
func generateTeamsAndUsers(b *testing.B, store db.DB, cfg *setting.Cfg, users int) ([]int64, []int64) {
teamSvc, err := teamimpl.ProvideService(store, cfg)
teamSvc, err := teamimpl.ProvideService(store, cfg, tracing.InitializeTracerForTest())
require.NoError(b, err)
numberOfTeams := int(math.Ceil(float64(users) / UsersPerTeam))
globalUserId := 0
@ -154,7 +155,7 @@ func generateTeamsAndUsers(b *testing.B, store db.DB, cfg *setting.Cfg, users in
// Create team
teamName := fmt.Sprintf("%s%v", "team", i)
teamEmail := fmt.Sprintf("%s@example.org", teamName)
team, err := teamSvc.CreateTeam(teamName, teamEmail, 1)
team, err := teamSvc.CreateTeam(context.Background(), teamName, teamEmail, 1)
require.NoError(b, err)
teamId := team.ID
teamIds = append(teamIds, teamId)

View File

@ -5,14 +5,14 @@ import (
)
type Service interface {
CreateTeam(name, email string, orgID int64) (Team, error)
CreateTeam(ctx context.Context, name, email string, orgID int64) (Team, error)
UpdateTeam(ctx context.Context, cmd *UpdateTeamCommand) error
DeleteTeam(ctx context.Context, cmd *DeleteTeamCommand) error
SearchTeams(ctx context.Context, query *SearchTeamsQuery) (SearchTeamQueryResult, error)
GetTeamByID(ctx context.Context, query *GetTeamByIDQuery) (*TeamDTO, error)
GetTeamsByUser(ctx context.Context, query *GetTeamsByUserQuery) ([]*TeamDTO, error)
GetTeamIDsByUser(ctx context.Context, query *GetTeamIDsByUserQuery) ([]int64, error)
IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error)
IsTeamMember(ctx context.Context, orgId int64, teamId int64, userId int64) (bool, error)
RemoveUsersMemberships(tx context.Context, userID int64) error
GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*TeamMemberDTO, error)
GetTeamMembers(ctx context.Context, query *GetTeamMembersQuery) ([]*TeamMemberDTO, error)

View File

@ -34,7 +34,7 @@ func (tapi *TeamAPI) createTeam(c *contextmodel.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
t, err := tapi.teamService.CreateTeam(cmd.Name, cmd.Email, c.SignedInUser.GetOrgID())
t, err := tapi.teamService.CreateTeam(c.Req.Context(), cmd.Name, cmd.Email, c.SignedInUser.GetOrgID())
if err != nil {
if errors.Is(err, team.ErrTeamNameTaken) {
return response.Error(http.StatusConflict, "Team name taken", err)

View File

@ -82,7 +82,7 @@ func (tapi *TeamAPI) addTeamMember(c *contextmodel.ReqContext) response.Response
return response.Error(http.StatusBadRequest, "teamId is invalid", err)
}
isTeamMember, err := tapi.teamService.IsTeamMember(c.SignedInUser.GetOrgID(), teamID, cmd.UserID)
isTeamMember, err := tapi.teamService.IsTeamMember(c.Req.Context(), c.SignedInUser.GetOrgID(), teamID, cmd.UserID)
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to add team member.", err)
}
@ -125,7 +125,7 @@ func (tapi *TeamAPI) updateTeamMember(c *contextmodel.ReqContext) response.Respo
}
orgId := c.SignedInUser.GetOrgID()
isTeamMember, err := tapi.teamService.IsTeamMember(orgId, teamId, userId)
isTeamMember, err := tapi.teamService.IsTeamMember(c.Req.Context(), orgId, teamId, userId)
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to update team member.", err)
}

View File

@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
@ -35,7 +36,7 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
}
t.Run("Testing Team commands and queries", func(t *testing.T) {
sqlStore, cfg := db.InitTestDBWithCfg(t)
teamSvc, err := ProvideService(sqlStore, cfg)
teamSvc, err := ProvideService(sqlStore, cfg, tracing.InitializeTracerForTest())
require.NoError(t, err)
testUser := &user.SignedInUser{
OrgID: 1,
@ -73,9 +74,9 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
require.NoError(t, err)
userIds = append(userIds, usr.ID)
}
team1, err = teamSvc.CreateTeam("group1 name", "test1@test.com", testOrgID)
team1, err = teamSvc.CreateTeam(context.Background(), "group1 name", "test1@test.com", testOrgID)
require.NoError(t, err)
team2, err = teamSvc.CreateTeam("group2 name", "test2@test.com", testOrgID)
team2, err = teamSvc.CreateTeam(context.Background(), "group2 name", "test2@test.com", testOrgID)
require.NoError(t, err)
}
setup()
@ -524,12 +525,12 @@ func TestIntegrationSQLStore_SearchTeams(t *testing.T) {
}
store, cfg := db.InitTestDBWithCfg(t, db.InitTestDBOpt{})
teamSvc, err := ProvideService(store, cfg)
teamSvc, err := ProvideService(store, cfg, tracing.InitializeTracerForTest())
require.NoError(t, err)
// Seed 10 teams
for i := 1; i <= 10; i++ {
_, err := teamSvc.CreateTeam(fmt.Sprintf("team-%d", i), fmt.Sprintf("team-%d@example.org", i), 1)
_, err := teamSvc.CreateTeam(context.Background(), fmt.Sprintf("team-%d", i), fmt.Sprintf("team-%d@example.org", i), 1)
require.NoError(t, err)
}
@ -561,11 +562,11 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
// Seed 2 teams with 2 members
setup := func(store db.DB, cfg *setting.Cfg) {
teamSvc, err := ProvideService(store, cfg)
teamSvc, err := ProvideService(store, cfg, tracing.InitializeTracerForTest())
require.NoError(t, err)
team1, errCreateTeam := teamSvc.CreateTeam("group1 name", "test1@example.org", testOrgID)
team1, errCreateTeam := teamSvc.CreateTeam(context.Background(), "group1 name", "test1@example.org", testOrgID)
require.NoError(t, errCreateTeam)
team2, errCreateTeam := teamSvc.CreateTeam("group2 name", "test2@example.org", testOrgID)
team2, errCreateTeam := teamSvc.CreateTeam(context.Background(), "group2 name", "test2@example.org", testOrgID)
require.NoError(t, errCreateTeam)
quotaService := quotaimpl.ProvideService(store, cfg)
orgSvc, err := orgimpl.ProvideService(store, cfg, quotaService)
@ -604,7 +605,7 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
store, cfg := db.InitTestDBWithCfg(t, db.InitTestDBOpt{})
setup(store, cfg)
teamSvc, err := ProvideService(store, cfg)
teamSvc, err := ProvideService(store, cfg, tracing.InitializeTracerForTest())
require.NoError(t, err)
type getTeamMembersTestCase struct {

View File

@ -3,65 +3,129 @@ package teamimpl
import (
"context"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/setting"
)
type Service struct {
store store
store store
tracer tracing.Tracer
}
func ProvideService(db db.DB, cfg *setting.Cfg) (team.Service, error) {
func ProvideService(db db.DB, cfg *setting.Cfg, tracer tracing.Tracer) (team.Service, error) {
store := &xormStore{db: db, cfg: cfg, deletes: []string{}}
if err := store.uidMigration(); err != nil {
return nil, err
}
return &Service{store: &xormStore{db: db, cfg: cfg, deletes: []string{}}}, nil
return &Service{
store: &xormStore{db: db, cfg: cfg, deletes: []string{}},
tracer: tracer,
}, nil
}
func (s *Service) CreateTeam(name, email string, orgID int64) (team.Team, error) {
func (s *Service) CreateTeam(ctx context.Context, name, email string, orgID int64) (team.Team, error) {
_, span := s.tracer.Start(ctx, "team.CreateTeam", trace.WithAttributes(
attribute.Int64("orgID", orgID),
attribute.String("name", name),
))
defer span.End()
return s.store.Create(name, email, orgID)
}
func (s *Service) UpdateTeam(ctx context.Context, cmd *team.UpdateTeamCommand) error {
ctx, span := s.tracer.Start(ctx, "team.UpdateTeam", trace.WithAttributes(
attribute.Int64("orgID", cmd.OrgID),
attribute.Int64("teamID", cmd.ID),
))
defer span.End()
return s.store.Update(ctx, cmd)
}
func (s *Service) DeleteTeam(ctx context.Context, cmd *team.DeleteTeamCommand) error {
ctx, span := s.tracer.Start(ctx, "team.DeleteTeam", trace.WithAttributes(
attribute.Int64("orgID", cmd.OrgID),
attribute.Int64("teamID", cmd.ID),
))
defer span.End()
return s.store.Delete(ctx, cmd)
}
func (s *Service) SearchTeams(ctx context.Context, query *team.SearchTeamsQuery) (team.SearchTeamQueryResult, error) {
ctx, span := s.tracer.Start(ctx, "team.SearchTeams", trace.WithAttributes(
attribute.Int64("orgID", query.OrgID),
attribute.String("query", query.Query),
))
defer span.End()
return s.store.Search(ctx, query)
}
func (s *Service) GetTeamByID(ctx context.Context, query *team.GetTeamByIDQuery) (*team.TeamDTO, error) {
ctx, span := s.tracer.Start(ctx, "team.GetTeamByID", trace.WithAttributes(
attribute.Int64("orgID", query.OrgID),
attribute.Int64("teamID", query.ID),
))
defer span.End()
return s.store.GetByID(ctx, query)
}
func (s *Service) GetTeamsByUser(ctx context.Context, query *team.GetTeamsByUserQuery) ([]*team.TeamDTO, error) {
ctx, span := s.tracer.Start(ctx, "team.GetTeamsByUser", trace.WithAttributes(
attribute.Int64("orgID", query.OrgID),
attribute.Int64("userID", query.UserID),
))
defer span.End()
return s.store.GetByUser(ctx, query)
}
func (s *Service) GetTeamIDsByUser(ctx context.Context, query *team.GetTeamIDsByUserQuery) ([]int64, error) {
ctx, span := s.tracer.Start(ctx, "team.GetTeamIDsByUser", trace.WithAttributes(
attribute.Int64("orgID", query.OrgID),
attribute.Int64("userID", query.UserID),
))
defer span.End()
return s.store.GetIDsByUser(ctx, query)
}
func (s *Service) IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error) {
func (s *Service) IsTeamMember(ctx context.Context, orgId int64, teamId int64, userId int64) (bool, error) {
_, span := s.tracer.Start(ctx, "team.IsTeamMember", trace.WithAttributes(
attribute.Int64("orgID", orgId),
attribute.Int64("teamID", teamId),
attribute.Int64("userID", userId),
))
defer span.End()
return s.store.IsMember(orgId, teamId, userId)
}
func (s *Service) RemoveUsersMemberships(ctx context.Context, userID int64) error {
ctx, span := s.tracer.Start(ctx, "team.RemoveUsersMemberships", trace.WithAttributes(
attribute.Int64("userID", userID),
))
defer span.End()
return s.store.RemoveUsersMemberships(ctx, userID)
}
func (s *Service) GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*team.TeamMemberDTO, error) {
ctx, span := s.tracer.Start(ctx, "team.GetUserTeamMemberships", trace.WithAttributes(
attribute.Int64("orgID", orgID),
attribute.Int64("userID", userID),
))
defer span.End()
return s.store.GetMemberships(ctx, orgID, userID, external)
}
func (s *Service) GetTeamMembers(ctx context.Context, query *team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) {
ctx, span := s.tracer.Start(ctx, "team.GetTeamMembers", trace.WithAttributes(
attribute.Int64("orgID", query.OrgID),
attribute.Int64("teamID", query.TeamID),
attribute.String("teamUID", query.TeamUID),
))
defer span.End()
return s.store.GetMembers(ctx, query)
}

View File

@ -20,7 +20,7 @@ func NewFakeService() *FakeService {
return &FakeService{}
}
func (s *FakeService) CreateTeam(name, email string, orgID int64) (team.Team, error) {
func (s *FakeService) CreateTeam(ctx context.Context, name, email string, orgID int64) (team.Team, error) {
return s.ExpectedTeam, s.ExpectedError
}
@ -44,7 +44,7 @@ func (s *FakeService) GetTeamsByUser(ctx context.Context, query *team.GetTeamsBy
return s.ExpectedTeamsByUser, s.ExpectedError
}
func (s *FakeService) IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error) {
func (s *FakeService) IsTeamMember(ctx context.Context, orgId int64, teamId int64, userId int64) (bool, error) {
return s.ExpectedIsMember, s.ExpectedError
}

View File

@ -25,6 +25,7 @@ import (
"k8s.io/client-go/rest"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/server"
"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
"github.com/grafana/grafana/pkg/services/auth/identity"
@ -392,7 +393,7 @@ func (c K8sTestHelper) createTestUsers(orgName string) OrgUsers {
c.env.Cfg.AutoAssignOrg = true
c.env.Cfg.AutoAssignOrgId = int(orgId)
teamSvc, err := teamimpl.ProvideService(store, c.env.Cfg)
teamSvc, err := teamimpl.ProvideService(store, c.env.Cfg, tracing.InitializeTracerForTest())
require.NoError(c.t, err)
cache := localcache.ProvideService()