diff --git a/roadmap-planner/backend/config.example.yaml b/roadmap-planner/backend/config.example.yaml index a4358f19..6748ef0a 100644 --- a/roadmap-planner/backend/config.example.yaml +++ b/roadmap-planner/backend/config.example.yaml @@ -49,6 +49,15 @@ metrics: path: "/metrics" namespace: "roadmap" # Metric prefix: roadmap_dora_* + # Components dropped from all component dimensions (D6 — v3-era + # plugins must not dilute v4 team metrics). Exact component names as + # parsed from version names (e.g. katanomi-v3.1.0 -> katanomi). + exclude_plugins: + - katanomi + - knative + - jenkins + - tekton-operator + filters: - name: "releases" enabled: true diff --git a/roadmap-planner/backend/internal/config/config.go b/roadmap-planner/backend/internal/config/config.go index 4f868b76..04d8e533 100644 --- a/roadmap-planner/backend/internal/config/config.go +++ b/roadmap-planner/backend/internal/config/config.go @@ -274,6 +274,22 @@ type Metrics struct { Prometheus PrometheusConfig `mapstructure:"prometheus"` Filters []OptionsConfig `mapstructure:"filters"` Calculators []OptionsConfig `mapstructure:"calculators"` + // ExcludePlugins lists component names dropped from all component + // dimensions (releases and issue components) during collection. + // Implements plan D6 — v3-era plugins (katanomi, knative, jenkins, + // tekton-operator) must not dilute v4 team metrics. Exact match. + ExcludePlugins []string `mapstructure:"exclude_plugins"` +} + +// IsPluginExcluded reports whether a component name is in the +// exclude_plugins list. +func (c *Metrics) IsPluginExcluded(name string) bool { + for _, p := range c.ExcludePlugins { + if p == name { + return true + } + } + return false } // PrometheusConfig represents Prometheus exporter configuration diff --git a/roadmap-planner/backend/internal/metrics/calculators/patch_ratio.go b/roadmap-planner/backend/internal/metrics/calculators/patch_ratio.go index 9c8fcc92..22ca62e1 100644 --- a/roadmap-planner/backend/internal/metrics/calculators/patch_ratio.go +++ b/roadmap-planner/backend/internal/metrics/calculators/patch_ratio.go @@ -70,7 +70,9 @@ func (c *PatchRatioCalculator) Calculate(ctx context.Context, data *models.Calcu component := release.Component if component == "" { - component = "unknown" + // Collector drops these, but guard here too: an unparsable + // version name is not a component bucket. + continue } // Apply component filter if specified diff --git a/roadmap-planner/backend/internal/metrics/calculators/patch_ratio_test.go b/roadmap-planner/backend/internal/metrics/calculators/patch_ratio_test.go new file mode 100644 index 00000000..03ccf8bf --- /dev/null +++ b/roadmap-planner/backend/internal/metrics/calculators/patch_ratio_test.go @@ -0,0 +1,51 @@ +/* +Copyright 2024 The AlaudaDevops Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package calculators + +import ( + "context" + "testing" + "time" + + "github.com/AlaudaDevops/toolbox/roadmap-planner/backend/internal/metrics/models" +) + +// TestPatchRatio_SkipsEmptyComponentReleases locks in the +// invalid-component guard: a release whose name did not parse +// (Component == "") used to be bucketed under "unknown"; it must now be +// skipped entirely so legacy version names like "0.3" never surface as +// component buckets. +func TestPatchRatio_SkipsEmptyComponentReleases(t *testing.T) { + c := NewPatchRatioCalculator(nil) + + relDate := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC) + ctx := &models.CalculationContext{ + Releases: []models.EnrichedRelease{ + {ID: "v1", Name: "argo-cd-2.9.1", Component: "argo-cd", Released: true, ReleaseDate: relDate, Type: "patch"}, + {ID: "v2", Name: "0.3", Component: "", Released: true, ReleaseDate: relDate, Type: "unknown"}, + }, + TimeRange: models.TimeRange{ + Start: relDate.AddDate(0, -6, 0), + End: relDate.AddDate(0, 1, 0), + }, + } + + results, err := c.Calculate(context.Background(), ctx) + if err != nil { + t.Fatalf("Calculate: %v", err) + } + if len(results) != 1 { + t.Fatalf("len(results) = %d, want 1 (only the valid component)", len(results)) + } + if got := results[0].Labels["component"]; got != "argo-cd" { + t.Errorf("component = %q, want argo-cd (empty-component release must not bucket as %q)", got, got) + } +} diff --git a/roadmap-planner/backend/internal/metrics/calculators/release_frequency.go b/roadmap-planner/backend/internal/metrics/calculators/release_frequency.go index bb75098a..7e2c0056 100644 --- a/roadmap-planner/backend/internal/metrics/calculators/release_frequency.go +++ b/roadmap-planner/backend/internal/metrics/calculators/release_frequency.go @@ -62,7 +62,9 @@ func (c *ReleaseFrequencyCalculator) Calculate(ctx context.Context, data *models component := release.Component if component == "" { - component = "unknown" + // Collector drops these, but guard here too: an unparsable + // version name is not a component bucket. + continue } // Apply component filter if specified diff --git a/roadmap-planner/backend/internal/metrics/calculators/release_frequency_test.go b/roadmap-planner/backend/internal/metrics/calculators/release_frequency_test.go new file mode 100644 index 00000000..56e823e0 --- /dev/null +++ b/roadmap-planner/backend/internal/metrics/calculators/release_frequency_test.go @@ -0,0 +1,51 @@ +/* +Copyright 2024 The AlaudaDevops Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package calculators + +import ( + "context" + "testing" + "time" + + "github.com/AlaudaDevops/toolbox/roadmap-planner/backend/internal/metrics/models" +) + +// TestReleaseFrequency_SkipsEmptyComponentReleases locks in the +// invalid-component guard: a release whose name did not parse +// (Component == "") used to be bucketed under "unknown"; it must now be +// skipped entirely so legacy version names like "0.3" never surface as +// component buckets. +func TestReleaseFrequency_SkipsEmptyComponentReleases(t *testing.T) { + c := NewReleaseFrequencyCalculator(nil) + + relDate := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC) + ctx := &models.CalculationContext{ + Releases: []models.EnrichedRelease{ + {ID: "v1", Name: "argo-cd-2.9.0", Component: "argo-cd", Released: true, ReleaseDate: relDate, Type: "minor"}, + {ID: "v2", Name: "0.3", Component: "", Released: true, ReleaseDate: relDate, Type: "unknown"}, + }, + TimeRange: models.TimeRange{ + Start: relDate.AddDate(0, -3, 0), + End: relDate.AddDate(0, 1, 0), + }, + } + + results, err := c.Calculate(context.Background(), ctx) + if err != nil { + t.Fatalf("Calculate: %v", err) + } + if len(results) != 1 { + t.Fatalf("len(results) = %d, want 1 (only the valid component)", len(results)) + } + if got := results[0].Labels["component"]; got != "argo-cd" { + t.Errorf("component = %q, want argo-cd (empty-component release must not bucket as %q)", got, got) + } +} diff --git a/roadmap-planner/backend/internal/metrics/collector.go b/roadmap-planner/backend/internal/metrics/collector.go index e1b05a01..e3edc277 100644 --- a/roadmap-planner/backend/internal/metrics/collector.go +++ b/roadmap-planner/backend/internal/metrics/collector.go @@ -159,10 +159,54 @@ func (c *Collector) fetchReleases(ctx context.Context) ([]models.EnrichedRelease c.logger.Debug("Filtered releases") } + releases = c.dropInvalidComponentReleases(releases) + c.logger.Debug("Fetched releases", zap.Int("count", len(releases)), zap.Int("original", originalCount)) return releases, nil } +// dropInvalidComponentReleases removes releases that must not feed any +// component dimension: names that do not parse to component-X.Y.Z +// (Component is empty — legacy names like "0.3", "v2.1") and components +// in metrics.exclude_plugins (D6 — v3-era plugins). Dropping here keeps +// every calculator clean without per-calculator filtering, and also +// removes the releases from the versionDates maps so issues linked only +// to invalid versions stay out of release-scoped metrics. +func (c *Collector) dropInvalidComponentReleases(releases []models.EnrichedRelease) []models.EnrichedRelease { + kept := make([]models.EnrichedRelease, 0, len(releases)) + dropped := make([]string, 0) + for _, r := range releases { + if r.Component == "" || c.config.IsPluginExcluded(r.Component) { + dropped = append(dropped, r.Name) + continue + } + kept = append(kept, r) + } + if len(dropped) > 0 { + c.logger.Debug("Dropped releases with invalid or excluded components", + zap.Int("count", len(dropped)), zap.Strings("names", dropped)) + } + return kept +} + +// dropExcludedComponents removes metrics.exclude_plugins entries from an +// issue's component list (both Jira Component fields and components +// extracted from version names) so D6 plugins disappear from issue-based +// component dimensions (cycle_time, time_to_patch). +func (c *Collector) dropExcludedComponents(components []string) []string { + if len(c.config.ExcludePlugins) == 0 || len(components) == 0 { + return components + } + kept := make([]string, 0, len(components)) + for _, comp := range components { + if c.config.IsPluginExcluded(comp) { + continue + } + kept = append(kept, comp) + } + return kept +} + // enrichRelease converts a basic Version to an EnrichedRelease func (c *Collector) enrichRelease(v baseModels.Version) models.EnrichedRelease { release := models.EnrichedRelease{ @@ -205,8 +249,11 @@ func parseVersionName(name string) (component string, major, minor, patch int) { return } - // If no version pattern found, the whole name is the component - component = name + // No version pattern found — the name does not identify a component. + // Legacy names like "0.3", "v2.1" or "1.0" used to fall back to the + // whole name here, which polluted every per-component metric with + // bogus component buckets. Return empty so collectors/calculators + // drop the release from component dimensions. return } @@ -286,6 +333,7 @@ func (c *Collector) fetchEpics(ctx context.Context) ([]models.EnrichedIssue, err } } } + enriched.Components = c.dropExcludedComponents(enriched.Components) epics = append(epics, enriched) } @@ -355,6 +403,7 @@ func (c *Collector) fetchIssues(ctx context.Context) ([]models.EnrichedIssue, er } } } + enriched.Components = c.dropExcludedComponents(enriched.Components) issues = append(issues, enriched) } diff --git a/roadmap-planner/backend/internal/metrics/collector_test.go b/roadmap-planner/backend/internal/metrics/collector_test.go new file mode 100644 index 00000000..3b4307c2 --- /dev/null +++ b/roadmap-planner/backend/internal/metrics/collector_test.go @@ -0,0 +1,105 @@ +/* +Copyright 2024 The AlaudaDevops Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package metrics + +import ( + "reflect" + "testing" + + "github.com/AlaudaDevops/toolbox/roadmap-planner/backend/internal/config" + "github.com/AlaudaDevops/toolbox/roadmap-planner/backend/internal/metrics/models" + "go.uber.org/zap" +) + +// TestParseVersionName guards the invalid-component fix: names that do +// not carry a component-X.Y.Z pattern must return an empty component +// instead of the legacy whole-name fallback that polluted every +// per-component metric with buckets like "0.3" or "v2.1". +func TestParseVersionName(t *testing.T) { + cases := []struct { + name string + wantComponent string + major, minor, patch int + }{ + // Valid names — component extracted. + {"argo-cd-2.9.0", "argo-cd", 2, 9, 0}, + {"tektoncd-operator-v4.6.3", "tektoncd-operator", 4, 6, 3}, + {"harbor 1.2.3", "harbor", 1, 2, 3}, + {"connectors-operator-1.2.3-rc1", "connectors-operator", 1, 2, 3}, + // Legacy / invalid names — no component. + {"0.3", "", 0, 0, 0}, + {"v2.1", "", 0, 0, 0}, + {"1.0", "", 0, 0, 0}, + {"Sprint 2024", "", 0, 0, 0}, + {"", "", 0, 0, 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + component, major, minor, patch := parseVersionName(tc.name) + if component != tc.wantComponent || major != tc.major || minor != tc.minor || patch != tc.patch { + t.Errorf("parseVersionName(%q) = (%q, %d, %d, %d), want (%q, %d, %d, %d)", + tc.name, component, major, minor, patch, + tc.wantComponent, tc.major, tc.minor, tc.patch) + } + }) + } +} + +// TestDropInvalidComponentReleases covers both invalid classes: empty +// component (unparsable version name) and metrics.exclude_plugins (D6 — +// v3-era plugins). +func TestDropInvalidComponentReleases(t *testing.T) { + c := &Collector{ + config: &config.Metrics{ + ExcludePlugins: []string{"katanomi", "knative", "jenkins", "tekton-operator"}, + }, + logger: zap.NewNop(), + } + in := []models.EnrichedRelease{ + {Name: "tektoncd-operator-v4.6.3", Component: "tektoncd-operator"}, + {Name: "0.3", Component: ""}, // unparsable → dropped + {Name: "katanomi-v3.1.0", Component: "katanomi"}, // D6 → dropped + {Name: "tekton-operator-v3.20.0", Component: "tekton-operator"}, // D6 → dropped + {Name: "argo-cd-2.9.0", Component: "argo-cd"}, + } + got := c.dropInvalidComponentReleases(in) + want := []string{"tektoncd-operator", "argo-cd"} + gotComponents := make([]string, 0, len(got)) + for _, r := range got { + gotComponents = append(gotComponents, r.Component) + } + if !reflect.DeepEqual(gotComponents, want) { + t.Errorf("kept components = %v, want %v", gotComponents, want) + } +} + +// TestDropExcludedComponents covers the issue-side path: D6 plugins are +// removed from issue component lists while everything else is kept. +func TestDropExcludedComponents(t *testing.T) { + c := &Collector{config: &config.Metrics{ExcludePlugins: []string{"katanomi", "jenkins"}}} + cases := []struct { + name string + in []string + want []string + }{ + {"mixed", []string{"katanomi", "tektoncd-operator", "jenkins"}, []string{"tektoncd-operator"}}, + {"all excluded", []string{"katanomi"}, []string{}}, + {"none excluded", []string{"argo-cd"}, []string{"argo-cd"}}, + {"empty", nil, nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := c.dropExcludedComponents(tc.in); !reflect.DeepEqual(got, tc.want) { + t.Errorf("dropExcludedComponents(%v) = %v, want %v", tc.in, got, tc.want) + } + }) + } +}