From c9c6445554d4eb3580258131ecd2b51c663fbd9b Mon Sep 17 00:00:00 2001 From: Ezequiel Victorero Date: Tue, 14 May 2024 07:24:18 -0300 Subject: [PATCH] Chore: Refactor render via http (#84613) --- pkg/api/api.go | 2 +- pkg/api/render.go | 34 +++-- pkg/services/rendering/http_mode.go | 167 ++++++++++----------- pkg/services/rendering/interface.go | 30 ++-- pkg/services/rendering/plugin_mode.go | 10 +- pkg/services/rendering/rendering.go | 8 +- pkg/services/rendering/rendering_test.go | 18 +-- pkg/services/screenshot/screenshot.go | 24 +-- pkg/services/screenshot/screenshot_test.go | 24 +-- 9 files changed, 163 insertions(+), 154 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 337f26819d7..0bd1dc6c9a3 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -583,7 +583,7 @@ func (hs *HTTPServer) registerRoutes() { }, reqSignedIn) // rendering - r.Get("/render/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), reqSignedIn, hs.RenderToPng) + r.Get("/render/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), reqSignedIn, hs.RenderHandler) // grafana.net proxy r.Any("/api/gnet/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), reqSignedIn, hs.ProxyGnetRequest) diff --git a/pkg/api/render.go b/pkg/api/render.go index 289d4d988c9..d3e25371641 100644 --- a/pkg/api/render.go +++ b/pkg/api/render.go @@ -15,7 +15,7 @@ import ( "github.com/grafana/grafana/pkg/web" ) -func (hs *HTTPServer) RenderToPng(c *contextmodel.ReqContext) { +func (hs *HTTPServer) RenderHandler(c *contextmodel.ReqContext) { queryReader, err := util.NewURLQueryReader(c.Req.URL) if err != nil { c.Handle(hs.Cfg, http.StatusBadRequest, "Render parameters error", err) @@ -58,23 +58,29 @@ func (hs *HTTPServer) RenderToPng(c *contextmodel.ReqContext) { encoding := queryReader.Get("encoding", "") - result, err := hs.RenderService.Render(c.Req.Context(), rendering.RenderPNG, rendering.Opts{ - TimeoutOpts: rendering.TimeoutOpts{ - Timeout: time.Duration(timeout) * time.Second, - }, - AuthOpts: rendering.AuthOpts{ - OrgID: c.SignedInUser.GetOrgID(), - UserID: userID, - OrgRole: c.SignedInUser.GetOrgRole(), + renderType := rendering.RenderPNG + if encoding == "pdf" { + renderType = rendering.RenderPDF + } + + result, err := hs.RenderService.Render(c.Req.Context(), renderType, rendering.Opts{ + CommonOpts: rendering.CommonOpts{ + TimeoutOpts: rendering.TimeoutOpts{ + Timeout: time.Duration(timeout) * time.Second, + }, + AuthOpts: rendering.AuthOpts{ + OrgID: c.SignedInUser.GetOrgID(), + UserID: userID, + OrgRole: c.SignedInUser.GetOrgRole(), + }, + Path: web.Params(c.Req)["*"] + queryParams, + Timezone: queryReader.Get("tz", ""), + ConcurrentLimit: hs.Cfg.RendererConcurrentRequestLimit, + Headers: headers, }, Width: width, Height: height, - Path: web.Params(c.Req)["*"] + queryParams, - Timezone: queryReader.Get("tz", ""), - Encoding: encoding, - ConcurrentLimit: hs.Cfg.RendererConcurrentRequestLimit, DeviceScaleFactor: scale, - Headers: headers, Theme: models.ThemeDark, }, nil) if err != nil { diff --git a/pkg/services/rendering/http_mode.go b/pkg/services/rendering/http_mode.go index 7140ef17aa0..0900f9fd90f 100644 --- a/pkg/services/rendering/http_mode.go +++ b/pkg/services/rendering/http_mode.go @@ -36,109 +36,115 @@ var ( remoteVersionRefreshInterval = time.Minute * 15 ) +// renderViaHTTP renders PNG or PDF via HTTP func (rs *RenderingService) renderViaHTTP(ctx context.Context, renderType RenderType, renderKey string, opts Opts) (*RenderResult, error) { - if renderType == RenderPDF { - opts.Encoding = "pdf" + imageRendererURL, err := rs.generateImageRendererURL(renderType, opts, renderKey) + if err != nil { + return nil, err } + result, err := rs.doRequestAndWriteToFile(ctx, renderType, imageRendererURL, opts.TimeoutOpts, opts.Headers) + if err != nil { + return nil, err + } + + return &RenderResult{FilePath: result.FilePath}, nil +} + +// renderViaHTTP renders CSV via HTTP +func (rs *RenderingService) renderCSVViaHTTP(ctx context.Context, renderKey string, csvOpts CSVOpts) (*RenderCSVResult, error) { + opts := Opts{CommonOpts: csvOpts.CommonOpts} + + imageRendererURL, err := rs.generateImageRendererURL(RenderCSV, opts, renderKey) + if err != nil { + return nil, err + } + + result, err := rs.doRequestAndWriteToFile(ctx, RenderCSV, imageRendererURL, opts.TimeoutOpts, opts.Headers) + if err != nil { + return nil, err + } + + return &RenderCSVResult{FilePath: result.FilePath, FileName: result.FileName}, nil +} + +func (rs *RenderingService) generateImageRendererURL(renderType RenderType, opts Opts, renderKey string) (*url.URL, error) { + rendererUrl := rs.Cfg.RendererUrl + if renderType == RenderCSV { + rendererUrl += "/csv" + } + + imageRendererURL, err := url.Parse(rendererUrl) + if err != nil { + return nil, err + } + + queryParams := imageRendererURL.Query() + url := rs.getGrafanaCallbackURL(opts.Path) + queryParams.Add("url", url) + queryParams.Add("renderKey", renderKey) + queryParams.Add("domain", rs.domain) + queryParams.Add("timezone", isoTimeOffsetToPosixTz(opts.Timezone)) + queryParams.Add("encoding", string(renderType)) + queryParams.Add("timeout", strconv.Itoa(int(opts.Timeout.Seconds()))) + + if renderType == RenderPNG { + queryParams.Add("width", strconv.Itoa(opts.Width)) + queryParams.Add("height", strconv.Itoa(opts.Height)) + } + + if renderType != RenderCSV { + queryParams.Add("deviceScaleFactor", fmt.Sprintf("%f", opts.DeviceScaleFactor)) + } + + imageRendererURL.RawQuery = queryParams.Encode() + return imageRendererURL, nil +} + +func (rs *RenderingService) doRequestAndWriteToFile(ctx context.Context, renderType RenderType, rendererURL *url.URL, timeoutOpts TimeoutOpts, headers map[string][]string) (*Result, error) { filePath, err := rs.getNewFilePath(renderType) if err != nil { return nil, err } - rendererURL, err := url.Parse(rs.Cfg.RendererUrl) - if err != nil { - return nil, err - } - - queryParams := rendererURL.Query() - url := rs.getURL(opts.Path) - queryParams.Add("url", url) - queryParams.Add("renderKey", renderKey) - queryParams.Add("width", strconv.Itoa(opts.Width)) - queryParams.Add("height", strconv.Itoa(opts.Height)) - queryParams.Add("domain", rs.domain) - queryParams.Add("timezone", isoTimeOffsetToPosixTz(opts.Timezone)) - queryParams.Add("encoding", opts.Encoding) - queryParams.Add("timeout", strconv.Itoa(int(opts.Timeout.Seconds()))) - queryParams.Add("deviceScaleFactor", fmt.Sprintf("%f", opts.DeviceScaleFactor)) - - rendererURL.RawQuery = queryParams.Encode() - // gives service some additional time to timeout and return possible errors. - reqContext, cancel := context.WithTimeout(ctx, getRequestTimeout(opts.TimeoutOpts)) + reqContext, cancel := context.WithTimeout(ctx, getRequestTimeout(timeoutOpts)) defer cancel() - resp, err := rs.doRequest(reqContext, rendererURL, opts.Headers) + resp, err := rs.doRequest(reqContext, rendererURL, headers) if err != nil { return nil, err } - // save response to file defer func() { if err := resp.Body.Close(); err != nil { rs.log.Warn("Failed to close response body", "err", err) } }() - err = rs.readFileResponse(reqContext, resp, filePath, url) - if err != nil { - return nil, err - } - - return &RenderResult{FilePath: filePath}, nil -} - -func (rs *RenderingService) renderCSVViaHTTP(ctx context.Context, renderKey string, opts CSVOpts) (*RenderCSVResult, error) { - filePath, err := rs.getNewFilePath(RenderCSV) - if err != nil { - return nil, err - } - - rendererURL, err := url.Parse(rs.Cfg.RendererUrl + "/csv") - if err != nil { - return nil, err - } - - queryParams := rendererURL.Query() - url := rs.getURL(opts.Path) - queryParams.Add("url", url) - queryParams.Add("renderKey", renderKey) - queryParams.Add("domain", rs.domain) - queryParams.Add("timezone", isoTimeOffsetToPosixTz(opts.Timezone)) - queryParams.Add("encoding", opts.Encoding) - queryParams.Add("timeout", strconv.Itoa(int(opts.Timeout.Seconds()))) - - rendererURL.RawQuery = queryParams.Encode() - - // gives service some additional time to timeout and return possible errors. - reqContext, cancel := context.WithTimeout(ctx, getRequestTimeout(opts.TimeoutOpts)) - defer cancel() - - resp, err := rs.doRequest(reqContext, rendererURL, opts.Headers) - if err != nil { - return nil, err + // if we didn't get a 200 response, something went wrong. + if resp.StatusCode != http.StatusOK { + rs.log.Error("Remote rendering request failed", "error", resp.Status, "url", rendererURL.Query().Get("url")) + return nil, fmt.Errorf("remote rendering request failed, status code: %d, status: %s", resp.StatusCode, + resp.Status) } // save response to file - defer func() { - if err := resp.Body.Close(); err != nil { - rs.log.Warn("Failed to close response body", "err", err) + err = rs.writeResponseToFile(reqContext, resp, filePath) + if err != nil { + return nil, err + } + + var downloadFileName string + if renderType == RenderCSV { + _, params, err := mime.ParseMediaType(resp.Header.Get("Content-Disposition")) + if err != nil { + return nil, err } - }() - - _, params, err := mime.ParseMediaType(resp.Header.Get("Content-Disposition")) - if err != nil { - return nil, err - } - downloadFileName := params["filename"] - - err = rs.readFileResponse(reqContext, resp, filePath, url) - if err != nil { - return nil, err + downloadFileName = params["filename"] } - return &RenderCSVResult{FilePath: filePath, FileName: downloadFileName}, nil + return &Result{FilePath: filePath, FileName: downloadFileName}, nil } func (rs *RenderingService) doRequest(ctx context.Context, u *url.URL, headers map[string][]string) (*http.Response, error) { @@ -171,20 +177,13 @@ func (rs *RenderingService) doRequest(ctx context.Context, u *url.URL, headers m return resp, nil } -func (rs *RenderingService) readFileResponse(ctx context.Context, resp *http.Response, filePath string, url string) error { +func (rs *RenderingService) writeResponseToFile(ctx context.Context, resp *http.Response, filePath string) error { // check for timeout first if errors.Is(ctx.Err(), context.DeadlineExceeded) { rs.log.Info("Rendering timed out") return ErrTimeout } - // if we didn't get a 200 response, something went wrong. - if resp.StatusCode != http.StatusOK { - rs.log.Error("Remote rendering request failed", "error", resp.Status, "url", url) - return fmt.Errorf("remote rendering request failed, status code: %d, status: %s", resp.StatusCode, - resp.Status) - } - //nolint:gosec out, err := os.Create(filePath) if err != nil { diff --git a/pkg/services/rendering/interface.go b/pkg/services/rendering/interface.go index 3fa172487af..25ffddbec9c 100644 --- a/pkg/services/rendering/interface.go +++ b/pkg/services/rendering/interface.go @@ -42,18 +42,25 @@ func getRequestTimeout(opt TimeoutOpts) time.Duration { return opt.Timeout * opt.RequestTimeoutMultiplier } -type Opts struct { +type CommonOpts struct { TimeoutOpts AuthOpts + Path string + Timezone string + ConcurrentLimit int + Headers map[string][]string +} + +type CSVOpts struct { + CommonOpts +} + +type Opts struct { + CommonOpts ErrorOpts Width int Height int - Path string - Encoding string - Timezone string - ConcurrentLimit int DeviceScaleFactor float64 - Headers map[string][]string Theme models.Theme } @@ -75,14 +82,9 @@ type SanitizeSVGResponse struct { Sanitized []byte } -type CSVOpts struct { - TimeoutOpts - AuthOpts - Path string - Encoding string - Timezone string - ConcurrentLimit int - Headers map[string][]string +type Result struct { + FilePath string + FileName string } type RenderResult struct { diff --git a/pkg/services/rendering/plugin_mode.go b/pkg/services/rendering/plugin_mode.go index 3b448551c36..6a69d1c56d0 100644 --- a/pkg/services/rendering/plugin_mode.go +++ b/pkg/services/rendering/plugin_mode.go @@ -9,10 +9,6 @@ import ( ) func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderType RenderType, renderKey string, opts Opts) (*RenderResult, error) { - if renderType == RenderPDF { - opts.Encoding = "pdf" - } - // gives plugin some additional time to timeout and return possible errors. ctx, cancel := context.WithTimeout(ctx, getRequestTimeout(opts.TimeoutOpts)) defer cancel() @@ -31,7 +27,7 @@ func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderType Rend } req := &pluginextensionv2.RenderRequest{ - Url: rs.getURL(opts.Path), + Url: rs.getGrafanaCallbackURL(opts.Path), Width: int32(opts.Width), Height: int32(opts.Height), DeviceScaleFactor: float32(opts.DeviceScaleFactor), @@ -42,7 +38,7 @@ func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderType Rend Domain: rs.domain, Headers: headers, AuthToken: rs.Cfg.RendererAuthToken, - Encoding: opts.Encoding, + Encoding: string(renderType), } rs.log.Debug("Calling renderer plugin", "req", req) @@ -83,7 +79,7 @@ func (rs *RenderingService) renderCSVViaPlugin(ctx context.Context, renderKey st } req := &pluginextensionv2.RenderCSVRequest{ - Url: rs.getURL(opts.Path), + Url: rs.getGrafanaCallbackURL(opts.Path), FilePath: filePath, RenderKey: renderKey, Domain: rs.domain, diff --git a/pkg/services/rendering/rendering.go b/pkg/services/rendering/rendering.go index a99f37b34b9..272de53350b 100644 --- a/pkg/services/rendering/rendering.go +++ b/pkg/services/rendering/rendering.go @@ -254,6 +254,7 @@ func (rs *RenderingService) renderUnavailableImage() *RenderResult { } } +// Render calls the grafana image renderer and returns Grafana resource as PNG or PDF func (rs *RenderingService) Render(ctx context.Context, renderType RenderType, opts Opts, session Session) (*RenderResult, error) { startTime := time.Now() @@ -264,7 +265,7 @@ func (rs *RenderingService) Render(ctx context.Context, renderType RenderType, o result, err := rs.render(ctx, renderType, opts, renderKeyProvider) elapsedTime := time.Since(startTime).Milliseconds() - saveMetrics(elapsedTime, err, RenderPNG) + saveMetrics(elapsedTime, err, renderType) return result, err } @@ -296,7 +297,7 @@ func (rs *RenderingService) render(ctx context.Context, renderType RenderType, o return rs.renderUnavailableImage(), nil } - if renderType == RenderPDF || opts.Encoding == "pdf" { + if renderType == RenderPDF { if !rs.features.IsEnabled(ctx, featuremgmt.FlagNewPDFRendering) { return nil, fmt.Errorf("feature 'newPDFRendering' disabled") } @@ -406,7 +407,8 @@ func (rs *RenderingService) getNewFilePath(rt RenderType) (string, error) { return filepath.Abs(filepath.Join(folder, fmt.Sprintf("%s.%s", rand, ext))) } -func (rs *RenderingService) getURL(path string) string { +// getGrafanaCallbackURL creates a URL to send to the image rendering as callback for rendering a Grafana resource +func (rs *RenderingService) getGrafanaCallbackURL(path string) string { if rs.Cfg.RendererUrl != "" { // The backend rendering service can potentially be remote. // So we need to use the root_url to ensure the rendering service diff --git a/pkg/services/rendering/rendering_test.go b/pkg/services/rendering/rendering_test.go index 36e78f36644..dc860d34600 100644 --- a/pkg/services/rendering/rendering_test.go +++ b/pkg/services/rendering/rendering_test.go @@ -27,7 +27,7 @@ func TestGetUrl(t *testing.T) { t.Run("When renderer and callback url configured should return callback url plus path", func(t *testing.T) { rs.Cfg.RendererUrl = "http://localhost:8081/render" rs.Cfg.RendererCallbackUrl = "http://public-grafana.com/" - url := rs.getURL(path) + url := rs.getGrafanaCallbackURL(path) require.Equal(t, rs.Cfg.RendererCallbackUrl+path+"&render=1", url) }) @@ -40,13 +40,13 @@ func TestGetUrl(t *testing.T) { rs.Cfg.ServeFromSubPath = false rs.Cfg.AppSubURL = "" rs.Cfg.Protocol = setting.HTTPScheme - url := rs.getURL(path) + url := rs.getGrafanaCallbackURL(path) require.Equal(t, "http://localhost:3000/"+path+"&render=1", url) t.Run("And serve from sub path should return expected path", func(t *testing.T) { rs.Cfg.ServeFromSubPath = true rs.Cfg.AppSubURL = "/grafana" - url := rs.getURL(path) + url := rs.getGrafanaCallbackURL(path) require.Equal(t, "http://localhost:3000/grafana/"+path+"&render=1", url) }) }) @@ -55,7 +55,7 @@ func TestGetUrl(t *testing.T) { rs.Cfg.ServeFromSubPath = false rs.Cfg.AppSubURL = "" rs.Cfg.Protocol = setting.HTTPSScheme - url := rs.getURL(path) + url := rs.getGrafanaCallbackURL(path) require.Equal(t, "https://localhost:3000/"+path+"&render=1", url) }) @@ -63,7 +63,7 @@ func TestGetUrl(t *testing.T) { rs.Cfg.ServeFromSubPath = false rs.Cfg.AppSubURL = "" rs.Cfg.Protocol = setting.HTTP2Scheme - url := rs.getURL(path) + url := rs.getGrafanaCallbackURL(path) require.Equal(t, "https://localhost:3000/"+path+"&render=1", url) }) }) @@ -151,7 +151,7 @@ func TestRenderLimitImage(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - opts := Opts{Theme: tc.theme, ConcurrentLimit: 1} + opts := Opts{Theme: tc.theme, CommonOpts: CommonOpts{ConcurrentLimit: 1}} result, err := rs.Render(context.Background(), RenderPNG, opts, nil) assert.NoError(t, err) assert.Equal(t, tc.expected, result.FilePath) @@ -166,9 +166,9 @@ func TestRenderLimitImageError(t *testing.T) { log: log.New("test"), } opts := Opts{ - ErrorOpts: ErrorOpts{ErrorConcurrentLimitReached: true}, - ConcurrentLimit: 1, - Theme: models.ThemeDark, + CommonOpts: CommonOpts{ConcurrentLimit: 1}, + ErrorOpts: ErrorOpts{ErrorConcurrentLimitReached: true}, + Theme: models.ThemeDark, } result, err := rs.Render(context.Background(), RenderPNG, opts, nil) assert.Equal(t, ErrConcurrentLimitReached, err) diff --git a/pkg/services/screenshot/screenshot.go b/pkg/services/screenshot/screenshot.go index 58241961106..9b1012d3ac4 100644 --- a/pkg/services/screenshot/screenshot.go +++ b/pkg/services/screenshot/screenshot.go @@ -108,22 +108,24 @@ func (s *HeadlessScreenshotService) Take(ctx context.Context, opts ScreenshotOpt u.RawQuery = p.Encode() renderOpts := rendering.Opts{ - AuthOpts: rendering.AuthOpts{ - OrgID: dashboard.OrgID, - OrgRole: org.RoleAdmin, + CommonOpts: rendering.CommonOpts{ + AuthOpts: rendering.AuthOpts{ + OrgID: dashboard.OrgID, + OrgRole: org.RoleAdmin, + }, + TimeoutOpts: rendering.TimeoutOpts{ + Timeout: opts.Timeout, + }, + ConcurrentLimit: s.cfg.RendererConcurrentRequestLimit, + Path: u.String(), }, ErrorOpts: rendering.ErrorOpts{ ErrorConcurrentLimitReached: true, ErrorRenderUnavailable: true, }, - TimeoutOpts: rendering.TimeoutOpts{ - Timeout: opts.Timeout, - }, - Width: opts.Width, - Height: opts.Height, - Theme: opts.Theme, - ConcurrentLimit: s.cfg.RendererConcurrentRequestLimit, - Path: u.String(), + Width: opts.Width, + Height: opts.Height, + Theme: opts.Theme, } result, err := s.rs.Render(ctx, rendering.RenderPNG, renderOpts, nil) diff --git a/pkg/services/screenshot/screenshot_test.go b/pkg/services/screenshot/screenshot_test.go index 392c45482e5..77583649883 100644 --- a/pkg/services/screenshot/screenshot_test.go +++ b/pkg/services/screenshot/screenshot_test.go @@ -39,22 +39,24 @@ func TestHeadlessScreenshotService(t *testing.T) { d.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(qResult, nil) renderOpts := rendering.Opts{ - AuthOpts: rendering.AuthOpts{ - OrgID: 2, - OrgRole: org.RoleAdmin, + CommonOpts: rendering.CommonOpts{ + AuthOpts: rendering.AuthOpts{ + OrgID: 2, + OrgRole: org.RoleAdmin, + }, + TimeoutOpts: rendering.TimeoutOpts{ + Timeout: DefaultTimeout, + }, + Path: "d-solo/foo/bar?from=now-6h&orgId=2&panelId=4&to=now-2h", + ConcurrentLimit: cfg.RendererConcurrentRequestLimit, }, ErrorOpts: rendering.ErrorOpts{ ErrorConcurrentLimitReached: true, ErrorRenderUnavailable: true, }, - TimeoutOpts: rendering.TimeoutOpts{ - Timeout: DefaultTimeout, - }, - Width: DefaultWidth, - Height: DefaultHeight, - Theme: DefaultTheme, - Path: "d-solo/foo/bar?from=now-6h&orgId=2&panelId=4&to=now-2h", - ConcurrentLimit: cfg.RendererConcurrentRequestLimit, + Width: DefaultWidth, + Height: DefaultHeight, + Theme: DefaultTheme, } opts.From = "now-6h"