Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions pkg/api/rendercsv.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import (
"errors"
"fmt"
"log/slog"
"mime"
"net/http"
"path"
"path/filepath"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -79,7 +82,7 @@ func HandleGetRenderCSV(browser *service.BrowserService) http.Handler {
attribute.String("renderKeyDomain", domain))

start := time.Now()
contents, err := browser.RenderCSV(ctx, url, renderKey, domain, acceptLanguage)
contents, fileName, err := browser.RenderCSV(ctx, url, renderKey, domain, acceptLanguage)
if err != nil {
MetricRenderCSVDuration.WithLabelValues("error").Observe(time.Since(start).Seconds())
span.SetStatus(codes.Error, "csv rendering failed")
Expand All @@ -97,8 +100,24 @@ func HandleGetRenderCSV(browser *service.BrowserService) http.Handler {
MetricRenderCSVDuration.WithLabelValues("success").Observe(time.Since(start).Seconds())
span.SetStatus(codes.Ok, "csv rendered successfully")

requestedFilePath := r.URL.Query().Get("filePath")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different to the fileName? Do we ignore cases where this is a path rather than just the filename?

requestedFilePath = filepath.ToSlash(requestedFilePath)
requestedFilePath = strings.TrimPrefix(requestedFilePath, "/")
requestedFilePath = path.Base(requestedFilePath)
if requestedFilePath == "." || requestedFilePath == "/" || requestedFilePath == "" {
requestedFilePath = fileName
}
if requestedFilePath == "" {
requestedFilePath = "data.csv"
}
if !strings.HasSuffix(strings.ToLower(requestedFilePath), ".csv") {
requestedFilePath += ".csv"
}

w.Header().Set("Content-Type", "text/csv")
w.Header().Set("Content-Disposition", `attachment; filename="data.csv"`)
w.Header().Set("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{
"filename": requestedFilePath,
}))
_, _ = w.Write(contents)
})
}
18 changes: 9 additions & 9 deletions pkg/service/browser.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,19 +279,19 @@ func (s *BrowserService) Render(ctx context.Context, url string, printer Printer
// You may be thinking: what the hell are we doing? Why are we using a browser for this?
// The CSV endpoint just returns HTML. The actual query is done by the browser, and then a script _in the webpage_ downloads it as a CSV file.
// This SHOULD be replaced at some point, such that the Grafana server does all the work; this is just not acceptable behaviour...
func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, acceptLanguage string) ([]byte, error) {
func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, acceptLanguage string) ([]byte, string, error) {
tracer := tracer(ctx)
ctx, span := tracer.Start(ctx, "BrowserService.RenderCSV")
defer span.End()
start := time.Now()

if url == "" {
return nil, fmt.Errorf("url must not be empty")
return nil, "", fmt.Errorf("url must not be empty")
}

allocatorOptions, err := s.createAllocatorOptions(s.cfg)
if err != nil {
return nil, fmt.Errorf("failed to create allocator options: %w", err)
return nil, "", fmt.Errorf("failed to create allocator options: %w", err)
}
allocatorCtx, cancelAllocator := chromedp.NewExecAllocator(ctx, allocatorOptions...)
defer cancelAllocator()
Expand All @@ -301,7 +301,7 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain,

tmpDir, err := os.MkdirTemp("", "gir-csv-*")
if err != nil {
return nil, fmt.Errorf("failed to create temporary directory: %w", err)
return nil, "", fmt.Errorf("failed to create temporary directory: %w", err)
}
defer func() {
if err := os.RemoveAll(tmpDir); err != nil {
Expand Down Expand Up @@ -337,15 +337,15 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain,
MetricBrowserInstancesActive.Inc()
defer MetricBrowserInstancesActive.Dec()
if err := chromedp.Run(browserCtx, actions...); err != nil {
return nil, fmt.Errorf("failed to run browser: %w", err)
return nil, "", fmt.Errorf("failed to run browser: %w", err)
}
span.AddEvent("actions completed")

// Wait for the file to be downloaded.
var entries []os.DirEntry
for {
if err := ctx.Err(); err != nil {
return nil, err
return nil, "", err
}

entries, err = os.ReadDir(tmpDir)
Expand All @@ -355,7 +355,7 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain,

select {
case <-ctx.Done():
return nil, ctx.Err()
return nil, "", ctx.Err()
case <-time.After(100 * time.Millisecond):
// try again
}
Expand All @@ -364,11 +364,11 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain,

fileContents, err := os.ReadFile(filepath.Join(tmpDir, entries[0].Name()))
if err != nil {
return nil, fmt.Errorf("failed to read temporary file: %w", err)
return nil, "", fmt.Errorf("failed to read temporary file: %w", err)
}

MetricBrowserRenderCSVDuration.Observe(time.Since(start).Seconds())
return fileContents, nil
return fileContents, filepath.Base(entries[0].Name()), nil
}

func (s *BrowserService) createAllocatorOptions(cfg config.BrowserConfig) ([]chromedp.ExecAllocatorOption, error) {
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/rendering_grafana_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestRenderingGrafana(t *testing.T) {
contentDisposition := resp.Header.Get("Content-Disposition")
_, params, err := mime.ParseMediaType(contentDisposition)
require.NoError(t, err, "could not parse Content-Disposition header")
require.NotEmpty(t, params["filename"], "no filename in Content-Disposition header")
require.Regexp(t, `^Prometheus_ 1-data-.*\.csv$`, params["filename"])

body := ReadBody(t, resp.Body)
const fixture = "render-prometheus-defaults.csv"
Expand Down
Loading