Chore: Refactor render via http (#84613)

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)

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,7 +58,13 @@ func (hs *HTTPServer) RenderToPng(c *contextmodel.ReqContext) {
 
 	encoding := queryReader.Get("encoding", "")
 
-	result, err := hs.RenderService.Render(c.Req.Context(), rendering.RenderPNG, rendering.Opts{
+	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,
 			},
@@ -67,14 +73,14 @@ func (hs *HTTPServer) RenderToPng(c *contextmodel.ReqContext) {
 				UserID:  userID,
 				OrgRole: c.SignedInUser.GetOrgRole(),
 			},
-		Width:             width,
-		Height:            height,
 			Path:            web.Params(c.Req)["*"] + queryParams,
 			Timezone:        queryReader.Get("tz", ""),
-		Encoding:          encoding,
 			ConcurrentLimit: hs.Cfg.RendererConcurrentRequestLimit,
-		DeviceScaleFactor: scale,
 			Headers:         headers,
+		},
+		Width:             width,
+		Height:            height,
+		DeviceScaleFactor: scale,
 		Theme:             models.ThemeDark,
 	}, nil)
 	if err != nil {

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 {
+	imageRendererURL, err := rs.generateImageRendererURL(renderType, opts, renderKey)
-		opts.Encoding = "pdf"
+	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 we didn't get a 200 response, something went wrong.
-	if err != nil {
+	if resp.StatusCode != http.StatusOK {
-		return nil, err
+		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)
-	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
 	}
 
 	// save response to file
-	defer func() {
+	err = rs.writeResponseToFile(reqContext, resp, filePath)
-		if err := resp.Body.Close(); err != nil {
+	if err != nil {
-			rs.log.Warn("Failed to close response body", "err", err)
+		return nil, err
 	}
-	}()
 
+	var downloadFileName string
+	if renderType == RenderCSV {
 		_, params, err := mime.ParseMediaType(resp.Header.Get("Content-Disposition"))
 		if err != nil {
 			return nil, err
 		}
-	downloadFileName := params["filename"]
+		downloadFileName = params["filename"]
-
-	err = rs.readFileResponse(reqContext, resp, filePath, url)
-	if err != nil {
-		return nil, err
 	}
 
-	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 {

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 {
+type Result struct {
-	TimeoutOpts
+	FilePath string
-	AuthOpts
+	FileName string
-	Path            string
-	Encoding        string
-	Timezone        string
-	ConcurrentLimit int
-	Headers         map[string][]string
 }
 
 type RenderResult struct {

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,

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

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,8 +166,8 @@ func TestRenderLimitImageError(t *testing.T) {
 		log:             log.New("test"),
 	}
 	opts := Opts{
+		CommonOpts: CommonOpts{ConcurrentLimit: 1},
 		ErrorOpts:  ErrorOpts{ErrorConcurrentLimitReached: true},
-		ConcurrentLimit: 1,
 		Theme:      models.ThemeDark,
 	}
 	result, err := rs.Render(context.Background(), RenderPNG, opts, nil)

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{
+		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(),
 	}
 
 	result, err := s.rs.Render(ctx, rendering.RenderPNG, renderOpts, nil)

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{
+		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,
 	}
 
 	opts.From = "now-6h"