Merge branch 'ds_cache_refactor' of https://github.com/marefr/grafana into marefr-ds_cache_refactor

torkelo · torkelo · commit 07d78da5ecb8 · 2018-10-31T06:06:24.000-07:00
diff --git a/pkg/api/dataproxy.go b/pkg/api/dataproxy.go
@@ -1,62 +1,22 @@
 package api
 
 import (
-	"fmt"
-	"github.com/pkg/errors"
-	"time"
-
 	"github.com/grafana/grafana/pkg/api/pluginproxy"
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/metrics"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
 )
 
-const HeaderNameNoBackendCache = "X-Grafana-NoCache"
-
-func (hs *HTTPServer) getDatasourceFromCache(id int64, c *m.ReqContext) (*m.DataSource, error) {
-	userPermissionsQuery := m.GetDataSourcePermissionsForUserQuery{
-		User: c.SignedInUser,
-	}
-	if err := bus.Dispatch(&userPermissionsQuery); err != nil {
-		if err != bus.ErrHandlerNotFound {
-			return nil, err
-		}
-	} else {
-		permissionType, exists := userPermissionsQuery.Result[id]
-		if exists && permissionType != m.DsPermissionQuery {
-			return nil, errors.New("User not allowed to access datasource")
-		}
-	}
-
-	nocache := c.Req.Header.Get(HeaderNameNoBackendCache) == "true"
-	cacheKey := fmt.Sprintf("ds-%d", id)
-
-	if !nocache {
-		if cached, found := hs.cache.Get(cacheKey); found {
-			ds := cached.(*m.DataSource)
-			if ds.OrgId == c.OrgId {
-				return ds, nil
-			}
-		}
-	}
-
-	query := m.GetDataSourceByIdQuery{Id: id, OrgId: c.OrgId}
-	if err := bus.Dispatch(&query); err != nil {
-		return nil, err
-	}
-
-	hs.cache.Set(cacheKey, query.Result, time.Second*5)
-	return query.Result, nil
-}
-
 func (hs *HTTPServer) ProxyDataSourceRequest(c *m.ReqContext) {
 	c.TimeRequest(metrics.M_DataSource_ProxyReq_Timer)
 
 	dsId := c.ParamsInt64(":id")
-	ds, err := hs.getDatasourceFromCache(dsId, c)
-
+	ds, err := hs.DatasourceCache.GetDatasource(dsId, c.SignedInUser, c.SkipCache)
 	if err != nil {
+		if err == m.ErrDataSourceAccessDenied {
+			c.JsonApiErr(403, "Access denied to datasource", err)
+			return
+		}
 		c.JsonApiErr(500, "Unable to load datasource meta data", err)
 		return
 	}
diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go
@@ -16,7 +16,6 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 
-	gocache "github.com/patrickmn/go-cache"
 	macaron "gopkg.in/macaron.v1"
 
 	"github.com/grafana/grafana/pkg/api/live"
@@ -28,6 +27,8 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/registry"
+	"github.com/grafana/grafana/pkg/services/cache"
+	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/hooks"
 	"github.com/grafana/grafana/pkg/services/rendering"
 	"github.com/grafana/grafana/pkg/setting"
@@ -46,19 +47,19 @@ type HTTPServer struct {
 	macaron       *macaron.Macaron
 	context       context.Context
 	streamManager *live.StreamManager
-	cache         *gocache.Cache
 	httpSrv       *http.Server
 
-	RouteRegister routing.RouteRegister `inject:""`
-	Bus           bus.Bus               `inject:""`
-	RenderService rendering.Service     `inject:""`
-	Cfg           *setting.Cfg          `inject:""`
-	HooksService  *hooks.HooksService   `inject:""`
+	RouteRegister   routing.RouteRegister    `inject:""`
+	Bus             bus.Bus                  `inject:""`
+	RenderService   rendering.Service        `inject:""`
+	Cfg             *setting.Cfg             `inject:""`
+	HooksService    *hooks.HooksService      `inject:""`
+	CacheService    *cache.CacheService      `inject:""`
+	DatasourceCache datasources.CacheService `inject:""`
 }
 
 func (hs *HTTPServer) Init() error {
 	hs.log = log.New("http.server")
-	hs.cache = gocache.New(5*time.Minute, 10*time.Minute)
 
 	hs.streamManager = live.NewStreamManager()
 	hs.macaron = hs.newMacaron()
@@ -231,6 +232,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() {
 		m.Use(middleware.ValidateHostHeader(setting.Domain))
 	}
 
+	m.Use(middleware.HandleNoCacheHeader())
 	m.Use(middleware.AddDefaultResponseHeaders())
 }
 
diff --git a/pkg/api/metrics.go b/pkg/api/metrics.go
@@ -25,8 +25,11 @@ func (hs *HTTPServer) QueryMetrics(c *m.ReqContext, reqDto dtos.MetricRequest) R
 		return Error(400, "Query missing datasourceId", nil)
 	}
 
