diff --git a/workspaces/scorecard/.changeset/green-cats-care.md b/workspaces/scorecard/.changeset/green-cats-care.md new file mode 100644 index 00000000000..295dd10ec37 --- /dev/null +++ b/workspaces/scorecard/.changeset/green-cats-care.md @@ -0,0 +1,7 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': minor +'@red-hat-developer-hub/backstage-plugin-scorecard-common': minor +'@red-hat-developer-hub/backstage-plugin-scorecard': minor +--- + +Implemented filter by `status` for scalar aggregation types (`sum`, `average`, `count`, `min`, `max`). diff --git a/workspaces/scorecard/README.md b/workspaces/scorecard/README.md index dc55dc824b0..5cd84e6ad47 100644 --- a/workspaces/scorecard/README.md +++ b/workspaces/scorecard/README.md @@ -15,10 +15,10 @@ yarn install ## Documentation -| Topic | Location | -| --------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------- | -| Aggregation KPIs (`statusGrouped`, `weightedStatusScore`, scalar `sum`/`average`/`max`/`min`/`count`), API, ownership | [plugins/scorecard-backend/docs/aggregation.md](plugins/scorecard-backend/docs/aggregation.md) | -| Backend installation and RBAC, **`scorecard.aggregationKPIs`** examples | [plugins/scorecard-backend/README.md](plugins/scorecard-backend/README.md) | -| Drill-down (entity list for a metric) | [plugins/scorecard-backend/docs/drill-down.md](plugins/scorecard-backend/docs/drill-down.md) | -| Metric thresholds, annotations, **weightedStatusScore KPI result colors** | [plugins/scorecard-backend/docs/thresholds.md](plugins/scorecard-backend/docs/thresholds.md) | -| Frontend (homepage cards, NFS) | [plugins/scorecard/README.md](plugins/scorecard/README.md) | +| Topic | Location | +| ------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------- | +| Aggregation KPIs (`statusGrouped`, `weightedStatusScore`, scalar `sum`/`average`/`max`/`min`/`count`, **`filter.status`**), API, ownership | [plugins/scorecard-backend/docs/aggregation.md](plugins/scorecard-backend/docs/aggregation.md) | +| Backend installation and RBAC, **`scorecard.aggregationKPIs`** examples | [plugins/scorecard-backend/README.md](plugins/scorecard-backend/README.md) | +| Drill-down (entity list for a metric) | [plugins/scorecard-backend/docs/drill-down.md](plugins/scorecard-backend/docs/drill-down.md) | +| Metric thresholds, annotations, **weightedStatusScore** and **scalar** KPI result thresholds, **`filter.status`** for scalar KPIs | [plugins/scorecard-backend/docs/thresholds.md](plugins/scorecard-backend/docs/thresholds.md) | +| Frontend (homepage cards, NFS) | [plugins/scorecard/README.md](plugins/scorecard/README.md) | diff --git a/workspaces/scorecard/app-config.yaml b/workspaces/scorecard/app-config.yaml index 11e87d52e8d..665ea9e2a82 100644 --- a/workspaces/scorecard/app-config.yaml +++ b/workspaces/scorecard/app-config.yaml @@ -274,20 +274,25 @@ scorecard: type: statusGrouped description: This KPI is provide information about whether the license file exists in the repository. metricId: filecheck.license - totalOpenPrs: - title: Total Open PRs - description: Sum of open PRs across owned entities + totalCriticalPRs: + title: Total Critical PRs + description: Sum of open PRs for entities in error status type: sum metricId: github.openPRs + filter: + status: error options: thresholds: rules: - key: success - expression: '>=80' - color: '#6bb300' # green + expression: '<30' + color: '#6bb300' - key: warning - expression: '<80' - color: 'rgb(224, 189, 108)' # light orange + expression: '30-80' + color: 'rgb(224, 189, 108)' + - key: error + expression: '>80' + color: '#be1ec7' avgOpenIssues: title: Average Open Issues description: Mean open issues count per entity diff --git a/workspaces/scorecard/plugins/scorecard-backend/README.md b/workspaces/scorecard/plugins/scorecard-backend/README.md index de05c762959..121ec624134 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend/README.md @@ -177,19 +177,59 @@ scorecard: - key: error expression: '<10' color: error.main + totalCriticalPRs: + title: Total Critical PRs + description: Sum of open PRs for entities in error status + type: sum + metricId: github.openPRs + filter: + status: error + options: + thresholds: + rules: + - key: success + expression: '<30' + color: '#6bb300' + - key: warning + expression: '30-80' + color: 'rgb(224, 189, 108)' + - key: error + expression: '>80' + color: '#be1ec7' + avgOpenIssues: + title: Average Open Issues + description: Mean open issues count per entity + type: average + metricId: jira.openIssues + entitiesWithOpenPrs: + title: Entities with Open PRs + description: Count of entities with a stored open-prs value + type: count + metricId: github.openPRs + maxOpenPrs: + title: Maximum Open PRs + description: Maximum open PR count per entity + type: max + metricId: github.openPRs + minOpenIssues: + title: Minimum Open Issues + description: Minimum open issues count per entity + type: min + metricId: jira.openIssues ``` -| Field | Description | -| ------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `title` | Display title for this aggregation (returned in API metadata). | -| `description` | Display description for this aggregation. | -| `type` | Aggregation algorithm: `statusGrouped` (counts per threshold status), `weightedStatusScore` (normalized weighted score), or a **scalar** type — `sum`, `average`, `max`, `min`, `count` (rollup of latest numeric metric values; see [aggregation.md](./docs/aggregation.md#aggregation-types)). Scalar types require a **number** metric. | -| `metricId` | Metric provider id used to load thresholds and compute counts or scalar values. | -| `options` | **Optional:** extra configuration attributes required to further configure the aggregated card for a specific type | +| Field | Description | +| ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `title` | Display title for this aggregation (returned in API metadata). | +| `description` | Display description for this aggregation. | +| `type` | Aggregation algorithm: `statusGrouped` (counts per threshold status), `weightedStatusScore` (normalized weighted score), or a **scalar** type — `sum`, `average`, `max`, `min`, `count` (rollup of latest numeric metric values; see [aggregation.md](./docs/aggregation.md#aggregation-types)). Scalar types require a **number** metric. | +| `metricId` | Metric provider id used to load thresholds and compute counts or values. | +| `filter` | Optional for **scalar types** only (silently ignored on **`statusGrouped`** / **`weightedStatusScore`**). **`filter.status`**: threshold status key (case-sensitive). Only latest rows with that status contribute to **`value`** and **`total`**. Must match a threshold rule **`key`** for **`metricId`** from provider defaults or app-config overrides at **`scorecard.metricProviders...metrics..thresholds`** (or provider-level **`scorecard.metricProviders...thresholds`**) (validated at startup for scalar types). **`entitiesConsidered`** and **`calculationErrorCount`** stay full-portfolio. Returned in API **`metadata.filter`** when set. | +| `options` | **Optional:** extra configuration attributes required to further configure the aggregated card for a specific type | - **Path**: `scorecard.aggregationKPIs.`. - If **`aggregationKPIs` is omitted** or a given id is not listed, **`GET /aggregations/:aggregationId`** still works when **`aggregationId` equals the metric id** (e.g. `github.openPRs`): the backend uses that metric with the default `statusGrouped` aggregation and metric-defined title/description. -- **Startup validation**: the backend validates every **`scorecard.aggregationKPIs`** entry when the plugin loads. Invalid configuration (including **`weightedStatusScore`** KPIs without **`options.statusScores`**, bad expressions, or unregistered **`metricId`**) causes the backend to **fail to start** with a clear error. At runtime, some edge cases may still be logged (for example skipping a KPI with unusable weights); prefer correcting app-config. See [aggregation.md](./docs/aggregation.md#configuration-validation). +- **Startup validation**: the backend validates every **`scorecard.aggregationKPIs`** entry when the plugin loads. Invalid configuration (including **`weightedStatusScore`** KPIs without **`options.statusScores`**, non-count scalar types on boolean metrics, invalid **`filter.status`** keys on scalar types, bad threshold expressions, or unregistered **`metricId`**) causes the backend to **fail to start** with a clear error. At runtime, some edge cases may still be logged (for example skipping a KPI with unusable weights); prefer correcting app-config. See [aggregation.md](./docs/aggregation.md#configuration-validation). **Homepage cards** are configured in the app (for example Dynamic Home Page mount points). They should pass **`aggregationId`** matching a key in `aggregationKPIs` or the metric id for the default case. See the [Scorecard frontend plugin README](../scorecard/README.md#homepage-scorecard-cards). @@ -262,7 +302,7 @@ curl -X GET "{{url}}/api/scorecard/metrics/catalog/component/default/my-service? Returns aggregated metrics for the authenticated user across all catalog entities they own (same ownership rules as the legacy route; see [aggregation.md](./docs/aggregation.md)). -Response **`result`** shape depends on **`metadata.aggregationType`**: status counts for **`statusGrouped`**, weighted score fields for **`weightedStatusScore`**, or scalar fields for **`sum`** / **`average`** / **`max`** / **`min`** / **`count`** — see [Scalar result fields](./docs/aggregation.md#scalar-result-fields). +Response **`result`** shape depends on **`metadata.aggregationType`**: status counts for **`statusGrouped`**, weighted score fields for **`weightedStatusScore`**, or scalar fields for **`sum`** / **`average`** / **`max`** / **`min`** / **`count`** — see [Scalar result fields](./docs/aggregation.md#scalar-result-fields). Scalar KPIs may also return **`metadata.filter`** when **`filter.status`** is configured. The **`aggregationId`** is either: @@ -292,7 +332,7 @@ curl -X GET "{{url}}/api/scorecard/aggregations/github.openPRs" \ ### `GET /aggregations/:aggregationId/metadata` -Returns **title**, **description**, **type**, **history**, and **aggregationType** for the aggregation without computing full aggregate counts. Uses the same resolution rules as `GET /aggregations/:aggregationId` (KPI config vs metric id fallback). +Returns **title**, **description**, **type**, **history**, and **aggregationType** for the aggregation without computing full aggregate counts. Includes **`filter`** when the KPI is a scalar type with **`filter.status`** configured. Uses the same resolution rules as `GET /aggregations/:aggregationId` (KPI config vs metric id fallback). ```bash curl -X GET "{{url}}/api/scorecard/aggregations/openIssuesKpi/metadata" \ diff --git a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts index de3ba0b2d56..670476a9527 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts @@ -40,6 +40,9 @@ export const buildMockMetricProvidersRegistry = ({ const getProvider = provider ? jest.fn().mockReturnValue(provider) : jest.fn(); + const listProviders = provider + ? jest.fn().mockReturnValue([provider]) + : jest.fn().mockReturnValue([]); const listMetrics = metricsList ? jest.fn().mockImplementation((metricIds?: string[]) => { if (metricIds && metricIds.length !== 0) { @@ -72,5 +75,6 @@ export const buildMockMetricProvidersRegistry = ({ hasProvider, getMetric, listMetrics, + listProviders, } as unknown as jest.Mocked; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockThresholds.ts b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockThresholds.ts index 471b1fa4710..afdc9ec1d5a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockThresholds.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockThresholds.ts @@ -16,7 +16,8 @@ import type { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -export const mockFirstThresholds = { +/** Success when value is >= 80; error otherwise. */ +export const mockHigherIsBetterThresholds = { rules: [ { key: 'success', @@ -31,7 +32,8 @@ export const mockFirstThresholds = { ], } as ThresholdConfig; -export const mockSecondThresholds = { +/** Error when value is > 50; warning 10–50; success when < 10. */ +export const mockLowerIsBetterThresholds = { rules: [ { key: 'error', diff --git a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts index 4aeb9b54908..6b625252e8a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts @@ -76,6 +76,11 @@ export interface Config { | typeof aggregationTypes.max | typeof aggregationTypes.min | typeof aggregationTypes.count; + /** Optional: filter applied when aggregating scalar KPI values */ + filter?: { + /** Threshold status key to include (e.g. success, warning, error) */ + status?: string; + }; /** Options specific to the scalar aggregation type */ options?: { /** Optional: threshold rules for coloring the KPI headline value from the aggregation result */ diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md index fd8943895fc..4c22afbd453 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md @@ -96,8 +96,9 @@ Shared behavior for all scalar types: 1. **Latest row per entity:** Same scope as other aggregation KPIs — one row per owned catalog entity ref (the row with the highest `id` for that entity and metric). 2. **Calculation failures excluded:** Rows where `error_message` is set and `value` is null are excluded from the aggregate (same rule as status-grouped aggregation). 3. **SQL function:** `sum` → `SUM(value)`, `average` → `AVG(value)`, `max` → `MAX(value)`, `min` → `MIN(value)`, `count` → `COUNT(*)` over rows with a non-null value. -4. **Metric type rules:** All scalar types require a **number** metric. Startup validation rejects scalar KPIs that target a boolean metric. +4. **Metric type rules:** `sum`, `average`, `max`, and `min` require a **number** metric. `count` is also valid for boolean metrics. Startup validation rejects non-count scalar KPIs that target a boolean metric. 5. **Optional result thresholds:** `options.thresholds` (number-style rules) can color or classify the aggregated **`value`**. When omitted, the API returns **`DEFAULT_NUMBER_THRESHOLDS`**. See [thresholds.md — Aggregation KPI result thresholds (scalar types)](./thresholds.md#5-aggregation-kpi-result-thresholds-scalar-types). +6. **Optional status filter:** Top-level **`filter.status`** restricts which latest rows contribute to **`value`** / **`total`**. See [Status filter (scalar types)](#status-filter-scalar-types). ### Sum type @@ -126,6 +127,14 @@ scorecard: - key: error expression: '>50' color: error.main + # Optional filter.status: only include entities whose metric status matches + totalCriticalBugs: + title: 'Total Critical Bugs' + description: 'Sum of open issues for entities in error status.' + type: sum + metricId: jira.openIssues + filter: + status: error ``` ### Average type @@ -204,6 +213,12 @@ scorecard: | **`min`** | Minimum latest metric value across owned entities | “Best-case / lowest value in the portfolio.” | | **`count`** | Number of entities with a non-null latest stored value | “How many entities have data for this metric.” | +### Status filter (scalar types) + +Scalar KPIs may include an optional top-level **`filter.status`** to restrict aggregation to latest rows whose threshold evaluation matches the given status key (for example `success`, `warning`, `error`, or any custom threshold key). On non-scalar types (`statusGrouped`, `weightedStatusScore`), **`filter`** is silently ignored. + +**Startup validation:** When **`filter.status`** is set, the backend validates at plugin load that the value is a **threshold rule key** for the KPI’s **`metricId`** (provider defaults plus app-config at **`scorecard.metricProviders...metrics..thresholds`** or provider-level **`scorecard.metricProviders...thresholds`** — for example `scorecard.metricProviders.jira.openIssues.metrics.openIssues.thresholds` when **`metricId`** is `jira.openIssues`). Keys are **case-sensitive** (`error` ≠ `Error`) and must be 1–64 characters. Invalid keys cause startup to fail with an error listing valid keys. Per-entity annotation threshold overrides are **not** considered (they apply at metric sync time only). See [thresholds.md — Scalar status filter](./thresholds.md#5-aggregation-kpi-result-thresholds-scalar-types). + ## Configuration validation - **`scorecard.aggregationKPIs`** is validated when the backend plugin starts. Invalid entries cause startup to fail with an error so misconfiguration is caught early. Fix app-config and redeploy. @@ -219,49 +234,56 @@ Use this endpoint for all new integrations. - **`aggregationId`** may be a key under **`scorecard.aggregationKPIs`** in app-config (see the [backend README](../README.md#aggregation-kpis-homepage-and-get-aggregations)), which supplies **title**, **description**, **type**, **metricId**, and type-specific **`options`** (for example **`options.statusScores`** for **`weightedStatusScore`**, or optional **`options.thresholds`** for scalar types and **`weightedStatusScore`**). - If there is **no** `scorecard.aggregationKPIs.` block, the backend still responds successfully: it treats **`aggregationId` as the `metricId`** and uses the default **statusGrouped** strategy (same as calling **`/aggregations/`** with a metric id). A **warning** is logged on the server so missing KPI config is visible in operator logs. To get a custom **title**, **`weightedStatusScore`** or **scalar** type, or other KPI options, you must add that block; a typo in the id falls through to this default and can look like “wrong” aggregation behavior in the UI, so check logs and app-config. -The response shape includes **`id`**, **`status`**, **`metadata`** (title, description, type, aggregation type), and **`result`**. The shape of **`result`** depends on the aggregation type: +The response shape includes **`id`**, **`status`**, **`metadata`** (title, description, type, aggregation type, and **`filter`** when configured), and **`result`**. The shape of **`result`** depends on the aggregation type: - **`statusGrouped`**: counts per threshold rule, **`total`**, **`thresholds`**, **`entitiesConsidered`**, **`calculationErrorCount`**, **`timestamp`**. - **`weightedStatusScore`**: same as status-grouped, plus **`weightedStatusScore`** (portfolio percentage in \[0, 100\], one decimal), **`weightedStatusSum`**, **`weightedStatusMaxPossible`**, and **`aggregationChartDisplayColor`** (see backend README). The homepage card shows a donut gauge for this type instead of a multi-slice status pie. -- **Scalar types** (`sum`, `average`, `max`, `min`, `count`): see [Scalar result fields](#scalar-result-fields) below. +- **Scalar types** (`sum`, `average`, `max`, `min`, `count`): see [Scalar result fields](#scalar-result-fields) below. When **`filter.status`** is configured, **`metadata.filter`** is also returned. ### Scalar result fields When **`metadata.aggregationType`** is one of **`sum`**, **`average`**, **`max`**, **`min`**, or **`count`**, **`result`** is a scalar aggregation payload: -| Field | Description | -| --------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **`value`** | Aggregated scalar from the configured SQL function (`sum`, `average`, `max`, `min`, or entity count for `count`) over latest non-null metric values. Evaluated by **`options.thresholds`** when present. | -| **`total`** | Number of latest rows that contributed to **`value`** (non-null values; calculation failures excluded). For **`count`**, equals **`value`**. | -| **`entitiesConsidered`** | In-scope owned entities with **at least one** latest `metric_values` row for this metric (includes calculation-error rows). | -| **`calculationErrorCount`** | Among **`entitiesConsidered`**, how many latest rows are metric calculation failures (`error_message` set and `value` null). | -| **`timestamp`** | ISO timestamp of the most recent among the latest rows in scope (same merge rule as other aggregation types). | -| **`thresholds`** | Number-style threshold rules for classifying **`value`**; from **`options.thresholds`** or **`DEFAULT_NUMBER_THRESHOLDS`** when omitted. First matching rule can drive custom UI coloring. | +| Field | Description | +| --------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **`value`** | Aggregated scalar from the configured SQL function (`sum`, `average`, `max`, `min`, or entity count for `count`) over latest non-null metric values. Evaluated by **`options.thresholds`** when present. When **`total`** is **`0`**, **`value`** is also **`0`** (SQL aggregates return null over an empty set); for **`min`** / **`max`**, treat **`total: 0`** as “no data”, not a real extreme of **`0`**. | +| **`total`** | Number of latest rows that contributed to **`value`** (non-null values; calculation failures excluded; optionally narrowed by **`filter.status`**). For **`count`**, equals **`value`**. Use **`total: 0`** to detect an empty contribution set (no owned entities, or filter matched nothing). | +| **`entitiesConsidered`** | In-scope owned entities with **at least one** latest `metric_values` row for this metric (includes calculation-error rows). | +| **`calculationErrorCount`** | Among **`entitiesConsidered`**, how many latest rows are metric calculation failures (`error_message` set and `value` null). | +| **`timestamp`** | ISO timestamp of the most recent among the latest rows in scope (portfolio data freshness. Same merge rule as other aggregation types). When **`filter.status`** matches no rows, still the portfolio freshness — not “now”. | +| **`thresholds`** | Number-style threshold rules for classifying **`value`**; from **`options.thresholds`** or **`DEFAULT_NUMBER_THRESHOLDS`** when omitted. First matching rule can drive custom UI coloring. | -Example (truncated): +Example scalar response with status filter: ```json { - "id": "totalOpenBugs", + "id": "jira.openIssues", "status": "success", "metadata": { - "title": "Total Open Bugs", - "description": "Sum of open issues across owned entities", + "title": "Total Critical Bugs", + "description": "Sum of open issues for entities in error status", "type": "number", - "aggregationType": "sum" + "aggregationType": "sum", + "filter": { "status": "error" } }, "result": { "value": 42, - "total": 8, + "total": 10, "entitiesConsidered": 10, - "calculationErrorCount": 2, - "timestamp": "2026-07-15T10:00:00.000Z", - "thresholds": { "rules": [] } + "calculationErrorCount": 1, + "timestamp": "2026-02-17T10:30:00.000Z", + "thresholds": { + "rules": [ + { "key": "success", "expression": "<100" }, + { "key": "warning", "expression": "100-500" }, + { "key": "error", "expression": ">500" } + ] + } } } ``` -**`entitiesConsidered`** (all types): count of in-scope owned entities that have **at least one** latest `metric_values` row for this metric. **`calculationErrorCount`**: how many of those latest rows are metric calculation failures (`error_message` set and `value` null), so the homepage ratio matches the population behind the drill-down table rather than the raw number of owned catalog refs. For scalar types, **`total`** is the number of rows that contributed to **`value`** (non-null latest values, calculation failures excluded). +**`entitiesConsidered`** (all types): count of in-scope owned entities that have **at least one** latest `metric_values` row for this metric. **`calculationErrorCount`**: how many of those latest rows are metric calculation failures (`error_message` set and `value` null), so the homepage ratio matches the population behind the drill-down table rather than the raw number of owned catalog refs. For scalar types, **`total`** is the number of rows that contributed to **`value`** (non-null latest values, calculation failures excluded, optionally narrowed by **`filter.status`**). **“Without calculation errors” on the homepage:** `healthy = entitiesConsidered - calculationErrorCount` counts only among entities that already have a latest stored row for this metric. Owned entities with **no** row yet are omitted from **`entitiesConsidered`** (same as omitting them from the drill-down list until data exists). @@ -269,7 +291,7 @@ Example (truncated): ### `GET /aggregations/:aggregationId/metadata` -Same resolution as above, but returns only metadata fields (no aggregate counts). Useful for UIs that list KPIs without loading full aggregation data. +Same resolution as above, but returns only metadata fields (no aggregate counts), including **`filter`** when configured on a scalar KPI. Useful for UIs that list KPIs without loading full aggregation data. #### Permissions and errors @@ -281,7 +303,7 @@ Same resolution as above, but returns only metadata fields (no aggregate counts) #### Empty results -When the user owns no relevant entities, the API returns an aggregation with **zero total** and zeroed bucket counts for distribution types, or **`value: 0`** with zeroed entity counts for scalar types (not an error). +When the user owns no relevant entities, the API returns an aggregation with **zero total** and zeroed bucket counts for distribution types, or **`value: 0`** with **`total: 0`** for scalar types (not an error). The same **`value: 0`** / **`total: 0`** shape applies when a scalar **`filter.status`** matches no latest rows — clients should key off **`total: 0`**, especially for **`min`** / **`max`**, where a bare **`value: 0`** would otherwise be ambiguous. ### Drill-down vs aggregation id diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md index 65b21deb850..5b034936983 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md @@ -191,7 +191,9 @@ These thresholds are **not** per-entity metric rules. They apply to homepage agg - **YAML shape:** Same as metric thresholds — a **`rules`** array of **`key`**, **`expression`**, and optional **`color`** (and optional **`icon`**). Expressions are **number**-style and are evaluated against **`result.value`**, the aggregated scalar from the KPI (see [Entity Aggregation — Scalar result fields](./aggregation.md#scalar-result-fields)). The **first** matching rule wins; its **`color`** and **`key`** can be used by custom UIs that render scalar KPIs. -- **Defaults:** If **`thresholds`** is omitted from app-config under **`options`**, **`ScalarAggregationStrategy`** applies **`DEFAULT_NUMBER_THRESHOLDS`** from scorecard-common when serving an aggregation and includes them on the API as **`result.thresholds`**. +- **Defaults:** If **`thresholds`** is omitted from app-config under **`options`**, **`ScalarAggregationStrategy`** applies **`DEFAULT_NUMBER_THRESHOLDS`** from scorecard-common when serving an aggregation and includes them on the API as **`result.thresholds`**: **`<10`** → success, **`10-50`** → warning, **`>50`** → error. + +- **Scalar status filter:** The **`key`** values from metric threshold rules (provider defaults or app-config overrides at **`scorecard.metricProviders...metrics..thresholds`** / provider-level **`scorecard.metricProviders...thresholds`** — see [§1 Provider Default Thresholds](#1-provider-default-thresholds) and [§2 App Configuration Thresholds](#2-app-configuration-thresholds)) are the valid values for scalar KPI **`filter.status`**. Startup validation checks **`filter.status`** against those keys only (not per-entity annotation overrides). See [Entity Aggregation — Status filter (scalar types)](./aggregation.md#status-filter-scalar-types). - **Startup validation:** Invalid rules or expressions are caught when the backend plugin loads, together with the rest of **`scorecard.aggregationKPIs`**. Scalar KPI **`options.thresholds`** must also satisfy **joint full-line coverage** for number expressions when multiple rules apply (see [Joint coverage (number metrics)](#joint-coverage-number-metrics)). See [aggregation.md — Configuration validation](./aggregation.md#configuration-validation). @@ -461,6 +463,6 @@ rules: ## Related documentation -- [Entity Aggregation](./aggregation.md) — ownership, **`GET /aggregations/:aggregationId`**, **`statusGrouped`**, **`weightedStatusScore`**, and scalar types +- [Entity Aggregation](./aggregation.md) — ownership, **`GET /aggregations/:aggregationId`**, **`statusGrouped`**, **`weightedStatusScore`**, scalar types, and **`filter.status`** - [Drill-down](./drill-down.md) — entity list for a metric (`metricId`, not KPI id) - [Scorecard backend README](../README.md) — install, RBAC, **`aggregationKPIs`** examples diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.test.ts index a9990377f7d..5b112214bac 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.test.ts @@ -1646,5 +1646,98 @@ describe('DatabaseMetricValues', () => { expect(result).toBeUndefined(); }, ); + + describe.each(databases.eachSupportedId())( + 'status filter - %p', + databaseId => { + let db: DatabaseMetricValues; + + beforeAll(async () => { + const database = await createDatabase(databaseId); + const { client } = database; + db = database.db; + + await client('metric_values').insert( + [ + createMetricValue({ + entityRef: 'component:default/service1', + value: 10, + status: 'success', + }), + createMetricValue({ + entityRef: 'component:default/service2', + value: 25, + status: 'error', + }), + createMetricValue({ + entityRef: 'component:default/service3', + value: 5, + status: 'error', + }), + createMetricValue({ + entityRef: 'component:default/service4', + value: null, + status: null, + errorMessage: 'Failed to fetch', + }), + ].map(toMetricValueRow), + ); + }); + + const entityRefs = [ + 'component:default/service1', + 'component:default/service2', + 'component:default/service3', + 'component:default/service4', + ]; + + const portfolioCounts = { + latestEntityCount: 4, + calculationErrorCount: 1, + maxTimestamp: baseTimestamp, + }; + + it.each([ + ['sum', 'error', { value: 30, total: 2 }], + ['count', 'error', { value: 2, total: 2 }], + ['max', 'error', { value: 25, total: 2 }], + ['min', 'error', { value: 5, total: 2 }], + ['average', 'error', { value: 15, total: 2 }], + ['sum', 'success', { value: 10, total: 1 }], + ] as const)( + 'should %s only rows matching filter.status=%s', + async (aggregationFn, status, expected) => { + const result = await db.readScalarAggregatedMetricByEntityRefs( + entityRefs, + 'github.metric1', + aggregationFn, + { status }, + ); + + expect(result).toEqual({ + metricId: 'github.metric1', + ...expected, + ...portfolioCounts, + }); + }, + ); + + it('should return zero value and total when no rows match filter.status', async () => { + const result = await db.readScalarAggregatedMetricByEntityRefs( + entityRefs, + 'github.metric1', + 'sum', + { status: 'warning' }, + ); + + expect(result).toEqual({ + metricId: 'github.metric1', + value: 0, + total: 0, + ...portfolioCounts, + }); + }); + }, + ); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts b/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts index 92cc7c89853..63dca5f2eab 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts @@ -15,6 +15,7 @@ */ import { Knex } from 'knex'; +import type { AggregationConfigFilter } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { DbMetricValueCreate, DbMetricValue, @@ -132,6 +133,7 @@ export class DatabaseMetricValues { private async readScalarAggregationByLatestIdsSubquery( latestIdsSubquery: Knex.QueryBuilder, aggregationFn: ScalarAggregationFn, + filter?: AggregationConfigFilter, ): Promise { const clientName: string = (this.dbClient as any).client?.config?.client ?? ''; @@ -150,6 +152,10 @@ export class DatabaseMetricValues { .whereIn('id', latestIdsSubquery) .whereRaw(`NOT ${DatabaseMetricValues.metricValueIsMissingExpr}`); + if (filter?.status && filter.status !== '') { + aggregateQuery.where('status', filter.status); + } + const aggregateRow = await aggregateQuery .select( this.dbClient.raw(`${aggregateExpression} as value`), @@ -165,11 +171,21 @@ export class DatabaseMetricValues { }; if (aggregateRow) { - aggregateResult.value = Number(aggregateRow.value); - aggregateResult.total = Number(aggregateRow.total); - aggregateResult.maxTimestamp = normalizeTimestamp( - aggregateRow.max_timestamp, - ); + const rawValue = Number(aggregateRow.value); + const rawTotal = Number(aggregateRow.total); + aggregateResult.value = Number.isFinite(rawValue) ? rawValue : 0; + aggregateResult.total = Number.isFinite(rawTotal) ? rawTotal : 0; + // MAX(timestamp) is null when no rows contribute (e.g. filter matches nothing). + // Keep epoch so mergeMaxTimestamp prefers the portfolio latest-row timestamp. + if ( + aggregateResult.total > 0 && + aggregateRow.max_timestamp !== null && + aggregateRow.max_timestamp !== '' + ) { + aggregateResult.maxTimestamp = normalizeTimestamp( + aggregateRow.max_timestamp, + ); + } } return { @@ -296,6 +312,7 @@ export class DatabaseMetricValues { catalogEntityRefs: string[], metricId: string, aggregationFn: ScalarAggregationFn, + filter?: AggregationConfigFilter, ): Promise { if (catalogEntityRefs.length === 0) { return undefined; @@ -319,6 +336,7 @@ export class DatabaseMetricValues { } = await this.readScalarAggregationByLatestIdsSubquery( latestIdsSubquery, aggregationFn, + filter, ); const mergedMax = mergeMaxTimestamp( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts b/workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts index 88e03197d0f..1e309f14cc4 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/database/utils/mapMetricValueRow.ts @@ -39,7 +39,7 @@ export type MetricValueRowWithId = MetricValueRow & { entity_namespace: string | null; }; -export type MetricValueCreateInput = Omit< +export type DbMetricValueCreateInput = Omit< DbMetricValueCreate, 'value' | 'errorMessage' | 'entityKind' | 'entityOwner' | 'entityNamespace' > & { @@ -51,7 +51,7 @@ export type MetricValueCreateInput = Omit< }; export function toMetricValueRow( - value: MetricValueCreateInput, + value: DbMetricValueCreateInput, ): MetricValueRow { return { catalog_entity_ref: value.catalogEntityRef, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts index 0f936a6eac2..5f926af2a82 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts @@ -136,6 +136,7 @@ export const scorecardPlugin = createBackendPlugin({ validateAggregationConfig({ rootConfig: config, registry: metricProvidersRegistry, + thresholdResolver, }); Scheduler.create({ diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts index 1ed8d72c9ff..9c44af17ec7 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts @@ -618,7 +618,7 @@ describe('CatalogMetricService', () => { expect(result).toEqual({ values: {}, total: 0, - timestamp: '2024-01-15T12:00:00.000Z', + timestamp: '1970-01-01T00:00:00.000Z', entitiesConsidered: 0, calculationErrorCount: 0, }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.test.ts index beba1e77723..26c8e93b71a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.test.ts @@ -136,6 +136,7 @@ describe('AggregatedMetricLoader', () => { ['component:default/a'], 'metric.id', 'sum', + undefined, ); }); @@ -150,5 +151,21 @@ describe('AggregatedMetricLoader', () => { scalarAggregatedMetric, ); }); + + it('should forward filter to scalar DB read', async () => { + await loader.loadScalarMetricByEntityRefs( + ['component:default/a'], + 'metric.id', + 'sum', + { status: 'error' }, + ); + + expect(readScalarAggregatedMetricByEntityRefs).toHaveBeenCalledWith( + ['component:default/a'], + 'metric.id', + 'sum', + { status: 'error' }, + ); + }); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.ts index 316845d3a30..19b9e475298 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregatedMetricLoader.ts @@ -16,6 +16,7 @@ import type { AggregatedMetric, + AggregationConfigFilter, ScalarAggregatedMetric, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { DatabaseMetricValues } from '../../database/DatabaseMetricValues'; @@ -46,6 +47,7 @@ export class AggregatedMetricLoader { entityRefs: string[], metricId: string, aggregationFn: ScalarAggregationFn, + filter?: AggregationConfigFilter, ): Promise { if (entityRefs.length === 0) { return AggregatedMetricMapper.toScalarAggregatedMetric(); @@ -56,6 +58,7 @@ export class AggregatedMetricLoader { entityRefs, metricId, aggregationFn, + filter, ); return AggregatedMetricMapper.toScalarAggregatedMetric(scalarMetric); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts index 7d9fb10f9d4..3272f22699d 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.test.ts @@ -28,7 +28,7 @@ import { mockWeightedStatusScoreAggregationResult, mockScalarAggregationResult, } from '../../../__fixtures__/mockAggregatedMetricResult'; -import { mockFirstThresholds } from '../../../__fixtures__/mockThresholds'; +import { mockHigherIsBetterThresholds } from '../../../__fixtures__/mockThresholds'; import { buildMockMetricProvidersRegistry } from '../../../__fixtures__/mockMetricProvidersRegistry'; import { createAggregationStrategyRegistry } from './strategies/registerStrategies'; import type { AggregationStrategy } from './strategies/types'; @@ -144,7 +144,7 @@ describe('AggregationsService', () => { const options = { metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: statusGroupedAggregationConfig, }; @@ -159,7 +159,7 @@ describe('AggregationsService', () => { const result = await service.getAggregatedMetricByEntityRefs({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: statusGroupedAggregationConfig, }); @@ -170,7 +170,7 @@ describe('AggregationsService', () => { const options = { metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: weightedAggregationConfig, }; @@ -184,7 +184,7 @@ describe('AggregationsService', () => { const options = { metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: scalarAggregationConfig, }; @@ -199,7 +199,7 @@ describe('AggregationsService', () => { service.getAggregatedMetricByEntityRefs({ metric, entityRefs: [], - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: { id: metric.id, metricId: metric.id, @@ -305,6 +305,45 @@ describe('AggregationsService', () => { ); }); + it('should parse filter.status for scalar KPIs from scorecard.aggregationKPIs', () => { + const config = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalCriticalPrs: { + title: 'Total Critical PRs', + description: 'Sum of open PRs for entities in error status', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + + const serviceWithConfig = new AggregationsService({ + config, + database, + logger, + }); + + expect( + serviceWithConfig.getAggregationConfig( + 'totalCriticalPrs', + metricProvidersRegistry, + ), + ).toEqual( + expect.objectContaining({ + id: 'totalCriticalPrs', + type: aggregationTypes.sum, + filter: { status: 'error' }, + }), + ); + }); + it('should return cached config on repeated lookup for configured KPIs', () => { const config = mockServices.rootConfig({ data: { @@ -348,7 +387,7 @@ describe('AggregationsService', () => { expect(second).toBe(first); }); - it('should not cache fallback config when KPI block is absent', () => { + it('should cache fallback config when KPI block is absent', () => { const first = service.getAggregationConfig( 'github.openPRs', metricProvidersRegistry, @@ -358,10 +397,9 @@ describe('AggregationsService', () => { metricProvidersRegistry, ); - expect(second).toEqual(first); - expect(second).not.toBe(first); - expect(logger.warn).toHaveBeenCalledTimes(2); - expect(metricProvidersRegistry.getMetric).toHaveBeenCalledTimes(2); + expect(second).toBe(first); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(metricProvidersRegistry.getMetric).toHaveBeenCalledTimes(1); }); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts index 9e24f274efb..06c16d36bd6 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/AggregationsService.ts @@ -82,13 +82,16 @@ export class AggregationsService { const metric = metricProviderRegistry.getMetric(aggregationId); - return { + const fallbackConfig: ValidatedAggregationConfig = { id: aggregationId, metricId: aggregationId, title: metric.title, description: metric.description, type: aggregationTypes.statusGrouped, }; + this.aggregationKpisConfigCache.set(aggregationId, fallbackConfig); + + return fallbackConfig; } const validatedConfig = parseValidatedAggregationConfig( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts index ecf1121dddd..598d12b1f4d 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts @@ -56,6 +56,7 @@ export class ScalarAggregationStrategy implements AggregationStrategy { entityRefs, metric.id, this.aggregationFn, + aggregationConfig.filter, ); const result = { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts index e589dbfc934..a5fdd1f765c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts @@ -24,7 +24,7 @@ import { ScalarAggregationStrategy } from './ScalarAggregationStrategy'; import * as aggregationUtils from '../../../utils/aggregation/isScalarAggregationConfig'; import { AggregatedMetricMapper } from '../../mappers'; import { mockScalarAggregationResult } from '../../../../__fixtures__/mockAggregatedMetricResult'; -import { mockFirstThresholds } from '../../../../__fixtures__/mockThresholds'; +import { mockHigherIsBetterThresholds } from '../../../../__fixtures__/mockThresholds'; import { mockGithubOpenPrsMetric } from '../../../../__fixtures__/mockMetric'; jest.mock('../../../utils/aggregation/isScalarAggregationConfig'); @@ -36,7 +36,7 @@ describe('ScalarAggregationStrategy', () => { id: 'totalOpenPrs', metricId: metric.id, options: { - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, }, }); @@ -96,7 +96,7 @@ describe('ScalarAggregationStrategy', () => { strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }), ).rejects.toThrow(/Expected a scalar aggregation config/); @@ -119,7 +119,7 @@ describe('ScalarAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: defaultAggregationConfig, }); @@ -134,7 +134,7 @@ describe('ScalarAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); @@ -142,6 +142,32 @@ describe('ScalarAggregationStrategy', () => { entityRefs, metric.id, 'sum', + undefined, + ); + }); + + it('should forward filter.status to the scalar loader', async () => { + const filteredConfig = mockScalarAggregationConfig(aggregationTypes.sum, { + id: 'totalCriticalPrs', + metricId: metric.id, + filter: { status: 'error' }, + options: { + thresholds: mockHigherIsBetterThresholds, + }, + }); + + await strategy.aggregate({ + metric, + entityRefs, + thresholds: mockHigherIsBetterThresholds, + aggregationConfig: filteredConfig, + }); + + expect(loader.loadScalarMetricByEntityRefs).toHaveBeenCalledWith( + entityRefs, + metric.id, + 'sum', + { status: 'error' }, ); }); @@ -149,13 +175,13 @@ describe('ScalarAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); expect(spyMethods.toAggregatedMetricResultSpy).toHaveBeenCalledWith( metric, - { ...loadedScalarMetric, thresholds: mockFirstThresholds }, + { ...loadedScalarMetric, thresholds: mockHigherIsBetterThresholds }, aggregationConfig, ); }); @@ -164,7 +190,7 @@ describe('ScalarAggregationStrategy', () => { const result = await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); expect(result).toEqual({ diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/statusGroupedAggregationStrategy.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/statusGroupedAggregationStrategy.test.ts index 2052d0f2803..7d446b7a7b2 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/statusGroupedAggregationStrategy.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/statusGroupedAggregationStrategy.test.ts @@ -20,7 +20,7 @@ import { mockStatusGroupedAggregationConfig, } from '../../../../__fixtures__/mockAggregationConfig'; import { mockStatusGroupedAggregationResult } from '../../../../__fixtures__/mockAggregatedMetricResult'; -import { mockFirstThresholds } from '../../../../__fixtures__/mockThresholds'; +import { mockHigherIsBetterThresholds } from '../../../../__fixtures__/mockThresholds'; import { AggregatedMetricMapper } from '../../mappers'; import { AggregatedMetricLoader } from '../AggregatedMetricLoader'; import { StatusGroupedAggregationStrategy } from './StatusGroupedAggregationStrategy'; @@ -51,7 +51,7 @@ describe('StatusGroupedAggregationStrategy', () => { { name: 'success', count: 2 }, { name: 'error', count: 0 }, ], - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, }; const entityRefs = ['component:default/a']; @@ -101,7 +101,7 @@ describe('StatusGroupedAggregationStrategy', () => { strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: invalidAggregationConfig, }), ).rejects.toThrow( @@ -113,7 +113,7 @@ describe('StatusGroupedAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); @@ -127,7 +127,7 @@ describe('StatusGroupedAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); @@ -142,7 +142,7 @@ describe('StatusGroupedAggregationStrategy', () => { const result = await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts index b965da89344..4440f091d92 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts @@ -21,7 +21,7 @@ import { mockWeightedStatusScoreAggregationConfig, } from '../../../../__fixtures__/mockAggregationConfig'; import { mockWeightedStatusScoreAggregationResult } from '../../../../__fixtures__/mockAggregatedMetricResult'; -import { mockFirstThresholds } from '../../../../__fixtures__/mockThresholds'; +import { mockHigherIsBetterThresholds } from '../../../../__fixtures__/mockThresholds'; import { AggregatedMetricMapper } from '../../mappers'; import { AggregatedMetricLoader } from '../AggregatedMetricLoader'; import { WeightedStatusScoreAggregationStrategy } from './WeightedStatusScoreAggregationStrategy'; @@ -52,7 +52,7 @@ describe('WeightedStatusScoreAggregationStrategy', () => { { name: 'success', count: 2, score: 100 }, { name: 'error', count: 0, score: 0 }, ], - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, weightedStatusScore: 100, weightedStatusSum: 200, weightedStatusMaxPossible: 200, @@ -104,7 +104,7 @@ describe('WeightedStatusScoreAggregationStrategy', () => { strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: invalidAggregationConfig, }), ).rejects.toThrow( @@ -132,7 +132,7 @@ describe('WeightedStatusScoreAggregationStrategy', () => { strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: aggregationConfigWithoutColors, }), ).rejects.toThrow( @@ -153,7 +153,7 @@ describe('WeightedStatusScoreAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig: defaultAggregationConfig, }); @@ -168,7 +168,7 @@ describe('WeightedStatusScoreAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); @@ -182,7 +182,7 @@ describe('WeightedStatusScoreAggregationStrategy', () => { await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); @@ -197,7 +197,7 @@ describe('WeightedStatusScoreAggregationStrategy', () => { const result = await strategy.aggregate({ metric, entityRefs, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, aggregationConfig, }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts index c8986edebd1..02b17a2f8c7 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts @@ -176,6 +176,33 @@ describe('AggregatedMetricMapper', () => { aggregationType: aggregationTypes.weightedStatusScore, }); }); + + it('should include filter in metadata for scalar KPI config', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + { + filter: { status: 'error' }, + }, + ); + const result = AggregatedMetricMapper.toAggregationMetadata( + mockMetric, + aggregationConfig, + ); + + expect(result.filter).toEqual({ status: 'error' }); + }); + + it('should omit filter in metadata when scalar KPI config has no filter', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + ); + const result = AggregatedMetricMapper.toAggregationMetadata( + mockMetric, + aggregationConfig, + ); + + expect(result).not.toHaveProperty('filter'); + }); }); describe('toAggregatedMetricResult', () => { @@ -260,38 +287,47 @@ describe('AggregatedMetricMapper', () => { expect((result.result as any).weightedStatusScore).toBe(50); }); - it('should wrap a scalar-shaped result and aggregationType from config', () => { + it('should include filter in metadata for scalar KPI result wrapper', () => { const aggregationConfig = mockScalarAggregationConfig( aggregationTypes.sum, { - id: 'totalOpenPrs', - title: 'Total Open PRs', - description: 'Sum of open PRs', + filter: { status: 'error' }, }, ); const result = AggregatedMetricMapper.toAggregatedMetricResult( mockMetric, { - value: 847, - total: 42, - entitiesConsidered: 45, - calculationErrorCount: 3, + value: 30, + total: 2, + entitiesConsidered: 4, + calculationErrorCount: 1, timestamp: '2024-01-15T10:00:00.000Z', thresholds, }, aggregationConfig, ); - expect(result.metadata.aggregationType).toBe(aggregationTypes.sum); - expect(result.metadata.title).toBe('Total Open PRs'); - expect(result.result).toEqual({ - value: 847, - total: 42, - entitiesConsidered: 45, - calculationErrorCount: 3, - timestamp: '2024-01-15T10:00:00.000Z', - thresholds, - }); + expect(result.metadata.filter).toEqual({ status: 'error' }); + }); + + it('should omit filter in metadata for scalar KPI result wrapper without filter', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + ); + const result = AggregatedMetricMapper.toAggregatedMetricResult( + mockMetric, + { + value: 30, + total: 2, + entitiesConsidered: 4, + calculationErrorCount: 1, + timestamp: '2024-01-15T10:00:00.000Z', + thresholds, + }, + aggregationConfig, + ); + + expect(result.metadata).not.toHaveProperty('filter'); }); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts index e2b23344ef9..f02f4996408 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts @@ -24,15 +24,17 @@ import { } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { DbAggregatedMetric } from '../database/types'; import type { DbScalarAggregatedMetric } from '../database/types'; -import { toIsoTimestamp } from '../utils/toIsoTimestamp'; import { ValidatedAggregationConfig } from '../validation/schemas/aggregationConfigSchemas'; +import { normalizeTimestamp } from '../utils/normalizeTimestamp'; export class AggregatedMetricMapper { static toAggregatedMetric( aggregatedMetric?: DbAggregatedMetric, ): AggregatedMetric { const total = aggregatedMetric?.total ?? 0; - const timestamp = toIsoTimestamp(aggregatedMetric?.maxTimestamp); + const timestamp = normalizeTimestamp( + aggregatedMetric?.maxTimestamp, + ).toISOString(); return { values: aggregatedMetric?.statusCounts ?? {}, @@ -46,7 +48,9 @@ export class AggregatedMetricMapper { static toScalarAggregatedMetric( scalarMetric?: DbScalarAggregatedMetric, ): ScalarAggregatedMetric { - const timestamp = toIsoTimestamp(scalarMetric?.maxTimestamp); + const timestamp = normalizeTimestamp( + scalarMetric?.maxTimestamp, + ).toISOString(); return { value: scalarMetric?.value ?? 0, @@ -61,13 +65,19 @@ export class AggregatedMetricMapper { metric: Metric, aggregationConfig: ValidatedAggregationConfig, ): AggregationMetadata { - return { + const metadata: AggregationMetadata = { type: metric.type, history: metric.history, title: aggregationConfig.title, description: aggregationConfig.description, aggregationType: aggregationConfig.type, }; + + if ('filter' in aggregationConfig && aggregationConfig.filter) { + metadata.filter = aggregationConfig.filter; + } + + return metadata; } static toAggregatedMetricResult( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts index 3b1163c6482..3736a2e53b9 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ConfigReader, Config } from '@backstage/config'; +import { Config } from '@backstage/config'; import { mockErrorHandler, mockServices, @@ -120,7 +120,7 @@ describe('createRouter', () => { beforeEach(async () => { metricProvidersRegistry = new MetricProvidersRegistry(); thresholdResolver = new ThresholdResolver( - new ConfigReader({}), + mockServices.rootConfig({ data: {} }), metricProvidersRegistry.listProviders(), ); const catalog = catalogServiceMock.mock(); @@ -1114,14 +1114,16 @@ describe('createRouter', () => { }); it('should use KPI config metricId and type when aggregationId is a KPI key', async () => { - const kpiConfig = new ConfigReader({ - scorecard: { - aggregationKPIs: { - myKpi: { - title: 'Custom KPI title', - description: 'Custom KPI description', - type: 'statusGrouped', - metricId: 'github.openPRs', + const kpiConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + myKpi: { + title: 'Custom KPI title', + description: 'Custom KPI description', + type: 'statusGrouped', + metricId: 'github.openPRs', + }, }, }, }, @@ -1173,19 +1175,21 @@ describe('createRouter', () => { }); it('should use KPI type weightedStatusScore when configured', async () => { - const kpiConfig = new ConfigReader({ - scorecard: { - aggregationKPIs: { - weightedKpi: { - title: 'Weighted health KPI', - description: 'Weighted status score', - type: 'weightedStatusScore', - metricId: 'github.openPRs', - options: { - statusScores: { - error: 0, - warning: 50, - success: 100, + const kpiConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + weightedKpi: { + title: 'Weighted health KPI', + description: 'Weighted status score', + type: 'weightedStatusScore', + metricId: 'github.openPRs', + options: { + statusScores: { + error: 0, + warning: 50, + success: 100, + }, }, }, }, @@ -1239,14 +1243,16 @@ describe('createRouter', () => { }); it('should use KPI type sum and return scalar result', async () => { - const kpiConfig = new ConfigReader({ - scorecard: { - aggregationKPIs: { - totalOpenPrs: { - title: 'Total Open PRs', - description: 'Sum of open PRs', - type: aggregationTypes.sum, - metricId: 'github.openPRs', + const kpiConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalOpenPrs: { + title: 'Total Open PRs', + description: 'Sum of open PRs', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + }, }, }, }, @@ -1310,6 +1316,7 @@ describe('createRouter', () => { ['component:default/my-service', 'component:default/my-other-service'], 'github.openPRs', 'sum', + undefined, ); expect(response.body.metadata.aggregationType).toBe(aggregationTypes.sum); expect(response.body.result).toEqual({ @@ -1321,6 +1328,94 @@ describe('createRouter', () => { thresholds: DEFAULT_NUMBER_THRESHOLDS, }); }); + + it('should use KPI filter.status and return filtered scalar result', async () => { + const kpiConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalCriticalBugs: { + title: 'Total Critical Bugs', + description: 'Sum for entities in error status', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + + const kpiService = new CatalogMetricService({ + catalog: mockCatalog, + auth: mockServices.auth.mock({ + getOwnServiceCredentials: jest.fn().mockResolvedValue({ + token: 'test-token', + }), + }), + registry: metricRegistry, + database: mockDatabaseMetricValues, + logger: mockServices.logger.mock(), + thresholdResolver, + }); + + const getSpy = jest + .spyOn( + mockDatabaseMetricValues, + 'readScalarAggregatedMetricByEntityRefs', + ) + .mockResolvedValue({ + metricId: 'jira.openIssues', + value: 30, + total: 2, + latestEntityCount: 4, + calculationErrorCount: 1, + maxTimestamp: new Date('2025-01-01T10:30:00.000Z'), + }); + + jest + .spyOn(AggregatedMetricMapper, 'toAggregatedMetricResult') + .mockRestore(); + + const aggregationsServiceFiltered = createTestAggregationsService( + mockDatabaseMetricValues, + kpiConfig, + ); + + const router = await createRouter({ + metricProvidersRegistry: metricRegistry, + service: { + aggregationsService: aggregationsServiceFiltered, + catalogMetricService: kpiService, + }, + catalog: mockCatalog, + httpAuth: httpAuthMock, + permissions: permissionsMock, + logger: mockServices.logger.mock(), + thresholdResolver, + }); + const kpiApp = express(); + kpiApp.use(router); + kpiApp.use(mockErrorHandler()); + + const response = await request(kpiApp).get( + '/aggregations/totalCriticalBugs', + ); + + expect(response.status).toBe(200); + expect(getSpy).toHaveBeenCalledWith( + ['component:default/my-service', 'component:default/my-other-service'], + 'jira.openIssues', + 'sum', + { status: 'error' }, + ); + expect(response.body.metadata.filter).toEqual({ status: 'error' }); + expect(response.body.result.value).toBe(30); + expect(response.body.result.total).toBe(2); + expect(response.body.result.entitiesConsidered).toBe(4); + }); }); describe('GET /aggregations/:aggregationId/metadata', () => { @@ -1337,14 +1432,16 @@ describe('createRouter', () => { metaCatalog = catalogServiceMock.mock(); - const metaAggregationConfig = new ConfigReader({ - scorecard: { - aggregationKPIs: { - myKpi: { - title: 'Custom KPI title', - description: 'Custom KPI description', - type: 'statusGrouped', - metricId: 'github.openPRs', + const metaAggregationConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + myKpi: { + title: 'Custom KPI title', + description: 'Custom KPI description', + type: 'statusGrouped', + metricId: 'github.openPRs', + }, }, }, }, @@ -1482,6 +1579,101 @@ describe('createRouter', () => { expect(response.body.title).toBe('GitHub Open PRs'); expect(response.body.aggregationType).toBe('statusGrouped'); }); + + it('should include filter in metadata for scalar KPI with filter.status', async () => { + const filteredMetaConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalCriticalBugs: { + title: 'Total Critical Bugs', + description: 'Sum for entities in error status', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + + const aggregationsMetaServiceFiltered = createTestAggregationsService( + mockDatabaseMetricValues as unknown as DatabaseMetricValues, + filteredMetaConfig, + ); + + const router = await createRouter({ + metricProvidersRegistry: metaRegistry, + service: { + aggregationsService: aggregationsMetaServiceFiltered, + catalogMetricService: metaCatalogMetricService, + }, + catalog: metaCatalog, + httpAuth: httpAuthMock, + permissions: permissionsMock, + logger: mockServices.logger.mock(), + thresholdResolver, + }); + const filteredMetaApp = express(); + filteredMetaApp.use(router); + filteredMetaApp.use(mockErrorHandler()); + + const response = await request(filteredMetaApp).get( + '/aggregations/totalCriticalBugs/metadata', + ); + + expect(response.status).toBe(200); + expect(response.body.filter).toEqual({ status: 'error' }); + expect(response.body.aggregationType).toBe(aggregationTypes.sum); + }); + + it('should omit filter in metadata for scalar KPI without filter', async () => { + const scalarMetaConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalOpenPrs: { + title: 'Total Open PRs', + description: 'Sum of open PRs', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + }, + }, + }, + }, + }); + + const aggregationsMetaServiceScalar = createTestAggregationsService( + mockDatabaseMetricValues as unknown as DatabaseMetricValues, + scalarMetaConfig, + ); + + const router = await createRouter({ + metricProvidersRegistry: metaRegistry, + service: { + aggregationsService: aggregationsMetaServiceScalar, + catalogMetricService: metaCatalogMetricService, + }, + catalog: metaCatalog, + httpAuth: httpAuthMock, + permissions: permissionsMock, + logger: mockServices.logger.mock(), + thresholdResolver, + }); + const scalarMetaApp = express(); + scalarMetaApp.use(router); + scalarMetaApp.use(mockErrorHandler()); + + const response = await request(scalarMetaApp).get( + '/aggregations/totalOpenPrs/metadata', + ); + + expect(response.status).toBe(200); + expect(response.body).not.toHaveProperty('filter'); + expect(response.body.aggregationType).toBe(aggregationTypes.sum); + }); }); describe('GET /metrics/:metricId/catalog/aggregations/entities', () => { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.test.ts index 7d5efdcd5d6..5fc73a56c31 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.test.ts @@ -19,8 +19,8 @@ import { aggregationTypes } from '@red-hat-developer-hub/backstage-plugin-scorec import { AGGREGATION_KPIS_CONFIG_PATH } from '../../constants'; import { buildAggregationConfig } from './buildAggregationConfig'; import { - mockFirstThresholds, - mockSecondThresholds, + mockHigherIsBetterThresholds, + mockLowerIsBetterThresholds, } from '../../../__fixtures__/mockThresholds'; describe('buildAggregationConfig', () => { @@ -108,7 +108,7 @@ describe('buildAggregationConfig', () => { metricId: 'github.openPRs', options: { statusScores: { success: 100, warning: 50, error: 0 }, - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, }, }, }, @@ -121,7 +121,7 @@ describe('buildAggregationConfig', () => { const result = buildAggregationConfig('weightedKpi', { config }); - expect(result.options?.thresholds).toEqual(mockFirstThresholds); + expect(result.options?.thresholds).toEqual(mockHigherIsBetterThresholds); }); it('should map optional thresholds for scalar KPIs', () => { @@ -135,7 +135,7 @@ describe('buildAggregationConfig', () => { type: aggregationTypes.sum, metricId: 'github.openPRs', options: { - thresholds: mockSecondThresholds, + thresholds: mockLowerIsBetterThresholds, }, }, }, @@ -148,7 +148,7 @@ describe('buildAggregationConfig', () => { const result = buildAggregationConfig('totalOpenPrsKpi', { config }); - expect(result.options?.thresholds).toEqual(mockSecondThresholds); + expect(result.options?.thresholds).toEqual(mockLowerIsBetterThresholds); }); it('should not map thresholds for statusGrouped KPIs even when configured', () => { @@ -162,7 +162,7 @@ describe('buildAggregationConfig', () => { type: aggregationTypes.statusGrouped, metricId: 'github.openPRs', options: { - thresholds: mockFirstThresholds, + thresholds: mockHigherIsBetterThresholds, }, }, }, @@ -217,5 +217,130 @@ describe('buildAggregationConfig', () => { thresholds: undefined, }, }); + expect(result.filter).toBeUndefined(); + }); + + it('should map filter.status for scalar KPIs', () => { + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalCriticalPrs: { + title: 'Total Critical PRs', + description: 'Sum of open PRs for entities in error status', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + const config = rootConfig.getConfig( + `${AGGREGATION_KPIS_CONFIG_PATH}.totalCriticalPrs`, + ); + + const result = buildAggregationConfig('totalCriticalPrs', { config }); + + expect(result.filter).toEqual({ status: 'error' }); + }); + + it('should omit filter when filter block has no properties', () => { + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalOpenPrsKpi: { + title: 'Total Open PRs', + description: 'Sum of open PRs', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + filter: {}, + }, + }, + }, + }, + }); + const config = rootConfig.getConfig( + `${AGGREGATION_KPIS_CONFIG_PATH}.totalOpenPrsKpi`, + ); + + const result = buildAggregationConfig('totalOpenPrsKpi', { config }); + + expect(result).toEqual({ + id: 'totalOpenPrsKpi', + title: 'Total Open PRs', + description: 'Sum of open PRs', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + options: { + thresholds: undefined, + }, + }); + expect(result.filter).toBeUndefined(); + expect('filter' in result).toBe(false); + }); + + it('should ignore filter for statusGrouped KPIs even when configured', () => { + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + statusKpi: { + title: 'Status breakdown', + description: 'Counts by status', + type: aggregationTypes.statusGrouped, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + const config = rootConfig.getConfig( + `${AGGREGATION_KPIS_CONFIG_PATH}.statusKpi`, + ); + + const result = buildAggregationConfig('statusKpi', { config }); + + expect(result.filter).toBeUndefined(); + }); + + it('should ignore filter for weightedStatusScore KPIs even when configured', () => { + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + weightedKpi: { + title: 'Weighted health', + description: 'Weighted health score across statuses', + type: aggregationTypes.weightedStatusScore, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + options: { + statusScores: { + error: 0, + warning: 50, + success: 100, + }, + }, + }, + }, + }, + }, + }); + const config = rootConfig.getConfig( + `${AGGREGATION_KPIS_CONFIG_PATH}.weightedKpi`, + ); + + const result = buildAggregationConfig('weightedKpi', { config }); + + expect(result.filter).toBeUndefined(); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts index 6a87b551fb6..a7f2675115b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfig.ts @@ -22,6 +22,7 @@ import { import { isScalarAggregationType } from './isScalarAggregationType'; import { buildAggregationConfigThresholds } from './buildAggregationConfigThresholds'; import { buildAggregationStatusScores } from './buildAggregationStatusScores'; +import { buildAggregationConfigFilter } from './buildAggregationConfigFilter'; type Options = { config: Config; @@ -33,6 +34,7 @@ export function buildAggregationConfig( ): AggregationConfig { const { config } = options; + // Represents base attributes for all aggregation types. const aggregationConfig: AggregationConfig = { id: aggregationId, type: config.getString('type'), @@ -51,6 +53,10 @@ export function buildAggregationConfig( } else if (isScalarAggregationType(aggregationConfig.type)) { aggregationConfig.options.thresholds = buildAggregationConfigThresholds(config); + const filter = buildAggregationConfigFilter(config); + if (filter) { + aggregationConfig.filter = filter; + } } return aggregationConfig; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.test.ts new file mode 100644 index 00000000000..e717c136db2 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.test.ts @@ -0,0 +1,92 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { mockServices } from '@backstage/backend-test-utils'; +import { aggregationTypes } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { AGGREGATION_KPIS_CONFIG_PATH } from '../../constants'; +import { buildAggregationConfigFilter } from './buildAggregationConfigFilter'; + +describe('buildAggregationConfigFilter', () => { + it('should return undefined when filter is absent', () => { + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalOpenPrsKpi: { + title: 'Total Open PRs', + description: 'Sum of open PRs', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + }, + }, + }, + }, + }); + const config = rootConfig.getConfig( + `${AGGREGATION_KPIS_CONFIG_PATH}.totalOpenPrsKpi`, + ); + + expect(buildAggregationConfigFilter(config)).toBeUndefined(); + }); + + it('should return undefined when filter has no properties', () => { + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalOpenPrsKpi: { + title: 'Total Open PRs', + description: 'Sum of open PRs', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + filter: {}, + }, + }, + }, + }, + }); + const config = rootConfig.getConfig( + `${AGGREGATION_KPIS_CONFIG_PATH}.totalOpenPrsKpi`, + ); + + expect(buildAggregationConfigFilter(config)).toBeUndefined(); + }); + + it('should map filter.status from KPI config', () => { + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalCriticalPrs: { + title: 'Total Critical PRs', + description: 'Sum of open PRs for entities in error status', + type: aggregationTypes.sum, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + const config = rootConfig.getConfig( + `${AGGREGATION_KPIS_CONFIG_PATH}.totalCriticalPrs`, + ); + + expect(buildAggregationConfigFilter(config)).toEqual({ status: 'error' }); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts new file mode 100644 index 00000000000..9e94233f598 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/buildAggregationConfigFilter.ts @@ -0,0 +1,34 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Config } from '@backstage/config'; +import type { AggregationConfigFilter } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; + +export function buildAggregationConfigFilter( + config: Config, +): AggregationConfigFilter | undefined { + const filter = config.getOptionalConfig('filter'); + if (!filter) { + return undefined; + } + + const status = filter.getOptionalString('status'); + if (!status) { + return undefined; + } + + return { status }; +} diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.test.ts index d51756b8624..4ce631a7aa5 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.test.ts @@ -44,7 +44,15 @@ describe('normalizeTimestamp', () => { ); }); - it('should return current time when input is undefined', () => { - expect(normalizeTimestamp(undefined)).toEqual(new Date(dateString)); + it('should return epoch when input is undefined', () => { + expect(normalizeTimestamp(undefined)).toEqual(new Date(0)); + }); + + it('should return epoch when input is null', () => { + expect(normalizeTimestamp(null)).toEqual(new Date(0)); + }); + + it('should return epoch when input is an empty string', () => { + expect(normalizeTimestamp('')).toEqual(new Date(0)); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts index ba68cc3b026..ec32e15bd99 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/normalizeTimestamp.ts @@ -20,8 +20,11 @@ export function normalizeTimestamp(timestamp?: unknown): Date { } if (typeof timestamp === 'number' || typeof timestamp === 'string') { + if (timestamp === '') { + return new Date(0); + } return new Date(timestamp); } - return new Date(); + return new Date(0); } diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/toIsoTimestamp.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/toIsoTimestamp.test.ts deleted file mode 100644 index d511f63ad28..00000000000 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/toIsoTimestamp.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright Red Hat, Inc. - * - * 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 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { toIsoTimestamp } from './toIsoTimestamp'; - -describe('toIsoTimestamp', () => { - const dateString = '2023-01-01T00:00:00.000Z'; - - beforeEach(() => { - jest.useFakeTimers().setSystemTime(new Date(dateString)); - }); - - afterEach(() => { - jest.useRealTimers(); - }); - - it('should return ISO string for a Date input', () => { - const timestamp = new Date('2024-06-15T12:30:00.000Z'); - - expect(toIsoTimestamp(timestamp)).toBe('2024-06-15T12:30:00.000Z'); - }); - - it('should return current time as ISO string when input is undefined', () => { - expect(toIsoTimestamp(undefined)).toBe(dateString); - }); -}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationConfigSchemas.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationConfigSchemas.ts index fb241c17bbc..e216a7271ab 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationConfigSchemas.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationConfigSchemas.ts @@ -19,16 +19,8 @@ import { aggregationTypes, scalarAggregationTypes, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; - -const thresholdsConfigSchema = z.object({ - rules: z.array( - z.object({ - key: z.string(), - expression: z.string(), - color: z.string().optional(), - }), - ), -}); +import { aggregationThresholdsConfigSchema } from './aggregationThresholdsConfigSchema'; +import { aggregationFilterSchema } from './aggregationFilterSchema'; const baseAggregationConfigSchema = z.object({ id: z.string().min(1).max(128), @@ -51,7 +43,7 @@ const weightedStatusScoreAggregationConfigSchema = z.object({ .refine(scores => Object.keys(scores).length > 0, { message: 'options.statusScores must contain at least one weight value', }), - thresholds: thresholdsConfigSchema.optional(), + thresholds: aggregationThresholdsConfigSchema.optional(), }), }); @@ -61,9 +53,10 @@ function scalarAggregationConfigSchema( return z.object({ ...baseAggregationConfigSchema.shape, type: z.literal(type), + filter: aggregationFilterSchema.optional(), options: z .strictObject({ - thresholds: thresholdsConfigSchema.optional(), + thresholds: aggregationThresholdsConfigSchema.optional(), }) .optional(), }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/toIsoTimestamp.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationFilterSchema.ts similarity index 79% rename from workspaces/scorecard/plugins/scorecard-backend/src/utils/toIsoTimestamp.ts rename to workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationFilterSchema.ts index 0407eab7299..e01d4d9d656 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/toIsoTimestamp.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationFilterSchema.ts @@ -14,8 +14,8 @@ * limitations under the License. */ -export function toIsoTimestamp(timestamp?: Date): string { - return timestamp - ? new Date(timestamp).toISOString() - : new Date().toISOString(); -} +import { z } from 'zod'; + +export const aggregationFilterSchema = z.object({ + status: z.string().min(1).max(64).optional(), +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationThresholdsConfigSchema.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationThresholdsConfigSchema.ts new file mode 100644 index 00000000000..40fa60019ac --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/schemas/aggregationThresholdsConfigSchema.ts @@ -0,0 +1,27 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { z } from 'zod'; + +export const aggregationThresholdsConfigSchema = z.object({ + rules: z.array( + z.object({ + key: z.string(), + expression: z.string(), + color: z.string().optional(), + }), + ), +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts index 83edfdc9767..62b83efdace 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.test.ts @@ -14,14 +14,12 @@ * limitations under the License. */ -import { mockServices } from '@backstage/backend-test-utils'; +import type { Config } from '@backstage/config'; import { InputError } from '@backstage/errors'; -import { - aggregationTypes, - scalarAggregationTypes, - ScorecardThresholdRuleColors, -} from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { aggregationTypes } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { validateAggregationConfig } from './validateAggregationConfig'; +import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; import { MockBooleanProvider, MockNumberProvider, @@ -31,61 +29,32 @@ import { buildMockMetricProvidersRegistry, mockMetricProvidersRegistry, } from '../../__fixtures__/mockMetricProvidersRegistry'; +import { mockServices } from '@backstage/backend-test-utils'; -const validWeightedThresholds = { - rules: [ - { - key: 'success', - expression: '>=75', - color: ScorecardThresholdRuleColors.SUCCESS, - }, - { - key: 'warning', - expression: '10-75', - color: ScorecardThresholdRuleColors.WARNING, - }, - { - key: 'error', - expression: '<10', - color: ScorecardThresholdRuleColors.ERROR, - }, - ], -}; - -const gapThresholds = { - rules: [ - { - key: 'success', - expression: '<10', - color: ScorecardThresholdRuleColors.SUCCESS, - }, - { - key: 'error', - expression: '>20', - color: ScorecardThresholdRuleColors.ERROR, - }, - ], -}; +function validateConfig( + rootConfig: Config, + registry: MetricProvidersRegistry, +): void { + const thresholdResolver = new ThresholdResolver( + rootConfig, + registry.listProviders(), + ); + validateAggregationConfig({ rootConfig, registry, thresholdResolver }); +} describe('validateAggregationConfig', () => { - const numberRegistry = buildMockMetricProvidersRegistry({ - provider: new MockNumberProvider('github.openPRs', 'github'), - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - it('should not throw error when scorecard.aggregationKPIs is absent', () => { - expect(() => - validateAggregationConfig({ - rootConfig: mockServices.rootConfig({ data: {} }), - registry: mockMetricProvidersRegistry, - }), - ).not.toThrow(); + const rootConfig = mockServices.rootConfig({}); + const registry = buildMockMetricProvidersRegistry({}); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); }); it('should not throw when all KPI entries are valid and metrics are registered', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { @@ -101,13 +70,15 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).not.toThrow(); + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); }); it('should throw InputError when a KPI entry fails schema validation', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); const tooLong = 'a'.repeat(256); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { @@ -123,12 +94,14 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).toThrow(InputError); + expect(() => validateConfig(rootConfig, registry)).toThrow(InputError); }); it('should throw InputError when aggregation type is invalid', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { @@ -144,9 +117,7 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).toThrow(InputError); + expect(() => validateConfig(rootConfig, registry)).toThrow(InputError); }); it('should throw when metric provider for metricId is not registered', () => { @@ -166,10 +137,7 @@ describe('validateAggregationConfig', () => { }); expect(() => - validateAggregationConfig({ - rootConfig, - registry: mockMetricProvidersRegistry, - }), + validateConfig(rootConfig, mockMetricProvidersRegistry), ).toThrow( new Error( `Metric provider with ID 'github.openPRs' is not registered (${AGGREGATION_KPIS_CONFIG_PATH}.openPrsKpi).`, @@ -177,7 +145,11 @@ describe('validateAggregationConfig', () => { ); }); - it('should not throw when weightedStatusScore KPI has options.statusScores', () => { + it('should not throw when weightedStatusScore KPI has options.statusScores (app-config shape)', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { @@ -200,18 +172,20 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).not.toThrow(); + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); }); - it('should throw InputError when type is weightedStatusScore but required options block is missing', () => { + it('should throw when type is weightedStatusScore but required options block is missing', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { aggregationKPIs: { weightedKpi: { - title: 'Weighted status score KPI', + title: 'Avg KPI', type: aggregationTypes.weightedStatusScore, description: 'Weighted health score', metricId: 'github.openPRs', @@ -221,18 +195,22 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).toThrow(/options\.statusScores must contain at least one weight value/); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /options.statusScores must contain at least one weight value for attribute \"options.statusScores\"/, + ); }); it('should throw InputError when type is weightedStatusScore but options.statusScores is empty', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { aggregationKPIs: { weightedKpi: { - title: 'Weighted status score KPI', + title: 'Avg KPI', type: aggregationTypes.weightedStatusScore, description: 'Weighted health score', metricId: 'github.openPRs', @@ -243,24 +221,40 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).toThrow(InputError); + expect(() => validateConfig(rootConfig, registry)).toThrow(InputError); }); it('should not throw when weightedStatusScore KPI includes optional thresholds', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { aggregationKPIs: { weightedKpi: { - title: 'Weighted status score KPI', + title: 'Avg KPI', type: aggregationTypes.weightedStatusScore, description: 'Weighted health score', metricId: 'github.openPRs', options: { statusScores: { success: 100, warning: 50, error: 0 }, - thresholds: validWeightedThresholds, + thresholds: { + rules: [ + { + key: 'success', + expression: '>=75', + color: 'success.main', + }, + { + key: 'warning', + expression: '10-75', + color: 'warning.main', + }, + { key: 'error', expression: '<10', color: 'error.main' }, + ], + }, }, }, }, @@ -268,18 +262,20 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).not.toThrow(); + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); }); it('should throw when thresholds has an invalid expression', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { aggregationKPIs: { weightedKpi: { - title: 'Weighted status score KPI', + title: 'Avg KPI', type: aggregationTypes.weightedStatusScore, description: 'Weighted health score', metricId: 'github.openPRs', @@ -290,7 +286,7 @@ describe('validateAggregationConfig', () => { { key: 'success', expression: '%%%invalid%%%', - color: ScorecardThresholdRuleColors.SUCCESS, + color: 'success.main', }, ], }, @@ -301,24 +297,46 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).toThrow(/Invalid thresholds configuration|Invalid threshold expression/); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /Invalid thresholds configuration|Invalid threshold expression/, + ); }); it('should throw when weightedStatusScore KPI thresholds leave a gap on the number line', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { aggregationKPIs: { weightedKpi: { - title: 'Weighted status score KPI', + title: 'Avg KPI', type: aggregationTypes.weightedStatusScore, description: 'Weighted health score', metricId: 'github.openPRs', options: { statusScores: { success: 100, warning: 50, error: 0 }, - thresholds: gapThresholds, + thresholds: { + rules: [ + { + key: 'success', + expression: '<10', + color: 'success.main', + }, + { + key: 'warning', + expression: '11-20', + color: 'warning.main', + }, + { + key: 'error', + expression: '>20', + color: 'error.main', + }, + ], + }, }, }, }, @@ -326,64 +344,119 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).toThrow(/do not cover the entire real line/); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /do not cover the entire real line/, + ); }); - it.each(scalarAggregationTypes)( - 'should not throw when scalar %s KPI is valid for a number metric', - type => { - const rootConfig = mockServices.rootConfig({ - data: { - scorecard: { - aggregationKPIs: { - scalarKpi: { - title: 'Scalar KPI', - description: 'Scalar aggregation', - type, - metricId: 'github.openPRs', - }, + it('should not throw when scalar sum KPI is valid for a number metric', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalOpenPrs: { + title: 'Total Open PRs', + description: 'Sum of open PRs', + type: aggregationTypes.sum, + metricId: 'github.openPRs', }, }, }, - }); + }, + }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).not.toThrow(); - }, - ); + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); + }); + + it('should throw InputError when sum KPI targets a boolean metric', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockBooleanProvider('jira.license', 'jira'), + }); - it.each(scalarAggregationTypes)( - 'should throw when scalar %s KPI targets a boolean metric', + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + badSumKpi: { + title: 'Bad sum', + description: 'Sum on boolean metric', + type: aggregationTypes.sum, + metricId: 'jira.license', + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).toThrow(InputError); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /requires a number metric/, + ); + }); + + it('should not throw when count KPI targets a boolean metric', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockBooleanProvider('jira.license', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + licenseCount: { + title: 'License count', + description: 'Count entities with license metric', + type: aggregationTypes.count, + metricId: 'jira.license', + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); + }); + + it.each([ + aggregationTypes.average, + aggregationTypes.max, + aggregationTypes.min, + aggregationTypes.count, + ])( + 'should not throw when scalar %s KPI is valid for a number metric', type => { const registry = buildMockMetricProvidersRegistry({ - provider: new MockBooleanProvider('jira.license', 'jira'), + provider: new MockNumberProvider('github.openPRs', 'github'), }); const rootConfig = mockServices.rootConfig({ data: { scorecard: { aggregationKPIs: { - badScalarKpi: { - title: 'Bad scalar', - description: 'Scalar on boolean metric', + scalarKpi: { + title: 'Scalar KPI', + description: 'Scalar aggregation', type, - metricId: 'jira.license', + metricId: 'github.openPRs', }, }, }, }, }); - expect(() => validateAggregationConfig({ rootConfig, registry })).toThrow( - /requires a number metric/, - ); + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); }, ); it('should throw when scalar KPI thresholds leave a gap on the real line', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + const rootConfig = mockServices.rootConfig({ data: { scorecard: { @@ -394,7 +467,20 @@ describe('validateAggregationConfig', () => { type: aggregationTypes.sum, metricId: 'github.openPRs', options: { - thresholds: gapThresholds, + thresholds: { + rules: [ + { + key: 'success', + expression: '<10', + color: 'success.main', + }, + { + key: 'error', + expression: '>20', + color: 'error.main', + }, + ], + }, }, }, }, @@ -402,8 +488,317 @@ describe('validateAggregationConfig', () => { }, }); - expect(() => - validateAggregationConfig({ rootConfig, registry: numberRegistry }), - ).toThrow(/do not cover the entire real line/); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /do not cover the entire real line/, + ); + }); + + it('should accept scalar KPI with filter.status', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('jira.openIssues', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + totalCriticalBugs: { + title: 'Total Critical Bugs', + description: 'Sum for entities in error status', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); + }); + + it('should reject scalar KPI with filter.status not in metric threshold keys', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('jira.openIssues', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + badFilterStatusKpi: { + title: 'Bad filter status', + description: 'Unknown threshold key', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: { + status: 'critical', + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).toThrow(InputError); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /filter\.status "critical" is not a threshold rule key.*Valid keys: error, warning, success/, + ); + }); + + it('should accept scalar KPI filter.status matching app-config threshold override keys', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('jira.openIssues', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + metricProviders: { + jira: { + openIssues: { + thresholds: { + rules: [ + { + key: 'high', + expression: '>100', + color: 'error.main', + icon: 'scorecardErrorStatusIcon', + }, + { + key: 'low', + expression: '<=100', + color: 'success.main', + icon: 'scorecardSuccessStatusIcon', + }, + ], + }, + }, + }, + }, + aggregationKPIs: { + highIssuesKpi: { + title: 'High issues', + description: 'Sum for high threshold bucket', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: { + status: 'high', + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); + }); + + it('should reject scalar KPI filter.status when app-config thresholds omit the key', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('jira.openIssues', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + metricProviders: { + jira: { + openIssues: { + thresholds: { + rules: [ + { + key: 'high', + expression: '>100', + color: 'error.main', + icon: 'scorecardErrorStatusIcon', + }, + { + key: 'low', + expression: '<=100', + color: 'success.main', + icon: 'scorecardSuccessStatusIcon', + }, + ], + }, + }, + }, + }, + aggregationKPIs: { + badFilterKpi: { + title: 'Error filter on custom thresholds', + description: 'Default error key not in override', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).toThrow(InputError); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /filter\.status "error" is not a threshold rule key.*Valid keys: high, low/, + ); + }); + + it('should accept scalar KPI with empty filter object when status is omitted', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('jira.openIssues', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + badFilterKpi: { + title: 'Bad filter KPI', + description: 'Missing filter.status', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: {}, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); + }); + + it('should reject scalar KPI with filter.status longer than 64 characters', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('jira.openIssues', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + longFilterKpi: { + title: 'Long filter status', + description: 'Filter status exceeds max length', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: { + status: 'a'.repeat(65), + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).toThrow(InputError); + expect(() => validateConfig(rootConfig, registry)).toThrow( + /at most 64 character/, + ); + }); + + it('should ignore filter on statusGrouped KPIs', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + statusKpi: { + title: 'Status breakdown', + description: 'Counts by status', + type: aggregationTypes.statusGrouped, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); + }); + + it('should ignore filter on weightedStatusScore KPIs', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('github.openPRs', 'github'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + weightedKpi: { + title: 'Weighted health', + description: 'Weighted health score across statuses', + type: aggregationTypes.weightedStatusScore, + metricId: 'github.openPRs', + filter: { + status: 'error', + }, + options: { + statusScores: { + error: 0, + warning: 50, + success: 100, + }, + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); + }); + + it('should accept scalar KPI with filter.status and valid options.thresholds', () => { + const registry = buildMockMetricProvidersRegistry({ + provider: new MockNumberProvider('jira.openIssues', 'jira'), + }); + + const rootConfig = mockServices.rootConfig({ + data: { + scorecard: { + aggregationKPIs: { + filteredThresholdKpi: { + title: 'Filtered with thresholds', + description: 'Scalar KPI with filter and custom thresholds', + type: aggregationTypes.sum, + metricId: 'jira.openIssues', + filter: { + status: 'error', + }, + options: { + thresholds: { + rules: [ + { + key: 'success', + expression: '>=75', + color: 'success.main', + }, + { + key: 'warning', + expression: '10-75', + color: 'warning.main', + }, + { key: 'error', expression: '<10', color: 'error.main' }, + ], + }, + }, + }, + }, + }, + }, + }); + + expect(() => validateConfig(rootConfig, registry)).not.toThrow(); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.ts index 04f7d63e5d6..edcb040f22e 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateAggregationConfig.ts @@ -14,18 +14,22 @@ * limitations under the License. */ +import { InputError } from '@backstage/errors'; import type { Config } from '@backstage/config'; import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; import { AGGREGATION_KPIS_CONFIG_PATH } from '../constants'; -import { buildAggregationConfig } from '../utils/aggregation/buildAggregationConfig'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; +import { validateScalarAggregationConfig } from './validateScalarAggregationConfig'; import { parseValidatedAggregationConfig } from '../utils/aggregation/parseValidatedAggregationConfig'; -import { isScalarAggregationType } from '../utils/aggregation/isScalarAggregationType'; +import { buildAggregationConfig } from '../utils/aggregation/buildAggregationConfig'; +import { isScalarAggregationConfig } from '../utils/aggregation/isScalarAggregationConfig'; export function validateAggregationConfig(options: { rootConfig: Config; registry: MetricProvidersRegistry; + thresholdResolver: ThresholdResolver; }): void { - const { rootConfig, registry } = options; + const { rootConfig, registry, thresholdResolver } = options; const aggregationKPIsConfig = rootConfig.getOptionalConfig( AGGREGATION_KPIS_CONFIG_PATH, @@ -45,20 +49,18 @@ export function validateAggregationConfig(options: { ); if (!registry.hasProvider(aggregationConfig.metricId)) { - throw new Error( + throw new InputError( `Metric provider with ID '${aggregationConfig.metricId}' is not registered (${AGGREGATION_KPIS_CONFIG_PATH}.${aggregationId}).`, ); } - const metric = registry.getMetric(aggregationConfig.metricId); - - if ( - isScalarAggregationType(aggregationConfig.type) && - metric.type === 'boolean' - ) { - throw new Error( - `Aggregation KPI "${aggregationId}" uses type "${aggregationConfig.type}" which requires a number metric, but "${aggregationConfig.metricId}" is boolean.`, - ); + if (isScalarAggregationConfig(aggregationConfig)) { + validateScalarAggregationConfig({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver, + }); } } } diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts new file mode 100644 index 00000000000..a557b10c4b9 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarAggregationConfig.ts @@ -0,0 +1,53 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InputError } from '@backstage/errors'; +import { aggregationTypes } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; +import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; +import { ScalarAggregationConfig } from './schemas/aggregationConfigSchemas'; +import { validateScalarFilterStatus } from './validateScalarFilterStatus'; + +type ValidateScalarAggregationConfigOptions = { + aggregationConfig: ScalarAggregationConfig; + aggregationId: string; + registry: MetricProvidersRegistry; + thresholdResolver: ThresholdResolver; +}; + +export function validateScalarAggregationConfig( + options: ValidateScalarAggregationConfigOptions, +): void { + const { aggregationConfig, aggregationId, registry, thresholdResolver } = + options; + const metric = registry.getMetric(aggregationConfig.metricId); + + if ( + metric.type === 'boolean' && + aggregationConfig.type !== aggregationTypes.count + ) { + throw new InputError( + `Aggregation KPI "${aggregationId}" uses type "${aggregationConfig.type}" which requires a number metric, but "${aggregationConfig.metricId}" is boolean.`, + ); + } + + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver, + }); +} diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.test.ts new file mode 100644 index 00000000000..0fa4e543f6c --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.test.ts @@ -0,0 +1,190 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InputError } from '@backstage/errors'; +import { mockServices } from '@backstage/backend-test-utils'; +import { aggregationTypes } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { validateScalarFilterStatus } from './validateScalarFilterStatus'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; +import { MockNumberProvider } from '../../__fixtures__/mockProviders'; +import { buildMockMetricProvidersRegistry } from '../../__fixtures__/mockMetricProvidersRegistry'; +import { mockScalarAggregationConfig } from '../../__fixtures__/mockAggregationConfig'; +import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; + +describe('validateScalarFilterStatus', () => { + const aggregationId = 'testFilterStatus'; + const metricId = 'jira.openIssues'; + const provider = new MockNumberProvider(metricId, 'jira'); + const customThresholdsConfig = mockServices.rootConfig({ + data: { + scorecard: { + metricProviders: { + jira: { + openIssues: { + thresholds: { + rules: [ + { + key: 'high', + expression: '>100', + color: 'error.main', + icon: 'scorecardErrorStatusIcon', + }, + { + key: 'low', + expression: '<=100', + color: 'success.main', + icon: 'scorecardSuccessStatusIcon', + }, + ], + }, + }, + }, + }, + }, + }, + }); + + let registry: MetricProvidersRegistry; + + beforeEach(() => { + registry = buildMockMetricProvidersRegistry({ + provider, + }); + }); + + function createThresholdResolver(rootConfig = mockServices.rootConfig({})) { + return new ThresholdResolver(rootConfig, registry.listProviders()); + } + + it('should pass validation when filter is absent', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + { + id: aggregationId, + metricId, + }, + ); + + expect(() => + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver: createThresholdResolver(), + }), + ).not.toThrow(); + }); + + it('should pass when filter.status matches a default threshold key', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + { + id: aggregationId, + metricId, + filter: { status: 'error' }, + }, + ); + + expect(() => + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver: createThresholdResolver(), + }), + ).not.toThrow(); + }); + + it('should throw when filter.status is not a valid threshold key', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + { + id: aggregationId, + metricId, + filter: { status: 'critical' }, + }, + ); + + expect(() => + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver: createThresholdResolver(), + }), + ).toThrow(InputError); + expect(() => + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver: createThresholdResolver(), + }), + ).toThrow( + 'Aggregation KPI "testFilterStatus" filter.status "critical" is not a threshold rule key for metric "jira.openIssues". Valid keys: error, warning, success.', + ); + }); + + it('should pass when filter.status matches an app-config threshold override key', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + { + id: aggregationId, + metricId, + filter: { status: 'high' }, + }, + ); + + expect(() => + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver: createThresholdResolver(customThresholdsConfig), + }), + ).not.toThrow(); + }); + + it('should throw when filter.status is omitted from app-config threshold overrides', () => { + const aggregationConfig = mockScalarAggregationConfig( + aggregationTypes.sum, + { + id: aggregationId, + metricId, + filter: { status: 'error' }, + }, + ); + + expect(() => + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver: createThresholdResolver(customThresholdsConfig), + }), + ).toThrow(InputError); + expect(() => + validateScalarFilterStatus({ + aggregationConfig, + aggregationId, + registry, + thresholdResolver: createThresholdResolver(customThresholdsConfig), + }), + ).toThrow( + /filter\.status "error" is not a threshold rule key.*Valid keys: high, low/, + ); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.ts new file mode 100644 index 00000000000..baed781eab8 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateScalarFilterStatus.ts @@ -0,0 +1,48 @@ +/* + * Copyright Red Hat, Inc. + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InputError } from '@backstage/errors'; +import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; +import { ScalarAggregationConfig } from './schemas/aggregationConfigSchemas'; + +export function validateScalarFilterStatus(options: { + aggregationConfig: ScalarAggregationConfig; + aggregationId: string; + registry: MetricProvidersRegistry; + thresholdResolver: ThresholdResolver; +}): void { + const { aggregationConfig, aggregationId, registry, thresholdResolver } = + options; + const status = aggregationConfig.filter?.status; + + if (!status) { + return; + } + + const metric = registry.getMetric(aggregationConfig.metricId); + const thresholds = thresholdResolver.resolveMetricThresholds(metric); + const validKeys = thresholds.rules.map(rule => rule.key); + + if (!validKeys.includes(status)) { + throw new InputError( + `Aggregation KPI "${aggregationId}" filter.status "${status}" is not a threshold rule key ` + + `for metric "${ + aggregationConfig.metricId + }". Valid keys: ${validKeys.join(', ')}.`, + ); + } +} diff --git a/workspaces/scorecard/plugins/scorecard-common/report.api.md b/workspaces/scorecard/plugins/scorecard-common/report.api.md index 6482949ca21..1433e3cfc75 100644 --- a/workspaces/scorecard/plugins/scorecard-common/report.api.md +++ b/workspaces/scorecard/plugins/scorecard-common/report.api.md @@ -36,9 +36,15 @@ export type AggregationConfig = { description: string; type: AggregationType; metricId: string; + filter?: AggregationConfigFilter; options?: AggregationConfigOptions; }; +// @public +export type AggregationConfigFilter = { + status?: string; +}; + // @public (undocumented) export type AggregationConfigOptions = { statusScores?: StatusScoreAggregationOption; @@ -52,6 +58,7 @@ export type AggregationMetadata = { type: MetricType; history?: boolean; aggregationType: AggregationType; + filter?: AggregationConfigFilter; }; // @public (undocumented) diff --git a/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts b/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts index fc58eb22ad9..c1f801d7ccc 100644 --- a/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts +++ b/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts @@ -61,6 +61,14 @@ export type ScalarAggregatedMetric = Omit & { value: number; }; +/** + * Optional filter applied to scalar aggregation KPIs. + * @public + */ +export type AggregationConfigFilter = { + status?: string; +}; + /** * @public */ @@ -70,6 +78,7 @@ export type AggregationMetadata = { type: MetricType; history?: boolean; aggregationType: AggregationType; + filter?: AggregationConfigFilter; }; /** @@ -138,5 +147,6 @@ export type AggregationConfig = { description: string; type: AggregationType; metricId: string; + filter?: AggregationConfigFilter; options?: AggregationConfigOptions; }; diff --git a/workspaces/scorecard/plugins/scorecard/README.md b/workspaces/scorecard/plugins/scorecard/README.md index 8ac483ad0d0..5fd8f984d0c 100644 --- a/workspaces/scorecard/plugins/scorecard/README.md +++ b/workspaces/scorecard/plugins/scorecard/README.md @@ -9,7 +9,7 @@ For NFS, register the default `scorecardPlugin` plus `scorecardTranslationsModul **Features:** - **Entity scorecard tab** — View scorecard metrics on catalog entity pages (components, websites, etc.). -- **Scorecard homepage card** — Show aggregated KPIs on the home page (e.g. GitHub open PRs, Jira open issues). +- **Scorecard homepage card** — Show aggregated KPIs on the home page (e.g. GitHub open PRs, Jira open issues). Supports **`statusGrouped`** (multi-slice pie) and **`weightedStatusScore`** (weighted health donut) KPI types configured under **`scorecard.aggregationKPIs`**. Scalar types (`sum`, `average`, `max`, `min`, `count`) are supported by the backend API but not yet rendered by the bundled homepage card (see [Homepage scorecard cards](#homepage-scorecard-cards)). - **Scorecard Entities page** — Drill down from an aggregated metric to see the list of entities contributing to that metric, with entity-level values and status, so you can identify services impacting the KPI and investigate issues. - **Metric group cards (grid layout)** — Group related metrics into cards with threshold bucket tiles, a filterable/sortable data sources dialog, and a Masonry grid layout. Enabled via app-config.yaml. @@ -456,7 +456,13 @@ Supported scorecard aggregation types (see [Entity Aggregation — Aggregation t - [`statusGrouped`](../scorecard-backend/docs/aggregation.md#status-grouped-type) — counts per status (pie chart). - [`weightedStatusScore`](../scorecard-backend/docs/aggregation.md#weighted-status-score-type) — weighted portfolio health percentage (donut); requires `options.statusScores`. -- Scalar types ([`sum`](../scorecard-backend/docs/aggregation.md#sum-type), [`average`](../scorecard-backend/docs/aggregation.md#average-type), [`max`](../scorecard-backend/docs/aggregation.md#max-type), [`min`](../scorecard-backend/docs/aggregation.md#min-type), [`count`](../scorecard-backend/docs/aggregation.md#count-type)) — roll up latest numeric metric values; number metrics only. +- Scalar types ([`sum`](../scorecard-backend/docs/aggregation.md#sum-type), [`average`](../scorecard-backend/docs/aggregation.md#average-type), [`max`](../scorecard-backend/docs/aggregation.md#max-type), [`min`](../scorecard-backend/docs/aggregation.md#min-type), [`count`](../scorecard-backend/docs/aggregation.md#count-type)) — roll up latest numeric metric values; number metrics only. Optional [`filter.status`](../scorecard-backend/docs/aggregation.md#status-filter-scalar-types) limits the rollup to a threshold status. + +**Scalar KPI types** (`sum`, `average`, `max`, `min`, `count`) are fully supported by **`GET /aggregations/:aggregationId`**, including optional **`filter.status`** and **`options.thresholds`**. The bundled **`ScorecardHomepageCard`** / **`AggregatedMetricCard`** only renders **`statusGrouped`** and **`weightedStatusScore`**; scalar types show an unsupported-type error panel. Build a custom card to display **`result.value`** and classify it with **`result.thresholds`**. + +**Terminology:** Scalar **`average`** (mean of numeric metric values) is distinct from the former weighted KPI type also named `average`, which was renamed to **`weightedStatusScore`**. Update app-config **`type: average`** entries for weighted health scoring to **`type: weightedStatusScore`** and migrate API consumers from **`result.averageScore`** to **`result.weightedStatusScore`**. + +The NFS example widget **`AggregatedCardWithGithubOpenPrs`** uses **`aggregationId: maxOpenPrs`** (scalar **`max`** KPI). Uncomment and configure **`maxOpenPrs`** under **`scorecard.aggregationKPIs`** in app-config to exercise the API; expect the unsupported-type panel until scalar homepage UI is added. #### Card props