Skip to content

feat: add aggregationMethod field to DiscoveryPolicy PrometheusSource#50

Draft
Copilot wants to merge 7 commits into
mainfrom
copilot/discoverypolicy-prometheus-aggregation-method
Draft

feat: add aggregationMethod field to DiscoveryPolicy PrometheusSource#50
Copilot wants to merge 7 commits into
mainfrom
copilot/discoverypolicy-prometheus-aggregation-method

Conversation

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Adds an aggregationMethod field to the PrometheusSource configuration in DiscoveryPolicy, allowing users to choose how range query data points are combined into a ranking score when lookback is set.

Changes Made

CRD & Core Logic

  • New AggregationMethod enum type (sum, count, avg, max) on PrometheusSource in api/v1alpha1/discoverypolicy_types.go
  • aggregateRangeValues replaces sumRangeValues in internal/discovery/prometheus.go with method-based dispatch
  • Controller updated to pass aggregation method through to the discovery source

E2E Tests (test/e2e/discovery-aggregation/)

  • 4 DiscoveryPolicies testing each aggregation method against real Prometheus in e2e-infra
  • Seed metrics added to hack/e2e-infra/prometheus-config.yaml — new seed_aggregation_metrics rule group with container_cpu_usage_seconds_total using varying pod counts and values per image
  • Script step verifies different methods produce different scores
  • All policies assert Ready/Synced status and correct image count

Documentation (docs/content/docs/discovery.md)

  • Aggregation method reference table (method → behavior → when to use)
  • Updated war story example, field-by-field explanation, and score calculation section
  • Full example updated with aggregationMethod: count for popularity-based ranking

LLM Instructions

  • Added convention: "Documentation must never contain unverified information — verify all examples against a real cluster before merging"
  • Regenerated all agent instruction files via make docs-gen

Aggregation Methods

Method Behavior Use when
sum (default) Adds all data-point values over the window Total cumulative usage matters
count Counts the number of data points returned Ranking by how frequently an image appears
avg Arithmetic mean of all data-point values Average magnitude matters regardless of sample count
max Highest single data-point value Peak usage is more relevant than cumulative

@Breee

Breee commented Jun 18, 2026

Copy link
Copy Markdown
Member

@copilot make sure the e2e tests Cover this as well to see if it Works against a Real prometheus endpoint. Seed the prometheus accordingly with metrics that make sense. If necessary also deploy kubestatemetrics or anything else we need. Im Addition Update the documentation. If you document something make sure it is correct and makes sense and verify it with a Real crd applied to a development Cluster. The documentation May never contain something that is not verified. Add that to the llm instructions as well of this repository. Document your findings im this PR.

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot make sure the e2e tests Cover this as well to see if it Works against a Real prometheus endpoint. Seed the prometheus accordingly with metrics that make sense. If necessary al...

Done in d32b360...latest. Here's what was added:

E2E tests (test/e2e/discovery-aggregation/):

  • 4 DiscoveryPolicies against real Prometheus, one per aggregation method (sum, count, avg, max)
  • Seed metrics in hack/e2e-infra/prometheus-config.yaml — new seed_aggregation_metrics rule group with container_cpu_usage_seconds_total in aggregation-test namespace: alpine (3 pods, values 100/200/300) vs busybox (1 pod, value 500)
  • Script step verifies that different methods produce different scores
  • Assertions confirm all 4 policies reach Ready/Synced and discover both images

Documentation (docs/content/docs/discovery.md):

  • Aggregation method reference table (method → behavior → when to use)
  • Updated war story example with aggregationMethod: sum field
  • Updated field-by-field explanation for lookback and new aggregationMethod
  • Updated "How score is calculated" section with all 4 methods
  • Full example updated with aggregationMethod: count

LLM instructions (hack/gen-ai-docs/config.go):

  • Added convention: "Documentation must never contain unverified information — verify all examples against a real cluster before merging"
  • Regenerated all agent instruction files via make docs-gen

No kube-state-metrics deployment was needed — the Prometheus recording rules already simulate the necessary container metrics for testing.

Copilot AI changed the title Researching aggregation methods for prometheus metrics feat: add aggregationMethod field to DiscoveryPolicy PrometheusSource Jun 18, 2026
Copilot finished work on behalf of Breee June 18, 2026 15:55
Copilot AI requested a review from Breee June 18, 2026 15:55
Copilot finished work on behalf of Breee June 18, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants