From 77770984537f151b384e729d98a1132ab5d06688 Mon Sep 17 00:00:00 2001 From: Abhishek Rai Date: Mon, 17 Aug 2026 16:24:24 -0700 Subject: [PATCH] Add configurable label normalization and event filtering. Operators can bound metric cardinality with repo/branch/workflow filters and optional branch classes without changing the default scrape shape. Co-authored-by: Cursor --- README.md | 8 + docs/usage.md | 45 ++++ helm/promgithub/templates/deployment.yaml | 20 ++ helm/promgithub/values.yaml | 12 + src/env.go | 17 ++ src/github.go | 49 +++- src/github_test.go | 2 + src/integration_test.go | 40 +++ src/label_policy.go | 226 ++++++++++++++++ src/label_policy_test.go | 300 ++++++++++++++++++++++ src/main.go | 7 + src/metric_recorder.go | 4 + src/metric_recorder_test.go | 15 ++ src/metrics.go | 8 + src/test_support_test.go | 24 ++ 15 files changed, 769 insertions(+), 8 deletions(-) create mode 100644 src/label_policy.go create mode 100644 src/label_policy_test.go diff --git a/README.md b/README.md index 00ea37d..99d5a27 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,7 @@ It is designed to be simple to deploy and can run either: | `promgithub_event_processing_duration_seconds` | Histogram | `event_type` | Duration of async webhook event processing | | `promgithub_duplicate_deliveries_seen_total` | Counter | `event_type` | Duplicate webhook deliveries observed | | `promgithub_duplicate_deliveries_dropped_total` | Counter | `event_type` | Duplicate webhook deliveries dropped | +| `promgithub_event_filtered_total` | Counter | `event_type`, `reason` | Webhook events dropped by the configured label policy | ## Metric model @@ -41,6 +42,13 @@ The exporter focuses on repository and workflow health signals while avoiding no This keeps the default metric set compact and practical for Prometheus while still preserving the `branch` label for branch-specific workflow and job visibility. +Operators can further bound series growth without a code change: + +- Filter repositories, branches, and workflows with allowlists, denylists, and regular expressions. +- Optionally normalize branch labels into `default`, `release`, and `feature` classes. + +These controls are off by default so existing scrapes keep raw branch names. See [Usage documentation](./docs/usage.md#label-normalization-and-event-filtering) for recommended production settings. + ## Redis-backed multi-instance mode When Redis is configured, `promgithub` uses it for: diff --git a/docs/usage.md b/docs/usage.md index fa5bc73..5e93031 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -23,9 +23,50 @@ The service supports the following environment variables: - `PROMGITHUB_REDIS_DELIVERY_TTL` (optional): TTL for webhook delivery dedupe keys, default `24h`. - `PROMGITHUB_EVENT_WORKERS` (optional): Number of async webhook processing workers, default `4`. - `PROMGITHUB_EVENT_QUEUE_SIZE` (optional): Bounded async webhook queue size, default `256`. +- `PROMGITHUB_REPO_ALLOWLIST` (optional): Comma-separated `owner/repo` list. When set, only these repositories are recorded. +- `PROMGITHUB_REPO_DENYLIST` (optional): Comma-separated `owner/repo` list. Matching repositories are dropped. +- `PROMGITHUB_BRANCH_ALLOW_REGEX` (optional): If set, only events whose branch matches this regular expression are recorded. +- `PROMGITHUB_BRANCH_DENY_REGEX` (optional): If set, events whose branch matches this regular expression are dropped. +- `PROMGITHUB_WORKFLOW_ALLOW_REGEX` (optional): If set, only workflow and job events whose workflow name matches are recorded. Push and pull request events skip this filter. +- `PROMGITHUB_WORKFLOW_DENY_REGEX` (optional): If set, matching workflow and job events are dropped. +- `PROMGITHUB_NORMALIZE_BRANCHES` (optional): When `true`, replace raw branch labels with `default`, `release`, or `feature`. Default `false`. +- `PROMGITHUB_DEFAULT_BRANCHES` (optional): Comma-separated branch names classified as `default` when normalization is enabled. Default `main,master`. +- `PROMGITHUB_RELEASE_BRANCH_REGEX` (optional): Branches matching this expression are classified as `release` when normalization is enabled. Default `^(release/|hotfix/).+`. If Redis is configured, the service stores delivery and run state in Redis. +### Label normalization and event filtering + +Filtered events are still signature-checked, deduplicated, and acknowledged so GitHub does not retry them. They do not update business metrics or run state. Drops are counted on `promgithub_event_filtered_total{event_type,reason}` with `reason` of `repository`, `branch`, or `workflow`. + +Branch filters apply to: + +- workflow and job `head_branch` +- push refs after stripping `refs/heads/` or `refs/tags/` +- pull request `base` refs + +Allowlists and denylists can be combined. A repository must be in the allowlist when one is set, and must not be in the denylist. + +Enabling `PROMGITHUB_NORMALIZE_BRANCHES` changes the `branch` and `base_branch` label values: + +| Class | Default match | +| --- | --- | +| `default` | `main` or `master`, or names in `PROMGITHUB_DEFAULT_BRANCHES` | +| `release` | `release/*` or `hotfix/*`, or `PROMGITHUB_RELEASE_BRANCH_REGEX` | +| `feature` | every other non-empty branch | + +This is a scrape-breaking change for existing dashboards. Enable it on new deployments, or expect old raw-branch series to go stale. + +Recommended production starting point for a multi-repo organization: + +```bash +PROMGITHUB_REPO_ALLOWLIST="acme/api,acme/web,acme/worker" +PROMGITHUB_BRANCH_DENY_REGEX="^(dependabot/|renovate/)" +PROMGITHUB_NORMALIZE_BRANCHES="true" +``` + +Keep `PROMGITHUB_NORMALIZE_BRANCHES=false` when you still need per-branch workflow health. In that case, prefer `PROMGITHUB_BRANCH_ALLOW_REGEX` such as `^(main|release/.+)$` so feature-branch series do not accumulate. + ### Async processing and backpressure Webhook requests are acknowledged after signature validation, duplicate-delivery recording, and enqueueing into the bounded async processor. @@ -160,6 +201,10 @@ promgithub: db: 0 keyPrefix: promgithub deliveryTTL: 24h + labelPolicy: + repoAllowlist: "acme/api,acme/web" + branchDenyRegex: "^(dependabot/|renovate/)" + normalizeBranches: true ``` When `redis.enabled=true`, the chart deploys Redis as a dependency and configures `promgithub` to connect to it automatically. diff --git a/helm/promgithub/templates/deployment.yaml b/helm/promgithub/templates/deployment.yaml index d9e8f00..8d7bb99 100644 --- a/helm/promgithub/templates/deployment.yaml +++ b/helm/promgithub/templates/deployment.yaml @@ -54,6 +54,26 @@ spec: - name: PROMGITHUB_REDIS_DELIVERY_TTL value: "{{ .Values.redisConfig.deliveryTTL | default "24h" }}" {{- end }} + {{- with .Values.labelPolicy }} + - name: PROMGITHUB_REPO_ALLOWLIST + value: "{{ .repoAllowlist }}" + - name: PROMGITHUB_REPO_DENYLIST + value: "{{ .repoDenylist }}" + - name: PROMGITHUB_BRANCH_ALLOW_REGEX + value: {{ .branchAllowRegex | quote }} + - name: PROMGITHUB_BRANCH_DENY_REGEX + value: {{ .branchDenyRegex | quote }} + - name: PROMGITHUB_WORKFLOW_ALLOW_REGEX + value: {{ .workflowAllowRegex | quote }} + - name: PROMGITHUB_WORKFLOW_DENY_REGEX + value: {{ .workflowDenyRegex | quote }} + - name: PROMGITHUB_NORMALIZE_BRANCHES + value: "{{ .normalizeBranches }}" + - name: PROMGITHUB_DEFAULT_BRANCHES + value: "{{ .defaultBranches | default "main,master" }}" + - name: PROMGITHUB_RELEASE_BRANCH_REGEX + value: {{ .releaseBranchRegex | default "^(release/|hotfix/).+" | quote }} + {{- end }} envFrom: - secretRef: name: "{{ include "promgithub.fullname" . }}" diff --git a/helm/promgithub/values.yaml b/helm/promgithub/values.yaml index f69931a..44325ba 100644 --- a/helm/promgithub/values.yaml +++ b/helm/promgithub/values.yaml @@ -46,6 +46,18 @@ redisConfig: keyPrefix: promgithub deliveryTTL: 24h +# Optional controls for metric cardinality. Empty filters keep current behavior. +labelPolicy: + repoAllowlist: "" + repoDenylist: "" + branchAllowRegex: "" + branchDenyRegex: "" + workflowAllowRegex: "" + workflowDenyRegex: "" + normalizeBranches: false + defaultBranches: "main,master" + releaseBranchRegex: "^(release/|hotfix/).+" + # This is for setting up the promgithub service service: # This sets the service type diff --git a/src/env.go b/src/env.go index da89f63..9c586b9 100644 --- a/src/env.go +++ b/src/env.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "strconv" + "strings" "time" ) @@ -43,3 +44,19 @@ func parseEnvDuration(key string, defaultValue time.Duration) (time.Duration, er return parsed, nil } + +func parseEnvBool(key string, defaultValue bool) (bool, error) { + value := strings.TrimSpace(os.Getenv(key)) + if value == "" { + return defaultValue, nil + } + + switch strings.ToLower(value) { + case "1", "true", "yes", "on": + return true, nil + case "0", "false", "no", "off": + return false, nil + default: + return false, fmt.Errorf("parse %s: invalid boolean %q", key, value) + } +} diff --git a/src/github.go b/src/github.go index c7aeba8..b02dd5b 100644 --- a/src/github.go +++ b/src/github.go @@ -221,13 +221,22 @@ func updateWorkflowMetrics(ctx context.Context, body []byte) { return } + labels, ok := applyLabelPolicy(githubEventWorkflowRun, eventLabels{ + Repository: payload.Workflow.Repository.FullName, + Branch: payload.Workflow.Branch, + Workflow: payload.Workflow.Name, + }) + if !ok { + return + } + updateTrackedRunMetrics( ctx, payload.Workflow.RunID, runMetricDetails{ - repository: payload.Workflow.Repository.FullName, - branch: payload.Workflow.Branch, - name: payload.Workflow.Name, + repository: labels.Repository, + branch: labels.Branch, + name: labels.Workflow, status: payload.Workflow.Status, conclusion: payload.Workflow.Conclusion, startedAt: payload.Workflow.CreatedAt, @@ -247,13 +256,22 @@ func updateJobMetrics(ctx context.Context, body []byte) { return } + labels, ok := applyLabelPolicy(githubEventWorkflowJob, eventLabels{ + Repository: payload.Job.Repository.FullName, + Branch: payload.Job.Branch, + Workflow: payload.Job.WorkflowName, + }) + if !ok { + return + } + updateTrackedRunMetrics( ctx, payload.Job.ID, runMetricDetails{ - repository: payload.Job.Repository.FullName, - branch: payload.Job.Branch, - name: payload.Job.WorkflowName, + repository: labels.Repository, + branch: labels.Branch, + name: labels.Workflow, status: payload.Job.Status, conclusion: payload.Job.Conclusion, startedAt: payload.Job.StartedAt, @@ -273,6 +291,13 @@ func updateCommitMetrics(body []byte) { return } + if _, ok := applyLabelPolicy(githubEventPush, eventLabels{ + Repository: payload.Repository.FullName, + Branch: branchFromRef(payload.Ref), + }); !ok { + return + } + for range payload.Commits { defaultMetricRecorder.RecordCommitPushed(payload.Repository.FullName) } @@ -286,9 +311,17 @@ func updatePullRequestMetrics(body []byte) { return } + labels, ok := applyLabelPolicy(githubEventPullRequest, eventLabels{ + Repository: payload.Repository.FullName, + Branch: payload.PullRequest.Base.Ref, + }) + if !ok { + return + } + defaultMetricRecorder.RecordPullRequest( - payload.Repository.FullName, - payload.PullRequest.Base.Ref, + labels.Repository, + labels.Branch, payload.Action, ) } diff --git a/src/github_test.go b/src/github_test.go index ee9b2b9..07b5141 100644 --- a/src/github_test.go +++ b/src/github_test.go @@ -47,6 +47,8 @@ func resetWebhookTestState() { asyncProcessingDurationHistogram.Reset() duplicateDeliveriesSeenCounter.Reset() duplicateDeliveriesDroppedCounter.Reset() + filteredEventsCounter.Reset() + defaultLabelPolicy = labelPolicy{} asyncQueueDepthGauge.Set(0) asyncQueueCapacityGauge.Set(0) asyncWorkerCountGauge.Set(0) diff --git a/src/integration_test.go b/src/integration_test.go index 65d2731..7881025 100644 --- a/src/integration_test.go +++ b/src/integration_test.go @@ -88,6 +88,44 @@ func TestIntegrationWebhookMetrics(t *testing.T) { } } +func TestIntegrationLabelPolicyFiltersDeniedRepository(t *testing.T) { + server := newIntegrationTestServer(t) + defer server.Close() + useLabelPolicy(t, testLabelPolicy(t, func(policy *labelPolicy) { + policy.repoDeny = parseSetList("user/repo") + })) + + body := mustReadFixture(t, "workflow_run.json") + resp := sendWebhookRequest(t, server.URL, githubEventWorkflowRun, body, "delivery-filtered") + assertResponseStatus(t, resp, http.StatusAccepted) + + metrics := waitForMetricsSubstring(t, server.URL, `promgithub_event_filtered_total{event_type="workflow_run",reason="repository"} 1`) + if !strings.Contains(metrics, `promgithub_event_filtered_total{event_type="workflow_run",reason="repository"} 1`) { + t.Fatalf("expected filtered event metric, got:\n%s", metrics) + } + if strings.Contains(metrics, `promgithub_workflow_status{branch="main",conclusion="success",repository="user/repo",workflow_name="CI",workflow_status="completed"} 1`) { + t.Fatalf("denied repository should not record workflow metrics:\n%s", metrics) + } +} + +func TestIntegrationLabelPolicyNormalizesDefaultBranch(t *testing.T) { + server := newIntegrationTestServer(t) + defer server.Close() + useLabelPolicy(t, testLabelPolicy(t, func(policy *labelPolicy) { + policy.normalizeBranches = true + })) + + body := mustReadFixture(t, "workflow_run.json") + resp := sendWebhookRequest(t, server.URL, githubEventWorkflowRun, body, "delivery-normalized") + assertResponseStatus(t, resp, http.StatusAccepted) + + expected := `promgithub_workflow_status{branch="default",conclusion="success",repository="user/repo",workflow_name="CI",workflow_status="completed"} 1` + metrics := waitForMetricsSubstring(t, server.URL, expected) + if !strings.Contains(metrics, expected) { + t.Fatalf("expected normalized branch label, got:\n%s", metrics) + } +} + func TestIntegrationWebhookInvalidSignature(t *testing.T) { server := newIntegrationTestServer(t) defer server.Close() @@ -462,6 +500,8 @@ func resetIntegrationTestMetrics() { asyncProcessingDurationHistogram.Reset() duplicateDeliveriesSeenCounter.Reset() duplicateDeliveriesDroppedCounter.Reset() + filteredEventsCounter.Reset() + defaultLabelPolicy = labelPolicy{} defaultServiceMetrics.apiCallsCounter.Reset() defaultServiceMetrics.requestDurationHistogram.Reset() asyncQueueDepthGauge.Set(0) diff --git a/src/label_policy.go b/src/label_policy.go new file mode 100644 index 0000000..11fa98b --- /dev/null +++ b/src/label_policy.go @@ -0,0 +1,226 @@ +package main + +import ( + "fmt" + "os" + "regexp" + "strings" + + "go.uber.org/zap" +) + +const ( + filterReasonRepository = "repository" + filterReasonBranch = "branch" + filterReasonWorkflow = "workflow" + + branchClassDefault = "default" + branchClassRelease = "release" + branchClassFeature = "feature" + + envRepoAllowlist = "PROMGITHUB_REPO_ALLOWLIST" + envRepoDenylist = "PROMGITHUB_REPO_DENYLIST" + envBranchAllowRegex = "PROMGITHUB_BRANCH_ALLOW_REGEX" + envBranchDenyRegex = "PROMGITHUB_BRANCH_DENY_REGEX" + envWorkflowAllowRegex = "PROMGITHUB_WORKFLOW_ALLOW_REGEX" + envWorkflowDenyRegex = "PROMGITHUB_WORKFLOW_DENY_REGEX" + envNormalizeBranches = "PROMGITHUB_NORMALIZE_BRANCHES" + envDefaultBranches = "PROMGITHUB_DEFAULT_BRANCHES" + envReleaseBranchRegex = "PROMGITHUB_RELEASE_BRANCH_REGEX" + defaultBranchList = "main,master" + defaultReleaseBranchRE = `^(release/|hotfix/).+` + refsHeadsPrefix = "refs/heads/" + refsTagsPrefix = "refs/tags/" +) + +type eventLabels struct { + Repository string + Branch string + Workflow string +} + +type labelPolicy struct { + repoAllow map[string]struct{} + repoDeny map[string]struct{} + branchAllow *regexp.Regexp + branchDeny *regexp.Regexp + workflowAllow *regexp.Regexp + workflowDeny *regexp.Regexp + normalizeBranches bool + defaultBranches map[string]struct{} + releaseBranch *regexp.Regexp +} + +var defaultLabelPolicy labelPolicy + +func loadLabelPolicyFromEnv() (labelPolicy, error) { + policy := labelPolicy{ + repoAllow: parseSetList(os.Getenv(envRepoAllowlist)), + repoDeny: parseSetList(os.Getenv(envRepoDenylist)), + defaultBranches: parseSetList(getEnvOrDefault(envDefaultBranches, defaultBranchList)), + } + + var err error + if policy.branchAllow, err = compileOptionalRegex(envBranchAllowRegex); err != nil { + return labelPolicy{}, err + } + if policy.branchDeny, err = compileOptionalRegex(envBranchDenyRegex); err != nil { + return labelPolicy{}, err + } + if policy.workflowAllow, err = compileOptionalRegex(envWorkflowAllowRegex); err != nil { + return labelPolicy{}, err + } + if policy.workflowDeny, err = compileOptionalRegex(envWorkflowDenyRegex); err != nil { + return labelPolicy{}, err + } + + policy.normalizeBranches, err = parseEnvBool(envNormalizeBranches, false) + if err != nil { + return labelPolicy{}, err + } + + releasePattern := strings.TrimSpace(os.Getenv(envReleaseBranchRegex)) + if releasePattern == "" { + releasePattern = defaultReleaseBranchRE + } + policy.releaseBranch, err = regexp.Compile(releasePattern) + if err != nil { + return labelPolicy{}, fmt.Errorf("parse %s: %w", envReleaseBranchRegex, err) + } + + return policy, nil +} + +func (p labelPolicy) FilterReason(labels eventLabels) string { + repository := normalizeRepoName(labels.Repository) + if len(p.repoAllow) > 0 { + if _, ok := p.repoAllow[repository]; !ok { + return filterReasonRepository + } + } + if _, denied := p.repoDeny[repository]; denied { + return filterReasonRepository + } + + branch := strings.TrimSpace(labels.Branch) + if p.branchAllow != nil && !p.branchAllow.MatchString(branch) { + return filterReasonBranch + } + if p.branchDeny != nil && p.branchDeny.MatchString(branch) { + return filterReasonBranch + } + + workflow := strings.TrimSpace(labels.Workflow) + if workflow == "" { + return "" + } + if p.workflowAllow != nil && !p.workflowAllow.MatchString(workflow) { + return filterReasonWorkflow + } + if p.workflowDeny != nil && p.workflowDeny.MatchString(workflow) { + return filterReasonWorkflow + } + + return "" +} + +func (p labelPolicy) NormalizeBranch(branch string) string { + branch = strings.TrimSpace(branch) + if !p.normalizeBranches || branch == "" { + return branch + } + + if _, ok := p.defaultBranches[strings.ToLower(branch)]; ok { + return branchClassDefault + } + if p.releaseBranch != nil && p.releaseBranch.MatchString(branch) { + return branchClassRelease + } + + return branchClassFeature +} + +func applyLabelPolicy(eventType string, labels eventLabels) (eventLabels, bool) { + if reason := defaultLabelPolicy.FilterReason(labels); reason != "" { + defaultMetricRecorder.RecordFilteredEvent(eventType, reason) + if logger != nil { + logger.Debug("Dropping event due to label policy", + zap.String("eventType", eventType), + zap.String("reason", reason), + zap.String(filterReasonRepository, labels.Repository), + zap.String(filterReasonBranch, labels.Branch), + zap.String(filterReasonWorkflow, labels.Workflow), + ) + } + return labels, false + } + + labels.Branch = defaultLabelPolicy.NormalizeBranch(labels.Branch) + return labels, true +} + +func branchFromRef(ref string) string { + ref = strings.TrimSpace(ref) + switch { + case strings.HasPrefix(ref, refsHeadsPrefix): + return strings.TrimPrefix(ref, refsHeadsPrefix) + case strings.HasPrefix(ref, refsTagsPrefix): + return strings.TrimPrefix(ref, refsTagsPrefix) + default: + return ref + } +} + +func normalizeRepoName(repository string) string { + return strings.ToLower(strings.TrimSpace(repository)) +} + +func parseSetList(value string) map[string]struct{} { + items := strings.Split(value, ",") + set := make(map[string]struct{}, len(items)) + for _, item := range items { + item = strings.ToLower(strings.TrimSpace(item)) + if item == "" { + continue + } + set[item] = struct{}{} + } + return set +} + +func compileOptionalRegex(key string) (*regexp.Regexp, error) { + value := strings.TrimSpace(os.Getenv(key)) + if value == "" { + return nil, nil + } + + compiled, err := regexp.Compile(value) + if err != nil { + return nil, fmt.Errorf("parse %s: %w", key, err) + } + return compiled, nil +} + +func getEnvOrDefault(key, defaultValue string) string { + value := strings.TrimSpace(os.Getenv(key)) + if value == "" { + return defaultValue + } + return value +} + +func logLabelPolicy(logger *zap.Logger, policy labelPolicy) { + if logger == nil { + return + } + + logger.Info("Label policy loaded", + zap.Int("repoAllowlist", len(policy.repoAllow)), + zap.Int("repoDenylist", len(policy.repoDeny)), + zap.Bool("branchAllowRegex", policy.branchAllow != nil), + zap.Bool("branchDenyRegex", policy.branchDeny != nil), + zap.Bool("workflowAllowRegex", policy.workflowAllow != nil), + zap.Bool("workflowDenyRegex", policy.workflowDeny != nil), + zap.Bool("normalizeBranches", policy.normalizeBranches), + ) +} diff --git a/src/label_policy_test.go b/src/label_policy_test.go new file mode 100644 index 0000000..e785400 --- /dev/null +++ b/src/label_policy_test.go @@ -0,0 +1,300 @@ +//go:build !integration + +package main + +import ( + "context" + "os" + "regexp" + "strings" + "testing" + + "github.com/prometheus/client_golang/prometheus/testutil" +) + +func TestLabelPolicyKeepsEventsByDefault(t *testing.T) { + policy := testLabelPolicy(t, nil) + + if reason := policy.FilterReason(eventLabels{ + Repository: "User/Repo", + Branch: "feature/login", + Workflow: "CI", + }); reason != "" { + t.Fatalf("expected default policy to keep the event, got reason %q", reason) + } +} + +func TestLabelPolicyFiltersRepositories(t *testing.T) { + policy := testLabelPolicy(t, func(policy *labelPolicy) { + policy.repoAllow = parseSetList("acme/api, acme/web") + policy.repoDeny = parseSetList("acme/web") + }) + + testCases := []struct { + name string + repository string + wantReason string + }{ + {name: "allowlist miss", repository: "acme/docs", wantReason: filterReasonRepository}, + {name: "denylist hit", repository: "acme/web", wantReason: filterReasonRepository}, + {name: "allowlist hit", repository: "ACME/api", wantReason: ""}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := policy.FilterReason(eventLabels{Repository: tc.repository, Branch: "main"}) + if got != tc.wantReason { + t.Fatalf("FilterReason() = %q, want %q", got, tc.wantReason) + } + }) + } +} + +func TestLabelPolicyFiltersBranchesAndWorkflows(t *testing.T) { + policy := testLabelPolicy(t, func(policy *labelPolicy) { + policy.branchAllow = regexp.MustCompile(`^(main|release/.+)$`) + policy.branchDeny = regexp.MustCompile(`^release/legacy$`) + policy.workflowAllow = regexp.MustCompile(`^(CI|Release)$`) + policy.workflowDeny = regexp.MustCompile(`^Release$`) + }) + + testCases := []struct { + name string + labels eventLabels + wantReason string + }{ + { + name: "branch allow miss", + labels: eventLabels{Repository: "acme/api", Branch: "feature/login", Workflow: "CI"}, + wantReason: filterReasonBranch, + }, + { + name: "branch deny hit", + labels: eventLabels{Repository: "acme/api", Branch: "release/legacy", Workflow: "CI"}, + wantReason: filterReasonBranch, + }, + { + name: "workflow allow miss", + labels: eventLabels{Repository: "acme/api", Branch: "main", Workflow: "Nightly"}, + wantReason: filterReasonWorkflow, + }, + { + name: "workflow deny hit", + labels: eventLabels{Repository: "acme/api", Branch: "main", Workflow: "Release"}, + wantReason: filterReasonWorkflow, + }, + { + name: "push without workflow skips workflow filters", + labels: eventLabels{Repository: "acme/api", Branch: "main"}, + wantReason: "", + }, + { + name: "kept workflow event", + labels: eventLabels{Repository: "acme/api", Branch: "main", Workflow: "CI"}, + wantReason: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := policy.FilterReason(tc.labels) + if got != tc.wantReason { + t.Fatalf("FilterReason() = %q, want %q", got, tc.wantReason) + } + }) + } +} + +func TestLabelPolicyNormalizesBranches(t *testing.T) { + policy := testLabelPolicy(t, func(policy *labelPolicy) { + policy.normalizeBranches = true + }) + + testCases := []struct { + branch string + want string + }{ + {branch: "main", want: branchClassDefault}, + {branch: "Master", want: branchClassDefault}, + {branch: "release/1.4", want: branchClassRelease}, + {branch: "hotfix/login", want: branchClassRelease}, + {branch: "feature/login", want: branchClassFeature}, + {branch: "dependabot/npm/lodash", want: branchClassFeature}, + {branch: "", want: ""}, + } + + for _, tc := range testCases { + name := tc.branch + if name == "" { + name = "empty" + } + t.Run(name, func(t *testing.T) { + got := policy.NormalizeBranch(tc.branch) + if got != tc.want { + t.Fatalf("NormalizeBranch(%q) = %q, want %q", tc.branch, got, tc.want) + } + }) + } + + raw := testLabelPolicy(t, nil) + if got := raw.NormalizeBranch("feature/login"); got != "feature/login" { + t.Fatalf("raw policy should preserve branch names, got %q", got) + } +} + +func TestBranchFromRef(t *testing.T) { + testCases := []struct { + ref string + want string + }{ + {ref: "refs/heads/main", want: "main"}, + {ref: "refs/heads/feature/login", want: "feature/login"}, + {ref: "refs/tags/v1.2.3", want: "v1.2.3"}, + {ref: "main", want: "main"}, + } + + for _, tc := range testCases { + if got := branchFromRef(tc.ref); got != tc.want { + t.Fatalf("branchFromRef(%q) = %q, want %q", tc.ref, got, tc.want) + } + } +} + +func TestLoadLabelPolicyFromEnv(t *testing.T) { + clearLabelPolicyEnv(t) + + policy, err := loadLabelPolicyFromEnv() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if policy.normalizeBranches { + t.Fatal("expected branch normalization to be off by default") + } + if _, ok := policy.defaultBranches["main"]; !ok { + t.Fatal("expected default branch list to include main") + } + + t.Setenv(envRepoAllowlist, "acme/api, acme/web") + t.Setenv(envRepoDenylist, "acme/legacy") + t.Setenv(envBranchAllowRegex, `^(main|release/.+)$`) + t.Setenv(envBranchDenyRegex, `^dependabot/`) + t.Setenv(envWorkflowAllowRegex, `^CI$`) + t.Setenv(envWorkflowDenyRegex, `^Nightly$`) + t.Setenv(envNormalizeBranches, "true") + t.Setenv(envDefaultBranches, "main,develop") + t.Setenv(envReleaseBranchRegex, `^release/.+`) + + policy, err = loadLabelPolicyFromEnv() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !policy.normalizeBranches { + t.Fatal("expected branch normalization to be enabled") + } + if _, ok := policy.repoAllow["acme/api"]; !ok { + t.Fatal("expected allowlist to include acme/api") + } + if _, ok := policy.defaultBranches["develop"]; !ok { + t.Fatal("expected custom default branches to include develop") + } + if policy.NormalizeBranch("release/2") != branchClassRelease { + t.Fatalf("expected custom release regex to classify release/2") + } +} + +func TestLoadLabelPolicyFromEnvRejectsInvalidConfig(t *testing.T) { + clearLabelPolicyEnv(t) + t.Setenv(envBranchAllowRegex, "(") + + if _, err := loadLabelPolicyFromEnv(); err == nil { + t.Fatal("expected invalid branch regex to fail") + } + + clearLabelPolicyEnv(t) + t.Setenv(envNormalizeBranches, "maybe") + if _, err := loadLabelPolicyFromEnv(); err == nil { + t.Fatal("expected invalid boolean to fail") + } +} + +func TestApplyLabelPolicyFiltersAndNormalizesEvents(t *testing.T) { + useInMemoryStateBackends(t) + filteredEventsCounter.Reset() + workflowStatusCounter.Reset() + workflowQueuedGauge.Reset() + workflowInProgressGauge.Reset() + workflowCompletedGauge.Reset() + workflowDurationHistogram.Reset() + + useLabelPolicy(t, testLabelPolicy(t, func(policy *labelPolicy) { + policy.repoDeny = parseSetList("user/repo") + })) + + body, err := os.ReadFile("../test_data/workflow_run.json") + if err != nil { + t.Fatalf("failed to read fixture: %v", err) + } + updateWorkflowMetrics(context.Background(), body) + + if err := testutil.CollectAndCompare(filteredEventsCounter, strings.NewReader(` + # HELP promgithub_event_filtered_total Total number of webhook events dropped by the configured label policy + # TYPE promgithub_event_filtered_total counter + promgithub_event_filtered_total{event_type="workflow_run",reason="repository"} 1 + `)); err != nil { + t.Fatalf("unexpected filtered metric: %v", err) + } + if got := testutil.CollectAndCount(workflowStatusCounter); got != 0 { + t.Fatalf("expected denied repository to skip workflow metrics, got %d series", got) + } + + filteredEventsCounter.Reset() + useLabelPolicy(t, testLabelPolicy(t, func(policy *labelPolicy) { + policy.normalizeBranches = true + })) + updateWorkflowMetrics(context.Background(), body) + + if err := testutil.CollectAndCompare(workflowStatusCounter, strings.NewReader(` + # HELP promgithub_workflow_status Total number of workflow runs with status + # TYPE promgithub_workflow_status counter + promgithub_workflow_status{branch="default",conclusion="success",repository="user/repo",workflow_name="CI",workflow_status="completed"} 1 + `)); err != nil { + t.Fatalf("unexpected normalized workflow metric: %v", err) + } +} + +func TestParseEnvBool(t *testing.T) { + t.Setenv("PROMGITHUB_TEST_BOOL", "") + got, err := parseEnvBool("PROMGITHUB_TEST_BOOL", true) + if err != nil || !got { + t.Fatalf("empty value should use default true, got %v err %v", got, err) + } + + t.Setenv("PROMGITHUB_TEST_BOOL", "YES") + got, err = parseEnvBool("PROMGITHUB_TEST_BOOL", false) + if err != nil || !got { + t.Fatalf("YES should parse as true, got %v err %v", got, err) + } + + t.Setenv("PROMGITHUB_TEST_BOOL", "off") + got, err = parseEnvBool("PROMGITHUB_TEST_BOOL", true) + if err != nil || got { + t.Fatalf("off should parse as false, got %v err %v", got, err) + } +} + +func clearLabelPolicyEnv(t *testing.T) { + t.Helper() + for _, key := range []string{ + envRepoAllowlist, + envRepoDenylist, + envBranchAllowRegex, + envBranchDenyRegex, + envWorkflowAllowRegex, + envWorkflowDenyRegex, + envNormalizeBranches, + envDefaultBranches, + envReleaseBranchRegex, + } { + t.Setenv(key, "") + } +} diff --git a/src/main.go b/src/main.go index 05c997b..d59a3da 100644 --- a/src/main.go +++ b/src/main.go @@ -244,6 +244,13 @@ func main() { } githubWebhookSecret = []byte(ghWebhookSecretEnv) + labelPolicy, err := loadLabelPolicyFromEnv() + if err != nil { + logger.Fatal("Invalid label policy configuration", zap.Error(err)) + } + defaultLabelPolicy = labelPolicy + logLabelPolicy(logger, labelPolicy) + redisConfig, redisEnabled, err := loadRedisConfigFromEnv() if err != nil { logger.Fatal("Invalid Redis configuration", zap.Error(err)) diff --git a/src/metric_recorder.go b/src/metric_recorder.go index 5ee51bf..bc005c8 100644 --- a/src/metric_recorder.go +++ b/src/metric_recorder.go @@ -16,6 +16,10 @@ func (prometheusMetricRecorder) RecordDuplicateDelivery(eventType string) { duplicateDeliveriesDroppedCounter.WithLabelValues(eventType).Inc() } +func (prometheusMetricRecorder) RecordFilteredEvent(eventType, reason string) { + filteredEventsCounter.WithLabelValues(eventType, reason).Inc() +} + func (prometheusMetricRecorder) RecordCommitPushed(repository string) { commitPushedCounter.WithLabelValues(repository).Inc() } diff --git a/src/metric_recorder_test.go b/src/metric_recorder_test.go index bd370ca..c38ccd5 100644 --- a/src/metric_recorder_test.go +++ b/src/metric_recorder_test.go @@ -33,6 +33,21 @@ func TestMetricRecorderPreservesDuplicateDeliveryCardinality(t *testing.T) { } } +func TestMetricRecorderCountsFilteredEvents(t *testing.T) { + filteredEventsCounter.Reset() + + recorder := prometheusMetricRecorder{} + recorder.RecordFilteredEvent(githubEventWorkflowRun, filterReasonRepository) + + if err := testutil.CollectAndCompare(filteredEventsCounter, strings.NewReader(` + # HELP promgithub_event_filtered_total Total number of webhook events dropped by the configured label policy + # TYPE promgithub_event_filtered_total counter + promgithub_event_filtered_total{event_type="workflow_run",reason="repository"} 1 + `)); err != nil { + t.Fatalf("unexpected filtered event metric: %v", err) + } +} + func TestMetricRecorderPreservesRepositoryEventCardinality(t *testing.T) { commitPushedCounter.Reset() pullRequestCounter.Reset() diff --git a/src/metrics.go b/src/metrics.go index d222fff..129e705 100644 --- a/src/metrics.go +++ b/src/metrics.go @@ -183,4 +183,12 @@ var ( }, []string{"event_type"}, ) + + filteredEventsCounter = promauto.NewCounterVec( + prometheus.CounterOpts{ + Name: "promgithub_event_filtered_total", + Help: "Total number of webhook events dropped by the configured label policy", + }, + []string{"event_type", "reason"}, + ) ) diff --git a/src/test_support_test.go b/src/test_support_test.go index efb316a..11e1967 100644 --- a/src/test_support_test.go +++ b/src/test_support_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "regexp" "testing" ) @@ -49,6 +50,29 @@ func useStateBackends( }) } +func useLabelPolicy(t *testing.T, policy labelPolicy) { + t.Helper() + + previous := defaultLabelPolicy + defaultLabelPolicy = policy + t.Cleanup(func() { + defaultLabelPolicy = previous + }) +} + +func testLabelPolicy(t *testing.T, mutate func(*labelPolicy)) labelPolicy { + t.Helper() + + policy := labelPolicy{ + defaultBranches: parseSetList(defaultBranchList), + releaseBranch: regexp.MustCompile(defaultReleaseBranchRE), + } + if mutate != nil { + mutate(&policy) + } + return policy +} + func useWorkflowRunAsyncEventHandler(processor *asyncEventProcessor, handler eventHandler) { processor.dispatcher = githubEventDispatcher{ handlers: map[string]eventHandler{