Provisioning: Add metrics around webhooks (#111453)

This commit is contained in:
Stephanie Hingtgen
2025-09-22 16:53:50 -05:00
committed by GitHub
parent 8b1caccc72
commit 63e1d52663
9 changed files with 196 additions and 18 deletions
@@ -6,6 +6,7 @@ import (
"net/url"
"path"
"strings"
"time"
"github.com/grafana/grafana-app-sdk/logging"
dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1"
@@ -15,6 +16,8 @@ import (
"github.com/grafana/grafana/pkg/infra/slugify"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/resources"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/utils"
"github.com/prometheus/client_golang/prometheus"
)
type changeInfo struct {
@@ -53,13 +56,16 @@ type evaluator struct {
render ScreenshotRenderer
parsers resources.ParserFactory
urlProvider func(namespace string) string
metrics screenshotMetrics
}
func NewEvaluator(render ScreenshotRenderer, parsers resources.ParserFactory, urlProvider func(namespace string) string) Evaluator {
func NewEvaluator(render ScreenshotRenderer, parsers resources.ParserFactory, urlProvider func(namespace string) string, registry prometheus.Registerer) Evaluator {
metrics := registerScreenshotMetrics(registry)
return &evaluator{
render: render,
parsers: parsers,
urlProvider: urlProvider,
metrics: metrics,
}
}
@@ -152,14 +158,14 @@ func (e *evaluator) evaluateFile(ctx context.Context, repo repository.Reader, ba
info.PreviewURL += "?" + query.Encode()
if shouldRender {
if info.GrafanaURL != "" {
info.GrafanaScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, e.render, info.Parsed.Repo, info.GrafanaURL)
info.GrafanaScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, e.render, info.Parsed.Repo, info.GrafanaURL, e.metrics)
if err != nil {
info.Error = err.Error()
}
}
if info.PreviewURL != "" {
info.PreviewScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, e.render, info.Parsed.Repo, info.PreviewURL)
info.PreviewScreenshotURL, err = renderScreenshotFromGrafanaURL(ctx, baseURL, e.render, info.Parsed.Repo, info.PreviewURL, e.metrics)
if err != nil {
info.Error = err.Error()
}
@@ -175,7 +181,13 @@ func renderScreenshotFromGrafanaURL(ctx context.Context,
renderer ScreenshotRenderer,
repo provisioning.ResourceRepositoryInfo,
grafanaURL string,
metrics screenshotMetrics,
) (string, error) {
outcome := utils.ErrorOutcome
duration := time.Now()
defer func() {
metrics.recordScreenshotDuration(outcome, time.Since(duration))
}()
parsed, err := url.Parse(grafanaURL)
if err != nil {
logging.FromContext(ctx).Warn("invalid", "url", grafanaURL, "err", err)
@@ -194,5 +206,6 @@ func renderScreenshotFromGrafanaURL(ctx context.Context,
logger.Warn("invalid base", "url", baseURL, "err", err)
return "", err
}
outcome = utils.SuccessOutcome
return base.JoinPath(snap).String(), nil
}
@@ -7,6 +7,7 @@ import (
"fmt"
"testing"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -759,7 +760,7 @@ func TestCalculateChanges(t *testing.T) {
}
return "http://host/"
})
}, prometheus.NewPedanticRegistry())
pullRequest := provisioning.PullRequestJobOptions{
Ref: "ref",
@@ -890,6 +891,7 @@ func TestRenderScreenshotFromGrafanaURL(t *testing.T) {
},
}
metrics := registerScreenshotMetrics(prometheus.NewPedanticRegistry())
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
renderer := NewMockScreenshotRenderer(t)
@@ -900,7 +902,7 @@ func TestRenderScreenshotFromGrafanaURL(t *testing.T) {
Name: "repo",
}
got, err := renderScreenshotFromGrafanaURL(context.Background(), tt.baseURL, renderer, repo, tt.grafanaURL)
got, err := renderScreenshotFromGrafanaURL(context.Background(), tt.baseURL, renderer, repo, tt.grafanaURL, metrics)
if tt.wantErr != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tt.wantErr)
@@ -0,0 +1,97 @@
package pullrequest
import (
"sync"
"time"
"github.com/prometheus/client_golang/prometheus"
)
var (
pullRequestMetricsOnce sync.Once
pullRequestMetricsInstance pullRequestMetrics
)
type pullRequestMetrics struct {
registry prometheus.Registerer
processingDuration *prometheus.HistogramVec
commentsPosted *prometheus.CounterVec
}
func registerPullRequestMetrics(registry prometheus.Registerer) pullRequestMetrics {
// called by both ProvidePullRequestWorker and ProvideWebhooksWithImages
// this ensures we only register the metric once
pullRequestMetricsOnce.Do(func() {
processingDuration := prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Name: "grafana_provisioning_pullrequest_processing_duration_seconds",
Help: "Duration of pull request processing",
Buckets: []float64{0.5, 1.0, 2.0, 5.0, 10.0, 30.0},
},
[]string{"outcome"},
)
registry.MustRegister(processingDuration)
commentsPosted := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "grafana_provisioning_pullrequest_comments_posted_total",
Help: "Total number of comments posted to pull requests",
},
[]string{"outcome"},
)
registry.MustRegister(commentsPosted)
pullRequestMetricsInstance = pullRequestMetrics{
registry: registry,
processingDuration: processingDuration,
commentsPosted: commentsPosted,
}
})
return pullRequestMetricsInstance
}
func (m *pullRequestMetrics) recordProcessed(outcome string, duration time.Duration) {
m.processingDuration.WithLabelValues(outcome).Observe(duration.Seconds())
}
func (m *pullRequestMetrics) recordCommentPosted(outcome string) {
m.commentsPosted.WithLabelValues(outcome).Inc()
}
type screenshotMetrics struct {
registry prometheus.Registerer
screenshotDuration *prometheus.HistogramVec
}
var (
screenshotMetricsOnce sync.Once
screenshotMetricsInstance screenshotMetrics
)
func registerScreenshotMetrics(registry prometheus.Registerer) screenshotMetrics {
// called by both ProvidePullRequestWorker and ProvideWebhooksWithImages
// this ensures we only register the metric once
screenshotMetricsOnce.Do(func() {
screenshotDuration := prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Name: "grafana_provisioning_pullrequest_screenshot_duration_seconds",
Help: "Duration of screenshot generation",
Buckets: []float64{1.0, 2.0, 5.0, 10.0, 30.0, 60.0},
},
[]string{"outcome"},
)
registry.MustRegister(screenshotDuration)
screenshotMetricsInstance = screenshotMetrics{
registry: registry,
screenshotDuration: screenshotDuration,
}
})
return screenshotMetricsInstance
}
func (m *screenshotMetrics) recordScreenshotDuration(outcome string, duration time.Duration) {
m.screenshotDuration.WithLabelValues(outcome).Observe(duration.Seconds())
}
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"time"
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -12,10 +13,12 @@ import (
"github.com/grafana/grafana/apps/provisioning/pkg/repository"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/resources"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/utils"
"github.com/grafana/grafana/pkg/services/apiserver"
"github.com/grafana/grafana/pkg/services/rendering"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/storage/unified/resource"
"github.com/prometheus/client_golang/prometheus"
)
func ProvidePullRequestWorker(
@@ -23,6 +26,7 @@ func ProvidePullRequestWorker(
renderer rendering.Service,
blobstore resource.ResourceClient,
configProvider apiserver.RestConfigProvider,
registry prometheus.Registerer,
) *PullRequestWorker {
urlProvider := func(_ string) string {
return cfg.AppURL
@@ -33,10 +37,10 @@ func ProvidePullRequestWorker(
clients := resources.NewClientFactory(configProvider)
parsers := resources.NewParserFactory(clients)
screenshotRenderer := NewScreenshotRenderer(renderer, blobstore)
evaluator := NewEvaluator(screenshotRenderer, parsers, urlProvider)
evaluator := NewEvaluator(screenshotRenderer, parsers, urlProvider, registry)
commenter := NewCommenter()
return NewPullRequestWorker(evaluator, commenter)
return NewPullRequestWorker(evaluator, commenter, registry)
}
//go:generate mockery --name=PullRequestRepo --structname=MockPullRequestRepo --inpackage --filename=mock_pullrequest_repo.go --with-expecter
@@ -60,12 +64,15 @@ type Commenter interface {
type PullRequestWorker struct {
evaluator Evaluator
commenter Commenter
metrics pullRequestMetrics
}
func NewPullRequestWorker(evaluator Evaluator, commenter Commenter) *PullRequestWorker {
func NewPullRequestWorker(evaluator Evaluator, commenter Commenter, registry prometheus.Registerer) *PullRequestWorker {
metrics := registerPullRequestMetrics(registry)
return &PullRequestWorker{
evaluator: evaluator,
commenter: commenter,
metrics: metrics,
}
}
@@ -80,30 +87,42 @@ func (c *PullRequestWorker) Process(ctx context.Context,
) error {
cfg := repo.Config().Spec
opts := job.Spec.PullRequest
startTime := time.Now()
outcome := utils.ErrorOutcome
defer func() {
duration := time.Since(startTime)
c.metrics.recordProcessed(outcome, duration)
}()
if opts == nil {
return apierrors.NewBadRequest("missing spec.pr")
}
logger := logging.FromContext(ctx).With("pr", opts.PR, "repo", repo.Config().GetName(), "namespace", job.GetNamespace())
if opts.Ref == "" {
logger.Debug("missing spec.ref")
return apierrors.NewBadRequest("missing spec.ref")
}
// FIXME: this is leaky because it's supposed to be already a PullRequestRepo
if cfg.GitHub == nil {
logger.Debug("expecting github configuration")
return apierrors.NewBadRequest("expecting github configuration")
}
reader, ok := repo.(repository.Reader)
if !ok {
logger.Debug("pull request job submitted targeting repository that is not a Reader")
return errors.New("pull request job submitted targeting repository that is not a Reader")
}
prRepo, ok := repo.(PullRequestRepo)
if !ok {
logger.Debug("pull request job submitted targeting repository that is not a PullRequestRepo")
return fmt.Errorf("repository is not a pull request repository")
}
logger := logging.FromContext(ctx).With("pr", opts.PR)
logger.Info("process pull request")
defer logger.Info("pull request processed")
@@ -112,6 +131,7 @@ func (c *PullRequestWorker) Process(ctx context.Context,
base := cfg.GitHub.Branch
files, err := prRepo.CompareFiles(ctx, base, opts.Ref)
if err != nil {
logger.Error("failed to list pull request files", "error", err)
return fmt.Errorf("failed to list pull request files: %w", err)
}
@@ -123,12 +143,16 @@ func (c *PullRequestWorker) Process(ctx context.Context,
changeInfo, err := c.evaluator.Evaluate(ctx, reader, *opts, files, progress)
if err != nil {
logger.Error("failed to calculate changes", "error", err)
return fmt.Errorf("calculate changes: %w", err)
}
if err := c.commenter.Comment(ctx, prRepo, opts.PR, changeInfo); err != nil {
c.metrics.recordCommentPosted(utils.ErrorOutcome)
return fmt.Errorf("comment pull request: %w", err)
}
outcome = utils.SuccessOutcome
c.metrics.recordCommentPosted(utils.SuccessOutcome)
logger.Info("preview comment added")
return nil
@@ -5,10 +5,12 @@ import (
"errors"
"testing"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/grafana/grafana-app-sdk/logging"
provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
"github.com/grafana/grafana/apps/provisioning/pkg/repository"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs"
@@ -44,7 +46,7 @@ func TestPullRequestWorker_IsSupported(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
evaluator := NewMockEvaluator(t)
commenter := NewMockCommenter(t)
worker := NewPullRequestWorker(evaluator, commenter)
worker := NewPullRequestWorker(evaluator, commenter, prometheus.NewPedanticRegistry())
result := worker.IsSupported(context.Background(), tt.job)
require.Equal(t, tt.expected, result)
})
@@ -68,7 +70,7 @@ func TestPullRequestWorker_Process_NotPullRequestRepository(t *testing.T) {
},
})
worker := NewPullRequestWorker(evaluator, commenter)
worker := NewPullRequestWorker(evaluator, commenter, prometheus.NewPedanticRegistry())
job := provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPullRequest,
@@ -106,7 +108,7 @@ func TestPullRequestWorker_Process_NotReaderRepository(t *testing.T) {
},
})
worker := NewPullRequestWorker(evaluator, commenter)
worker := NewPullRequestWorker(evaluator, commenter, prometheus.NewPedanticRegistry())
job := provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPullRequest,
@@ -392,7 +394,7 @@ func TestPullRequestWorker_Process(t *testing.T) {
progress := jobs.NewMockJobProgressRecorder(t)
tt.setupMocks(evaluator, commenter, &repo, progress)
worker := NewPullRequestWorker(evaluator, commenter)
worker := NewPullRequestWorker(evaluator, commenter, prometheus.NewPedanticRegistry())
job := provisioning.Job{
Spec: provisioning.JobSpec{
Action: provisioning.JobActionPullRequest,
@@ -400,7 +402,7 @@ func TestPullRequestWorker_Process(t *testing.T) {
},
}
err := worker.Process(context.Background(), repo, job, progress)
err := worker.Process(logging.Context(context.Background(), logging.DefaultLogger), repo, job, progress)
if tt.expectedError != "" {
require.EqualError(t, err, tt.expectedError)
} else {