-	ds, err := hs.getDatasourceFromCache(datasourceId, c)
+	ds, err := hs.DatasourceCache.GetDatasource(datasourceId, c.SignedInUser, c.SkipCache)
 	if err != nil {
+		if err == m.ErrDataSourceAccessDenied {
+			return Error(403, "Access denied to datasource", err)
+		}
 		return Error(500, "Unable to load datasource meta data", err)
 	}
 
diff --git a/pkg/cmd/grafana-server/server.go b/pkg/cmd/grafana-server/server.go
@@ -15,24 +15,29 @@ import (
 	"github.com/grafana/grafana/pkg/api"
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/bus"
-	_ "github.com/grafana/grafana/pkg/extensions"
-	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/login"
-	_ "github.com/grafana/grafana/pkg/metrics"
 	"github.com/grafana/grafana/pkg/middleware"
-	_ "github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/registry"
+	"github.com/grafana/grafana/pkg/social"
+
+	"golang.org/x/sync/errgroup"
+
+	"github.com/grafana/grafana/pkg/log"
+	"github.com/grafana/grafana/pkg/services/cache"
+	"github.com/grafana/grafana/pkg/setting"
+
+	// self registering services
+	_ "github.com/grafana/grafana/pkg/extensions"
+	_ "github.com/grafana/grafana/pkg/metrics"
+	_ "github.com/grafana/grafana/pkg/plugins"
 	_ "github.com/grafana/grafana/pkg/services/alerting"
 	_ "github.com/grafana/grafana/pkg/services/cleanup"
 	_ "github.com/grafana/grafana/pkg/services/notifications"
 	_ "github.com/grafana/grafana/pkg/services/provisioning"
 	_ "github.com/grafana/grafana/pkg/services/rendering"
 	_ "github.com/grafana/grafana/pkg/services/search"
 	_ "github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/grafana/grafana/pkg/setting"
-	"github.com/grafana/grafana/pkg/social" // self registering services
 	_ "github.com/grafana/grafana/pkg/tracing"
-	"golang.org/x/sync/errgroup"
 )
 
 func NewGrafanaServer() *GrafanaServerImpl {
@@ -72,6 +77,7 @@ func (g *GrafanaServerImpl) Run() error {
 	serviceGraph.Provide(&inject.Object{Value: bus.GetBus()})
 	serviceGraph.Provide(&inject.Object{Value: g.cfg})
 	serviceGraph.Provide(&inject.Object{Value: routing.NewRouteRegister(middleware.RequestMetrics, middleware.RequestTracing)})
+	serviceGraph.Provide(&inject.Object{Value: cache.New(5*time.Minute, 10*time.Minute)})
 
 	// self registered services
 	services := registry.GetServices()
@@ -138,7 +144,6 @@ func (g *GrafanaServerImpl) Run() error {
 	}
 
 	sendSystemdNotification("READY=1")
-
 	return g.childRoutines.Wait()
 }
 
diff --git a/pkg/login/auth.go b/pkg/login/auth.go
@@ -2,6 +2,7 @@ package login
 
 import (
 	"errors"
+
 	"github.com/grafana/grafana/pkg/bus"
 	m "github.com/grafana/grafana/pkg/models"
 )
diff --git a/pkg/middleware/headers.go b/pkg/middleware/headers.go
@@ -0,0 +1,14 @@
+package middleware
+
+import (
+	m "github.com/grafana/grafana/pkg/models"
+	macaron "gopkg.in/macaron.v1"
+)
+
+const HeaderNameNoBackendCache = "X-Grafana-NoCache"
+
+func HandleNoCacheHeader() macaron.Handler {
+	return func(ctx *m.ReqContext) {
+		ctx.SkipCache = ctx.Req.Header.Get(HeaderNameNoBackendCache) == "true"
+	}
+}
diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go
@@ -29,6 +29,7 @@ func GetContextHandler() macaron.Handler {
 			Session:        session.GetSession(),
 			IsSignedIn:     false,
 			AllowAnonymous: false,
+			SkipCache:      false,
 			Logger:         log.New("context"),
 		}
 
diff --git a/pkg/models/context.go b/pkg/models/context.go
@@ -20,6 +20,7 @@ type ReqContext struct {
 	IsSignedIn     bool
 	IsRenderCall   bool
 	AllowAnonymous bool
+	SkipCache      bool
 	Logger         log.Logger
 }
 
diff --git a/pkg/models/datasource.go b/pkg/models/datasource.go
@@ -207,11 +207,6 @@ func (p DsPermissionType) String() string {
 	return names[int(p)]
 }
 
-type GetDataSourcePermissionsForUserQuery struct {
-	User   *SignedInUser
-	Result map[int64]DsPermissionType
-}
-
 type DatasourcesPermissionFilterQuery struct {
 	User        *SignedInUser
 	Datasources []*DataSource
diff --git a/pkg/models/user.go b/pkg/models/user.go
@@ -165,6 +165,7 @@ type SignedInUser struct {
 	IsAnonymous    bool
 	HelpFlags1     HelpFlags1
 	LastSeenAt     time.Time
+	Teams          []int64
 }
 
 func (u *SignedInUser) ShouldUpdateLastSeenAt() bool {
diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go
@@ -29,11 +29,42 @@ func Register(descriptor *Descriptor) {
 }
 
 func GetServices() []*Descriptor {
-	sort.Slice(services, func(i, j int) bool {
-		return services[i].InitPriority > services[j].InitPriority
+	slice := getServicesWithOverrides()
+
+	sort.Slice(slice, func(i, j int) bool {
+		return slice[i].InitPriority > slice[j].InitPriority
 	})
 
-	return services
+	return slice
+}
+
+type OverrideServiceFunc func(descriptor Descriptor) (*Descriptor, bool)
+
+var overrides []OverrideServiceFunc
+
+func RegisterOverride(fn OverrideServiceFunc) {
+	overrides = append(overrides, fn)
+}
+
+func getServicesWithOverrides() []*Descriptor {
+	slice := []*Descriptor{}
+	for _, s := range services {
+		var descriptor *Descriptor
+		for _, fn := range overrides {
+			if newDescriptor, override := fn(*s); override {
+				descriptor = newDescriptor
+				break
+			}
+		}
+
+		if descriptor != nil {
+			slice = append(slice, descriptor)
+		} else {
+			slice = append(slice, s)
+		}
+	}
+
+	return slice
 }
 
 // Service interface is the lowest common shape that services
diff --git a/pkg/services/cache/cache.go b/pkg/services/cache/cache.go
@@ -0,0 +1,17 @@
+package cache
+
+import (
+	"time"
+
+	gocache "github.com/patrickmn/go-cache"
+)
+
+type CacheService struct {
+	*gocache.Cache
+}
+
+func New(defaultExpiration, cleanupInterval time.Duration) *CacheService {
+	return &CacheService{
+		Cache: gocache.New(defaultExpiration, cleanupInterval),
+	}
+}
diff --git a/pkg/services/datasources/cache.go b/pkg/services/datasources/cache.go
@@ -0,0 +1,53 @@
+package datasources
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/grafana/grafana/pkg/bus"
+	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/registry"
+	"github.com/grafana/grafana/pkg/services/cache"
+)
+
+type CacheService interface {
+	GetDatasource(datasourceID int64, user *m.SignedInUser, skipCache bool) (*m.DataSource, error)
+}
+
+type CacheServiceImpl struct {
+	Bus          bus.Bus             `inject:""`
+	CacheService *cache.CacheService `inject:""`
+}
+
+func init() {
+	registry.Register(&registry.Descriptor{
+		Name:         "DatasourceCacheService",
+		Instance:     &CacheServiceImpl{},
+		InitPriority: registry.Low,
+	})
+}
+
+func (dc *CacheServiceImpl) Init() error {
+	return nil
+}
+
+func (dc *CacheServiceImpl) GetDatasource(datasourceID int64, user *m.SignedInUser, skipCache bool) (*m.DataSource, error) {
+	cacheKey := fmt.Sprintf("ds-%d", datasourceID)
+
+	if !skipCache {
+		if cached, found := dc.CacheService.Get(cacheKey); found {
+			ds := cached.(*m.DataSource)
+			if ds.OrgId == user.OrgId {
+				return ds, nil
+			}
+		}
+	}
+
+	query := m.GetDataSourceByIdQuery{Id: datasourceID, OrgId: user.OrgId}
+	if err := dc.Bus.Dispatch(&query); err != nil {
+		return nil, err
+	}
+
+	dc.CacheService.Set(cacheKey, query.Result, time.Second*5)
+	return query.Result, nil
+}
diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go
@@ -387,6 +387,17 @@ func GetSignedInUser(query *m.GetSignedInUserQuery) error {
 	if user.OrgRole == "" {
 		user.OrgId = -1
 		user.OrgName = "Org missing"
+	} else {
+		getTeamsByUserQuery := &m.GetTeamsByUserQuery{OrgId: user.OrgId, UserId: user.UserId}
+		err = GetTeamsByUser(getTeamsByUserQuery)
+		if err != nil {
+			return err
+		}
+
+		user.Teams = make([]int64, len(getTeamsByUserQuery.Result))
+		for i, t := range getTeamsByUserQuery.Result {
+			user.Teams[i] = t.Id
+		}
 	}
 
 	query.Result = &user

Original file line number	Diff line number	Diff line change
`@@ -25,8 +25,11 @@ func (hs HTTPServer) QueryMetrics(c m.ReqContext, reqDto dtos.MetricRequest) R`
`25`	`25`	`return Error(400, "Query missing datasourceId", nil)`
`26`	`26`	`}`
`27`	`27`
`28`		`- ds, err := hs.getDatasourceFromCache(datasourceId, c)`
	`28`	`+ ds, err := hs.DatasourceCache.GetDatasource(datasourceId, c.SignedInUser, c.SkipCache)`
`29`	`29`	`if err != nil {`
	`30`	`+ if err == m.ErrDataSourceAccessDenied {`
	`31`	`+ return Error(403, "Access denied to datasource", err)`
	`32`	`+ }`
`30`	`33`	`return Error(500, "Unable to load datasource meta data", err)`
`31`	`34`	`}`
`32`	`35`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@ package login`
`2`	`2`
`3`	`3`	`import (`
`4`	`4`	`"errors"`
	`5`	`+`
`5`	`6`	`"github.com/grafana/grafana/pkg/bus"`
`6`	`7`	`m "github.com/grafana/grafana/pkg/models"`
`7`	`8`	`)`
Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,7 @@ func GetContextHandler() macaron.Handler {`
`29`	`29`	`Session: session.GetSession(),`
`30`	`30`	`IsSignedIn: false,`
`31`	`31`	`AllowAnonymous: false,`
	`32`	`+ SkipCache: false,`
`32`	`33`	`Logger: log.New("context"),`
`33`	`34`	`}`
`34`	`35`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ type ReqContext struct {`
`20`	`20`	`IsSignedIn bool`
`21`	`21`	`IsRenderCall bool`
`22`	`22`	`AllowAnonymous bool`
	`23`	`+ SkipCache bool`
`23`	`24`	`Logger log.Logger`
`24`	`25`	`}`
`25`	`26`
Original file line number	Diff line number	Diff line change
`@@ -165,6 +165,7 @@ type SignedInUser struct {`
`165`	`165`	`IsAnonymous bool`
`166`	`166`	`HelpFlags1 HelpFlags1`
`167`	`167`	`LastSeenAt time.Time`
	`168`	`+ Teams []int64`
`168`	`169`	`}`
`169`	`170`
`170`	`171`	`func (u *SignedInUser) ShouldUpdateLastSeenAt() bool {`