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

Caching: Refactor enterprise query caching middleware to a wire service (#65616)

Browse Source 
* define initial service and add to wire

* update caching service interface

* add skipQueryCache header handler and update metrics query function to use it

* add caching service as a dependency to query service

* working caching impl

* propagate cache status to frontend in response

* beginning of improvements suggested by Lean - separate caching logic from query logic.

* more changes to simplify query function

* Decided to revert renaming of function

* Remove error status from cache request

* add extra documentation

* Move query caching duration metric to query package

* add a little bit of documentation

* wip: convert resource caching

* Change return type of query service QueryData to a QueryDataResponse with Headers

* update codeowners

* change X-Cache value to const

* use resource caching in endpoint handlers

* write resource headers to response even if it's not a cache hit

* fix panic caused by lack of nil check

* update unit test

* remove NONE header - shouldn't show up in OSS

* Convert everything to use the plugin middleware

* revert a few more things

* clean up unused vars

* start reverting resource caching, start to implement in plugin middleware

* revert more, fix typo

* Update caching interfaces - resource caching now has a separate cache method

* continue wiring up new resource caching conventions - still in progress

* add more safety to implementation

* remove some unused objects

* remove some code that I left in by accident

* add some comments, fix codeowners, fix duplicate registration

* fix source of panic in resource middleware

* Update client decorator test to provide an empty response object

* create tests for caching middleware

* fix unit test

* Update pkg/services/caching/service.go

Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com>

* improve error message in error log

* quick docs update

* Remove use of mockery. Update return signature to return an explicit hit/miss bool

* create unit test for empty request context

* rename caching metrics to make it clear they pertain to caching

* Update pkg/services/pluginsintegration/clientmiddleware/caching_middleware.go

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>

* Add clarifying comments to cache skip middleware func

* Add comment pointing to the resource cache update call

* fix unit tests (missing dependency)

* try to fix mystery syntax error

* fix a panic

* Caching: Introduce feature toggle to caching service refactor (#66323)

* introduce new feature toggle

* hide calls to new service behind a feature flag

* remove licensing flag from toggle (misunderstood what it was for)

* fix unit tests

* rerun toggle gen

---------

Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com>
Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
This commit is contained in:
Michael Mandrus
2023-04-12 12:30:33 -04:00
committed by GitHub
co-authored by Arati R. Marcus Efraimsson
parent e78be44e1a
commit 5626461b3c
41 changed files with 639 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/datasources.go
+4 -4
View File
@@ -653,7 +653,7 @@ func (hs *HTTPServer) CallDatasourceResource(c *contextmodel.ReqContext) {
c.JsonApiErr(http.StatusBadRequest, "id is invalid", nil)
return
}
ds, err := hs.DataSourceCache.GetDatasource(c.Req.Context(), datasourceID, c.SignedInUser, c.SkipCache)
ds, err := hs.DataSourceCache.GetDatasource(c.Req.Context(), datasourceID, c.SignedInUser, c.SkipDSCache)
if err != nil {
if errors.Is(err, datasources.ErrDataSourceAccessDenied) {
c.JsonApiErr(403, "Access denied to datasource", err)
@@ -690,7 +690,7 @@ func (hs *HTTPServer) CallDatasourceResourceWithUID(c *contextmodel.ReqContext)
return
}
ds, err := hs.DataSourceCache.GetDatasourceByUID(c.Req.Context(), dsUID, c.SignedInUser, c.SkipCache)
ds, err := hs.DataSourceCache.GetDatasourceByUID(c.Req.Context(), dsUID, c.SignedInUser, c.SkipDSCache)
if err != nil {
if errors.Is(err, datasources.ErrDataSourceAccessDenied) {
c.JsonApiErr(http.StatusForbidden, "Access denied to datasource", err)
@@ -760,7 +760,7 @@ func (hs *HTTPServer) CheckDatasourceHealthWithUID(c *contextmodel.ReqContext) r
return response.Error(http.StatusBadRequest, "UID is invalid", nil)
}
ds, err := hs.DataSourceCache.GetDatasourceByUID(c.Req.Context(), dsUID, c.SignedInUser, c.SkipCache)
ds, err := hs.DataSourceCache.GetDatasourceByUID(c.Req.Context(), dsUID, c.SignedInUser, c.SkipDSCache)
if err != nil {
if errors.Is(err, datasources.ErrDataSourceAccessDenied) {
return response.Error(http.StatusForbidden, "Access denied to datasource", err)
@@ -790,7 +790,7 @@ func (hs *HTTPServer) CheckDatasourceHealth(c *contextmodel.ReqContext) response
return response.Error(http.StatusBadRequest, "id is invalid", nil)
}
ds, err := hs.DataSourceCache.GetDatasource(c.Req.Context(), datasourceID, c.SignedInUser, c.SkipCache)
ds, err := hs.DataSourceCache.GetDatasource(c.Req.Context(), datasourceID, c.SignedInUser, c.SkipDSCache)
if err != nil {
if errors.Is(err, datasources.ErrDataSourceAccessDenied) {
return response.Error(http.StatusForbidden, "Access denied to datasource", err)
pkg/api/http_server.go
+6 -2
View File
@@ -41,6 +41,7 @@ import (
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/caching"
"github.com/grafana/grafana/pkg/services/cleanup"
"github.com/grafana/grafana/pkg/services/contexthandler"
"github.com/grafana/grafana/pkg/services/correlations"
@@ -210,6 +211,7 @@ type HTTPServer struct {
statsService stats.Service
authnService authn.Service
starApi *starApi.API
cachingService caching.CachingService
}
type ServerOptions struct {
@@ -252,7 +254,8 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
accesscontrolService accesscontrol.Service, dashboardThumbsService thumbs.DashboardThumbService, navTreeService navtree.Service,
annotationRepo annotations.Repository, tagService tag.Service, searchv2HTTPService searchV2.SearchHTTPService, oauthTokenService oauthtoken.OAuthTokenService,
statsService stats.Service, authnService authn.Service, pluginsCDNService *pluginscdn.Service,
starApi *starApi.API,
starApi *starApi.API, cachingService caching.CachingService,
) (*HTTPServer, error) {
web.Env = cfg.Env
m := web.New()
@@ -358,6 +361,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
authnService: authnService,
pluginsCDNService: pluginsCDNService,
starApi: starApi,
cachingService: cachingService,
}
if hs.Listener != nil {
hs.log.Debug("Using provided listener")
@@ -630,7 +634,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() {
m.Use(middleware.ValidateHostHeader(hs.Cfg))
}
m.Use(middleware.HandleNoCacheHeader)
m.Use(middleware.HandleNoCacheHeaders)
if hs.Cfg.CSPEnabled || hs.Cfg.CSPReportOnlyEnabled {
m.UseMiddleware(middleware.ContentSecurityPolicy(hs.Cfg, hs.log))
pkg/api/metrics.go
+1 -1
View File
@@ -52,7 +52,7 @@ func (hs *HTTPServer) QueryMetricsV2(c *contextmodel.ReqContext) response.Respon
return response.Error(http.StatusBadRequest, "bad request data", err)
}
resp, err := hs.queryDataService.QueryData(c.Req.Context(), c.SignedInUser, c.SkipCache, reqDTO)
resp, err := hs.queryDataService.QueryData(c.Req.Context(), c.SignedInUser, c.SkipDSCache, reqDTO)
if err != nil {
return hs.handleQueryMetricsError(err)
}
pkg/api/plugin_resource.go
+11 -3
View File
@@ -15,6 +15,7 @@ import (
"github.com/grafana/grafana/pkg/plugins/backendplugin"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/util/proxyutil"
"github.com/grafana/grafana/pkg/web"
)
@@ -129,7 +130,7 @@ func (hs *HTTPServer) makePluginResourceRequest(w http.ResponseWriter, req *http
var flushStreamErr error
go func() {
flushStreamErr = hs.flushStream(stream, w)
flushStreamErr = hs.flushStream(req.Context(), crReq, stream, w)
wg.Done()
}()
@@ -140,9 +141,10 @@ func (hs *HTTPServer) makePluginResourceRequest(w http.ResponseWriter, req *http
return flushStreamErr
}
func (hs *HTTPServer) flushStream(stream callResourceClientResponseStream, w http.ResponseWriter) error {
func (hs *HTTPServer) flushStream(ctx context.Context, req *backend.CallResourceRequest, stream callResourceClientResponseStream, w http.ResponseWriter) error {
processedStreams := 0
ctx, cancel := context.WithCancel(ctx)
defer cancel()
for {
resp, err := stream.Recv()
if errors.Is(err, io.EOF) {
@@ -198,6 +200,12 @@ func (hs *HTTPServer) flushStream(stream callResourceClientResponseStream, w htt
if _, err := w.Write(resp.Body); err != nil {
hs.log.Error("Failed to write resource response", "err", err)
} else if hs.Features.IsEnabled(featuremgmt.FlagUseCachingService) {
// Placing the new service implementation behind a feature flag until it is known to be stable
// The enterprise implementation of this function will use the headers and status of the first response,
// And append the body of any subsequent responses. It waits for the context to be canceled before caching the cumulative result.
hs.cachingService.CacheResourceResponse(ctx, req, resp)
}
if flusher, ok := w.(http.Flusher); ok {
pkg/api/plugin_resource_test.go
+3 -1
View File
@@ -29,6 +29,7 @@ import (
"github.com/grafana/grafana/pkg/plugins/manager/store"
"github.com/grafana/grafana/pkg/plugins/pluginscdn"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/caching"
datasources "github.com/grafana/grafana/pkg/services/datasources/fakes"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/oauthtoken/oauthtokentest"
@@ -77,6 +78,7 @@ func TestCallResource(t *testing.T) {
hs.QuotaService = quotatest.New(false, nil)
hs.pluginStore = ps
hs.pluginClient = pluginClient.ProvideService(reg, pCfg)
hs.cachingService = &caching.OSSCachingService{}
})
t.Run("Test successful response is received for valid request", func(t *testing.T) {
@@ -106,7 +108,7 @@ func TestCallResource(t *testing.T) {
req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error {
return errors.New("something went wrong")
}),
}, pluginsintegration.CreateMiddlewares(cfg, &oauthtokentest.Service{}, tracing.InitializeTracerForTest())...)
}, pluginsintegration.CreateMiddlewares(cfg, &oauthtokentest.Service{}, tracing.InitializeTracerForTest(), &caching.OSSCachingService{}, &featuremgmt.FeatureManager{})...)
require.NoError(t, err)
srv = SetupAPITestServer(t, func(hs *HTTPServer) {
pkg/api/plugins_test.go
+16 -6
View File
@@ -30,7 +30,9 @@ import (
"github.com/grafana/grafana/pkg/plugins/manager/store"
"github.com/grafana/grafana/pkg/plugins/pluginscdn"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/caching"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol"
@@ -371,9 +373,11 @@ func Test_GetPluginAssets(t *testing.T) {
func TestMakePluginResourceRequest(t *testing.T) {
hs := HTTPServer{
Cfg: setting.NewCfg(),
log: log.New(),
pluginClient: &fakePluginClient{},
Cfg: setting.NewCfg(),
log: log.New(),
pluginClient: &fakePluginClient{},
cachingService: &caching.OSSCachingService{},
Features: &featuremgmt.FeatureManager{},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
@@ -399,6 +403,8 @@ func TestMakePluginResourceRequestSetCookieNotPresent(t *testing.T) {
pluginClient: &fakePluginClient{
headers: map[string][]string{"Set-Cookie": {"monster"}},
},
cachingService: &caching.OSSCachingService{},
Features: &featuremgmt.FeatureManager{},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
resp := httptest.NewRecorder()
@@ -433,6 +439,8 @@ func TestMakePluginResourceRequestContentTypeUnique(t *testing.T) {
"x-another": {"hello"},
},
},
cachingService: &caching.OSSCachingService{},
Features: &featuremgmt.FeatureManager{},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
resp := httptest.NewRecorder()
@@ -456,9 +464,11 @@ func TestMakePluginResourceRequestContentTypeEmpty(t *testing.T) {
statusCode: http.StatusNoContent,
}
hs := HTTPServer{
Cfg: setting.NewCfg(),
log: log.New(),
pluginClient: pluginClient,
Cfg: setting.NewCfg(),
log: log.New(),
pluginClient: pluginClient,
cachingService: &caching.OSSCachingService{},
Features: &featuremgmt.FeatureManager{},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
resp := httptest.NewRecorder()