From 602bac1490275db47d9f0e1968e5afeec7f2955b Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:51:32 +0000 Subject: [PATCH 1/4] feat(#4586): add enabled/disabled support for metric providers and metrics Allow MetricProviders and individual Metrics to be optionally disabled by default. This introduces: - enabled field on the Metric type (scorecard-common) - isEnabled method on MetricProvider interface (scorecard-node) - Config-level enabled attribute for providers and metrics (scorecard-backend) - Resolution chain: config metric > code metric > config provider > code provider > default (true) Disabled metrics are excluded from: - Scheduled data collection (PullMetricsByProviderTask) - Provider task initialization (scheduler) - API responses (router GET /metrics endpoints) - Scaffolder actions (listMetrics) - CatalogMetricService queries Old data for disabled metrics remains in the database. Co-Authored-By: Claude Opus 4.6 --- .../add-metric-provider-enabled-flag.md | 7 + .../plugins/scorecard-backend/config.d.ts | 18 ++ .../scorecard-backend/src/actions/index.ts | 2 + .../src/actions/listMetrics.test.ts | 7 + .../src/actions/listMetrics.ts | 13 +- .../plugins/scorecard-backend/src/plugin.ts | 3 + .../scorecard-backend/src/scheduler/index.ts | 12 ++ .../tasks/PullMetricsByProviderTask.ts | 23 ++- .../src/service/CatalogMetricService.test.ts | 2 + .../src/service/CatalogMetricService.ts | 23 ++- .../src/service/router.test.ts | 23 +++ .../scorecard-backend/src/service/router.ts | 34 +++- .../src/utils/metricProviderConfigKeys.ts | 50 ++++++ .../src/utils/metricUtils.test.ts | 170 +++++++++++++++++- .../src/utils/metricUtils.ts | 63 +++++++ .../plugins/scorecard-common/report.api.md | 1 + .../scorecard-common/src/types/Metric.ts | 7 + .../plugins/scorecard-node/report.api.md | 1 + .../scorecard-node/src/api/MetricProvider.ts | 10 ++ .../scorecard/plugins/scorecard/report.api.md | 14 +- 20 files changed, 464 insertions(+), 19 deletions(-) create mode 100644 workspaces/scorecard/.changeset/add-metric-provider-enabled-flag.md diff --git a/workspaces/scorecard/.changeset/add-metric-provider-enabled-flag.md b/workspaces/scorecard/.changeset/add-metric-provider-enabled-flag.md new file mode 100644 index 00000000000..02149b9f00d --- /dev/null +++ b/workspaces/scorecard/.changeset/add-metric-provider-enabled-flag.md @@ -0,0 +1,7 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard-common': minor +'@red-hat-developer-hub/backstage-plugin-scorecard-node': minor +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': minor +--- + +Add optional `enabled` flag for metrics and `isEnabled()` method for metric providers, allowing them to be disabled by default. Administrators can override these defaults via app-config. Disabled metrics are excluded from scheduling, API responses, and scaffolder actions. diff --git a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts index c3402acbb67..6c991c2b8a6 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts @@ -138,6 +138,15 @@ export interface Config { * Use the local name without datasource prefix (e.g., `openPRs` instead of `github.openPRs`). */ [providerName: string]: { + /** + * Whether this metric provider is enabled. Overrides the + * provider's code-level `isEnabled()` default. When `false`, + * the provider and all its metrics are disabled (unless + * individual metrics are re-enabled via their own `enabled` + * flag). When `true`, a provider that is disabled by default + * in code is re-enabled. + */ + enabled?: boolean; /** How often metrics will be calculated for this provider. */ schedule?: SchedulerServiceTaskScheduleDefinitionConfig; /** @@ -151,6 +160,15 @@ export interface Config { * Use the local name without datasource prefix (e.g., 'openPRs' instead of 'github.openPRs'). */ [metricName: string]: { + /** + * Whether this metric is enabled. Overrides both the + * metric's code-level `enabled` default and the + * provider-level enabled state. Set to `true` to + * re-enable a metric that is disabled by default; set + * to `false` to disable a metric that is normally + * enabled. + */ + enabled?: boolean; /** * How metric values are categorized for this metric. * Overrides provider-level thresholds. diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts index 22d158ced6f..6a3584e3eca 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import type { Config } from '@backstage/config'; import { AuthService, PermissionsService } from '@backstage/backend-plugin-api'; import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha'; import { CatalogService } from '@backstage/plugin-catalog-node'; @@ -27,6 +28,7 @@ export { createListMetricsAction } from './listMetrics'; export const createScorecardActions = (options: { actionsRegistry: ActionsRegistryService; auth: AuthService; + config: Config; permissions: PermissionsService; catalog: CatalogService; metricProvidersRegistry: MetricProvidersRegistry; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts index fd1dc98c3b0..1fd13953b71 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts @@ -23,6 +23,10 @@ import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; describe('createListMetricsAction', () => { const mockRegistry = { listMetrics: jest.fn(), + getProvider: jest.fn().mockReturnValue({ + getProviderId: () => 'github.openPRs', + getProviderDatasourceId: () => 'github', + }), } as unknown as MetricProvidersRegistry; beforeEach(() => { @@ -56,6 +60,7 @@ describe('createListMetricsAction', () => { createListMetricsAction({ actionsRegistry: mockActionsRegistry, + config: mockServices.rootConfig({ data: {} }), permissions: mockPermissions, metricProvidersRegistry: mockRegistry, }); @@ -78,6 +83,7 @@ describe('createListMetricsAction', () => { createListMetricsAction({ actionsRegistry: mockActionsRegistry, + config: mockServices.rootConfig({ data: {} }), permissions: mockPermissions, metricProvidersRegistry: mockRegistry, }); @@ -126,6 +132,7 @@ describe('createListMetricsAction', () => { createListMetricsAction({ actionsRegistry: mockActionsRegistry, + config: mockServices.rootConfig({ data: {} }), permissions: mockPermissions, metricProvidersRegistry: mockRegistry, }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts index c82d9cbcf86..c3f3dc18631 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import type { Config } from '@backstage/config'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha'; import { scorecardMetricReadPermission } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; @@ -21,13 +22,16 @@ import { authorizeConditional, filterAuthorizedMetrics, } from '../permissions/permissionUtils'; +import { isMetricEnabledByDefault } from '../utils/metricUtils'; export const createListMetricsAction = ({ actionsRegistry, + config, permissions, metricProvidersRegistry, }: { actionsRegistry: ActionsRegistryService; + config: Config; permissions: PermissionsService; metricProvidersRegistry: MetricProvidersRegistry; }) => { @@ -66,7 +70,14 @@ export const createListMetricsAction = ({ scorecardMetricReadPermission, ); - const allMetrics = metricProvidersRegistry.listMetrics(); + const allMetrics = metricProvidersRegistry.listMetrics().filter(m => { + try { + const provider = metricProvidersRegistry.getProvider(m.id); + return isMetricEnabledByDefault(config, m, provider); + } catch { + return true; + } + }); const metrics = filterAuthorizedMetrics(allMetrics, conditions); return { output: { metrics } }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts index 2c4efa2cf73..2d5e1e5a6b0 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts @@ -119,6 +119,7 @@ export const scorecardPlugin = createBackendPlugin({ ); const catalogMetricService = new CatalogMetricService({ + config, catalog, auth, registry: metricProvidersRegistry, @@ -159,6 +160,7 @@ export const scorecardPlugin = createBackendPlugin({ createScorecardActions({ actionsRegistry, auth, + config, permissions, catalog, metricProvidersRegistry, @@ -167,6 +169,7 @@ export const scorecardPlugin = createBackendPlugin({ httpRouter.use( await createRouter({ + config, metricProvidersRegistry, service, catalog, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts index 06612ca15f8..6fb5e22fd0a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts @@ -29,6 +29,7 @@ import { SchedulerOptions, SchedulerTask } from './types'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; import { ThresholdEvaluator } from '../threshold/ThresholdEvaluator'; import { ThresholdResolver } from '../threshold/ThresholdResolver'; +import { isMetricEnabledByDefault } from '../utils/metricUtils'; export class Scheduler { private readonly auth: AuthService; @@ -101,6 +102,17 @@ export class Scheduler { const providers = this.metricProvidersRegistry.listProviders(); for (const provider of providers) { + const hasEnabledMetric = provider + .getMetrics() + .some(m => isMetricEnabledByDefault(this.config, m, provider)); + + if (!hasEnabledMetric) { + this.logger.info( + `Skipping provider '${provider.getProviderId()}': all metrics disabled`, + ); + continue; + } + this.tasks.push({ name: provider.getProviderId(), task: new PullMetricsByProviderTask( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts index e0aae21ac96..4d32f12801c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts @@ -24,7 +24,10 @@ import { import type { Config } from '@backstage/config'; import { CatalogService } from '@backstage/plugin-catalog-node'; import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; -import { isMetricIdDisabled } from '../../utils/metricUtils'; +import { + isMetricIdDisabled, + isMetricEnabledByDefault, +} from '../../utils/metricUtils'; import { randomUUID } from 'node:crypto'; import { normalizeOwnerRef } from '../../utils/normalizeOwnerRef'; import { resolveScheduleFromConfig } from '../../utils/metricProviderConfigKeys'; @@ -124,7 +127,23 @@ export class PullMetricsByProviderTask implements SchedulerTask { let totalProcessed = 0; let cursor: string | undefined = undefined; - const metrics = provider.getMetrics(); + const allMetrics = provider.getMetrics(); + const metrics = allMetrics.filter(m => + isMetricEnabledByDefault(this.config, m, provider), + ); + + if (metrics.length < allMetrics.length) { + const skipped = allMetrics.length - metrics.length; + logger.info( + `Skipping ${skipped} disabled metric(s) for ${this.providerId}`, + ); + } + + if (metrics.length === 0) { + logger.info(`No enabled metrics for ${this.providerId}, skipping pull`); + return; + } + const metricsById = new Map(metrics.map(m => [m.id, m])); const metricIds = metrics.map(m => m.id); 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 866f6ff74d2..dadbb3870ba 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts @@ -151,6 +151,7 @@ describe('CatalogMetricService', () => { ); service = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: mockedCatalog, auth: mockedAuth, registry: mockedRegistry, @@ -686,6 +687,7 @@ describe('CatalogMetricService', () => { ); service = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: mockedCatalog, auth: mockedAuth, registry: mockedRegistry, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts index 922ad8e7743..5dba83db809 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts @@ -25,6 +25,7 @@ import { MetricTimeSeriesResponse, MetricTimeSeriesPoint, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import type { Config } from '@backstage/config'; import type { Entity } from '@backstage/catalog-model'; import { normalizeOwnerRef } from '../utils/normalizeOwnerRef'; import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; @@ -47,11 +48,13 @@ import { import { CatalogService } from '@backstage/plugin-catalog-node'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; import { isMetricCalculationError } from '../utils/metricCalculationError'; +import { isMetricEnabledByDefault } from '../utils/metricUtils'; import { AggregatedMetricMapper } from './mappers'; import { DbMetricValue } from '../database/types'; import { ThresholdResolver } from '../threshold/ThresholdResolver'; type CatalogMetricServiceOptions = { + config: Config; catalog: CatalogService; auth: AuthService; registry: MetricProvidersRegistry; @@ -77,6 +80,7 @@ export class CatalogMetricService { private readonly logger: LoggerService; + private readonly config: Config; private readonly catalog: CatalogService; private readonly auth: AuthService; private readonly registry: MetricProvidersRegistry; @@ -87,6 +91,7 @@ export class CatalogMetricService { private static readonly BATCH_SIZE = 100; constructor(options: CatalogMetricServiceOptions) { + this.config = options.config; this.catalog = options.catalog; this.auth = options.auth; this.registry = options.registry; @@ -118,7 +123,14 @@ export class CatalogMetricService { throw new NotFoundError(`Entity not found: ${entityRef}`); } - const metricsToFetch = this.registry.listMetrics(metricIds); + const metricsToFetch = this.registry.listMetrics(metricIds).filter(m => { + try { + const provider = this.registry.getProvider(m.id); + return isMetricEnabledByDefault(this.config, m, provider); + } catch { + return true; + } + }); const authorizedMetricsToFetch = filterAuthorizedMetrics( metricsToFetch, @@ -219,6 +231,11 @@ export class CatalogMetricService { } const metric = this.registry.getMetric(metricId); + const provider = this.registry.getProvider(metricId); + if (!isMetricEnabledByDefault(this.config, metric, provider)) { + throw new NotFoundError(`Metric '${metricId}' is disabled`); + } + const authorizedMetrics = filterAuthorizedMetrics([metric], filter); if (authorizedMetrics.length === 0) { throw new NotAllowedError( @@ -351,6 +368,10 @@ export class CatalogMetricService { ): Promise { // Get metric metadata const metric = this.registry.getMetric(metricId); + const provider = this.registry.getProvider(metricId); + if (!isMetricEnabledByDefault(this.config, metric, provider)) { + throw new NotFoundError(`Metric '${metricId}' is disabled`); + } // High-page early-exit guard if ( 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 cae23c10960..d07ceae3a4c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts @@ -137,6 +137,7 @@ describe('createRouter', () => { collect: jest.fn(), }; catalogMetricService = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog, registry: metricProvidersRegistry, auth: mockServices.auth(), @@ -167,6 +168,7 @@ describe('createRouter', () => { }); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry, service: { aggregationsService, catalogMetricService }, catalog, @@ -890,6 +892,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry, service: { aggregationsService: aggregationsServiceLocal, @@ -1118,6 +1121,7 @@ describe('createRouter', () => { ); const batchAggregationRouter = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry, service: { aggregationsService: batchAggregationsService, @@ -1241,6 +1245,7 @@ describe('createRouter', () => { mockCatalog.getEntities.mockResolvedValue({ items: [componentEntity] }); mockCatalogMetricService = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: mockCatalog, auth: mockServices.auth.mock({ getOwnServiceCredentials: jest.fn().mockResolvedValue({ @@ -1281,6 +1286,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metricRegistry, service: { aggregationsService: aggregationsServiceAgRoute, @@ -1369,6 +1375,7 @@ describe('createRouter', () => { ); const batchRouter = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metricRegistry, service: { aggregationsService: aggregationsServiceBatch, @@ -1420,6 +1427,7 @@ describe('createRouter', () => { }, }); const kpiService = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: mockCatalog, auth: mockServices.auth.mock({ getOwnServiceCredentials: jest.fn().mockResolvedValue({ @@ -1442,6 +1450,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metricRegistry, service: { aggregationsService: aggregationsServiceKpi, @@ -1489,6 +1498,7 @@ describe('createRouter', () => { }, }); const kpiService = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: mockCatalog, auth: mockServices.auth.mock({ getOwnServiceCredentials: jest.fn().mockResolvedValue({ @@ -1511,6 +1521,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metricRegistry, service: { aggregationsService: aggregationsServiceWeightedKpi, @@ -1551,6 +1562,7 @@ describe('createRouter', () => { }, }); const kpiService = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: mockCatalog, auth: mockServices.auth.mock({ getOwnServiceCredentials: jest.fn().mockResolvedValue({ @@ -1587,6 +1599,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metricRegistry, service: { aggregationsService: aggregationsServiceSum, @@ -1643,6 +1656,7 @@ describe('createRouter', () => { }); const kpiService = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: mockCatalog, auth: mockServices.auth.mock({ getOwnServiceCredentials: jest.fn().mockResolvedValue({ @@ -1679,6 +1693,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metricRegistry, service: { aggregationsService: aggregationsServiceFiltered, @@ -2003,6 +2018,7 @@ describe('createRouter', () => { }); metaCatalogMetricService = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: metaCatalog, auth: mockServices.auth.mock({ getOwnServiceCredentials: jest.fn().mockResolvedValue({ @@ -2021,6 +2037,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metaRegistry, service: { aggregationsService: aggregationsMetaService, @@ -2066,6 +2083,7 @@ describe('createRouter', () => { metaRegistry.register(batchProvider); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metaRegistry, service: { aggregationsService: createTestAggregationsService( @@ -2095,6 +2113,7 @@ describe('createRouter', () => { it('returns metadata for metric id when no KPI row exists', async () => { const svc = new CatalogMetricService({ + config: mockServices.rootConfig({ data: {} }), catalog: metaCatalog, auth: mockServices.auth.mock({ getOwnServiceCredentials: jest.fn().mockResolvedValue({ @@ -2113,6 +2132,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metaRegistry, service: { aggregationsService: aggregationsSvcNoKpi, @@ -2164,6 +2184,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metaRegistry, service: { aggregationsService: aggregationsMetaServiceFiltered, @@ -2211,6 +2232,7 @@ describe('createRouter', () => { ); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry: metaRegistry, service: { aggregationsService: aggregationsMetaServiceScalar, @@ -2304,6 +2326,7 @@ describe('createRouter', () => { const mockCatalog = catalogServiceMock.mock(); const router = await createRouter({ + config: mockServices.rootConfig({ data: {} }), metricProvidersRegistry, service: { aggregationsService, catalogMetricService }, catalog: mockCatalog, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts index a5e53184450..328382e585a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts @@ -21,6 +21,7 @@ import { } from '@backstage/errors'; import express from 'express'; import Router from 'express-promise-router'; +import type { Config } from '@backstage/config'; import type { CatalogMetricService } from './CatalogMetricService'; import type { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; import { @@ -43,6 +44,7 @@ import { } from '../middlewares/validateTimeSeriesQueryParams'; import { getEntitiesOwnedByUser } from '../utils/getEntitiesOwnedByUser'; import { parseCommaSeparatedString } from '../utils/parseCommaSeparatedString'; +import { isMetricEnabledByDefault } from '../utils/metricUtils'; import { AggregatedMetricMapper } from './mappers'; import { validateDrillDownMetricsSchema } from '../validation/validateDrillDownMetricsSchema'; import { validateAggregationIdParam } from '../middlewares/validateAggregationIdParam'; @@ -53,6 +55,7 @@ import { ThresholdResolver } from '../threshold/ThresholdResolver'; import type { ScorecardCollectorsService } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; export type ScorecardRouterOptions = { + config: Config; service: { aggregationsService: AggregationsService; catalogMetricService: CatalogMetricService; @@ -67,6 +70,7 @@ export type ScorecardRouterOptions = { }; export async function createRouter({ + config, metricProvidersRegistry, service, catalog, @@ -81,6 +85,20 @@ export async function createRouter({ const { aggregationsService, catalogMetricService } = service; + const filterEnabledMetrics = ( + metrics: ReturnType, + ) => + metrics.filter(m => { + try { + const provider = metricProvidersRegistry.getProvider(m.id); + return isMetricEnabledByDefault(config, m, provider); + } catch { + // Provider not found — treat as enabled to avoid hiding + // broken registrations from the listing. + return true; + } + }); + router.get( '/metrics', validateMetricIdsQueryParams, @@ -94,21 +112,27 @@ export async function createRouter({ if (metricIds) { return res.json({ - metrics: metricProvidersRegistry.listMetrics( - parseCommaSeparatedString(metricIds as string), + metrics: filterEnabledMetrics( + metricProvidersRegistry.listMetrics( + parseCommaSeparatedString(metricIds as string), + ), ), }); } if (datasource) { return res.json({ - metrics: metricProvidersRegistry.listMetricsByDatasource( - datasource as string, + metrics: filterEnabledMetrics( + metricProvidersRegistry.listMetricsByDatasource( + datasource as string, + ), ), }); } - return res.json({ metrics: metricProvidersRegistry.listMetrics() }); + return res.json({ + metrics: filterEnabledMetrics(metricProvidersRegistry.listMetrics()), + }); }, ); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts index c17574a0411..34547192991 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts @@ -116,3 +116,53 @@ export function resolveThresholdsConfigPath( ]; return paths.find(path => config.has(path)); } + +/** Config path: `scorecard.metricProviders...enabled` */ +export function getProviderEnabledConfigPath( + datasourceId: string, + providerId: string, +): string { + const providerKey = getProviderLocalConfigKey(providerId, datasourceId); + return `scorecard.metricProviders.${datasourceId}.${providerKey}.enabled`; +} + +/** Config path: `scorecard.metricProviders...metrics..enabled` */ +export function getMetricEnabledConfigPath( + datasourceId: string, + providerId: string, + metricId: string, +): string { + const providerKey = getProviderLocalConfigKey(providerId, datasourceId); + const metricKey = getMetricLocalConfigKey(metricId, datasourceId); + return ( + `scorecard.metricProviders.${datasourceId}.${providerKey}` + + `.metrics.${metricKey}.enabled` + ); +} + +/** + * Read the provider-level `enabled` flag from config. + * Returns `undefined` when not set (caller uses code default). + */ +export function resolveProviderEnabledFromConfig( + config: Config, + datasourceId: string, + providerId: string, +): boolean | undefined { + const path = getProviderEnabledConfigPath(datasourceId, providerId); + return config.getOptionalBoolean(path); +} + +/** + * Read the metric-level `enabled` flag from config. + * Returns `undefined` when not set (caller uses code / provider default). + */ +export function resolveMetricEnabledFromConfig( + config: Config, + datasourceId: string, + providerId: string, + metricId: string, +): boolean | undefined { + const path = getMetricEnabledConfigPath(datasourceId, providerId, metricId); + return config.getOptionalBoolean(path); +} diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts index 57483b578a5..bc12451606b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts @@ -15,8 +15,11 @@ */ import { mockServices } from '@backstage/backend-test-utils'; +import { ConfigReader, type JsonObject } from '@backstage/config'; import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; -import { isMetricIdDisabled } from './metricUtils'; +import { isMetricIdDisabled, isMetricEnabledByDefault } from './metricUtils'; +import type { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import type { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; describe('isMetricIdDisabled', () => { const metricId = 'openssf.maintained'; @@ -210,3 +213,168 @@ describe('isMetricIdDisabled', () => { expect(result).toBe(true); }); }); + +describe('isMetricEnabledByDefault', () => { + function createMetric(overrides: Partial = {}): Metric { + return { + id: 'github.openPRs', + title: 'Open PRs', + description: 'Number of open PRs', + type: 'number', + thresholds: { + rules: [ + { key: 'error', expression: '>40' }, + { key: 'success', expression: '<=40' }, + ], + }, + ...overrides, + }; + } + + function createProvider( + overrides: { + providerId?: string; + datasourceId?: string; + isEnabled?: () => boolean; + } = {}, + ): MetricProvider { + return { + getProviderId: () => overrides.providerId ?? 'github.openPRs', + getProviderDatasourceId: () => overrides.datasourceId ?? 'github', + getMetrics: () => [], + calculateMetrics: jest.fn(), + getCatalogFilter: () => ({}), + ...(overrides.isEnabled !== undefined && { + isEnabled: overrides.isEnabled, + }), + }; + } + + function createEnabledConfig( + providerEnabled?: boolean, + metricEnabled?: boolean, + ) { + const data: JsonObject = {}; + const providerConfig: JsonObject = {}; + + if (providerEnabled !== undefined) { + providerConfig.enabled = providerEnabled; + } + + if (metricEnabled !== undefined) { + providerConfig.metrics = { + openPRs: { enabled: metricEnabled }, + }; + } + + if (Object.keys(providerConfig).length > 0) { + data.metricProviders = { github: { openPRs: providerConfig } }; + } + + return new ConfigReader({ scorecard: data }); + } + + it('returns true when no enabled flag is set anywhere (backward compatible)', () => { + const config = createEnabledConfig(); + const metric = createMetric(); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + }); + + it('returns false when metric code-level enabled is false', () => { + const config = createEnabledConfig(); + const metric = createMetric({ enabled: false }); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + }); + + it('returns true when metric code-level enabled is true', () => { + const config = createEnabledConfig(); + const metric = createMetric({ enabled: true }); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + }); + + it('returns false when provider isEnabled returns false', () => { + const config = createEnabledConfig(); + const metric = createMetric(); + const provider = createProvider({ isEnabled: () => false }); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + }); + + it('config metric enabled:true overrides code metric enabled:false', () => { + const config = createEnabledConfig(undefined, true); + const metric = createMetric({ enabled: false }); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + }); + + it('config metric enabled:false overrides code metric enabled:true', () => { + const config = createEnabledConfig(undefined, false); + const metric = createMetric({ enabled: true }); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + }); + + it('config provider enabled:true overrides provider isEnabled:false', () => { + const config = createEnabledConfig(true); + const metric = createMetric(); + const provider = createProvider({ isEnabled: () => false }); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + }); + + it('config provider enabled:false disables an otherwise-enabled provider', () => { + const config = createEnabledConfig(false); + const metric = createMetric(); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + }); + + it('code metric enabled:true overrides provider isEnabled:false', () => { + const config = createEnabledConfig(); + const metric = createMetric({ enabled: true }); + const provider = createProvider({ isEnabled: () => false }); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + }); + + it('config metric enabled:true overrides config provider enabled:false', () => { + const config = createEnabledConfig(false, true); + const metric = createMetric(); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + }); + + it('config metric enabled:false takes precedence over config provider enabled:true', () => { + const config = createEnabledConfig(true, false); + const metric = createMetric(); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + }); + + it('metric without enabled inherits provider isEnabled:false', () => { + const config = createEnabledConfig(); + const metric = createMetric(); + const provider = createProvider({ isEnabled: () => false }); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + }); + + it('metric without enabled and provider without isEnabled defaults to true', () => { + const config = createEnabledConfig(); + const metric = createMetric(); + const provider = createProvider(); + + expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts index 00d76271ca9..49a969abb61 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts @@ -17,7 +17,13 @@ import type { Config } from '@backstage/config'; import type { Entity } from '@backstage/catalog-model'; import type { LoggerService } from '@backstage/backend-plugin-api'; +import type { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import type { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import { parseCommaSeparatedString } from './parseCommaSeparatedString'; +import { + resolveMetricEnabledFromConfig, + resolveProviderEnabledFromConfig, +} from './metricProviderConfigKeys'; /** * Check if the metric is disabled by app-config, if is disabled by the entity annotation and if it has any exception rule. @@ -80,3 +86,60 @@ export function isMetricIdDisabled( return false; } + +/** + * Check whether a metric is enabled by default, considering code-level + * defaults and config-level overrides. + * + * Resolution order (first defined value wins): + * 1. Config metric `enabled` — most specific config + * 2. Code metric `enabled` field — code-level default + * 3. Config provider `enabled` — provider config + * 4. Code provider `isEnabled()` — provider code default + * 5. `true` — backward-compatible default + * + * This check is independent of the global `disabledMetrics` list and + * entity annotations which are handled by {@link isMetricIdDisabled}. + */ +export function isMetricEnabledByDefault( + config: Config, + metric: Metric, + provider: MetricProvider, +): boolean { + const datasourceId = provider.getProviderDatasourceId(); + const providerId = provider.getProviderId(); + + // 1. Config metric enabled (most specific) + const metricConfigEnabled = resolveMetricEnabledFromConfig( + config, + datasourceId, + providerId, + metric.id, + ); + if (metricConfigEnabled !== undefined) { + return metricConfigEnabled; + } + + // 2. Code metric enabled + if (metric.enabled !== undefined) { + return metric.enabled; + } + + // 3. Config provider enabled + const providerConfigEnabled = resolveProviderEnabledFromConfig( + config, + datasourceId, + providerId, + ); + if (providerConfigEnabled !== undefined) { + return providerConfigEnabled; + } + + // 4. Code provider isEnabled() + if (provider.isEnabled) { + return provider.isEnabled(); + } + + // 5. Default: enabled + return true; +} diff --git a/workspaces/scorecard/plugins/scorecard-common/report.api.md b/workspaces/scorecard/plugins/scorecard-common/report.api.md index a655b08089a..c6adb71f240 100644 --- a/workspaces/scorecard/plugins/scorecard-common/report.api.md +++ b/workspaces/scorecard/plugins/scorecard-common/report.api.md @@ -153,6 +153,7 @@ export type Metric = { history?: boolean; defaultVisualization?: ScorecardVisualizationType; collectorIds?: string[]; + enabled?: boolean; }; // @public (undocumented) diff --git a/workspaces/scorecard/plugins/scorecard-common/src/types/Metric.ts b/workspaces/scorecard/plugins/scorecard-common/src/types/Metric.ts index 2189173db92..781c4711150 100644 --- a/workspaces/scorecard/plugins/scorecard-common/src/types/Metric.ts +++ b/workspaces/scorecard/plugins/scorecard-common/src/types/Metric.ts @@ -48,6 +48,13 @@ export type Metric = { * provider config at startup. Omitted when the metric does not use collectors. */ collectorIds?: string[]; + /** + * Whether this metric is enabled by default. When `false`, the metric + * is disabled unless the administrator explicitly enables it in + * `app-config.yaml`. Omitting this field (or setting it to `true`) + * means the metric is enabled by default. + */ + enabled?: boolean; }; /** diff --git a/workspaces/scorecard/plugins/scorecard-node/report.api.md b/workspaces/scorecard/plugins/scorecard-node/report.api.md index 7795957dc8d..470121404ee 100644 --- a/workspaces/scorecard/plugins/scorecard-node/report.api.md +++ b/workspaces/scorecard/plugins/scorecard-node/report.api.md @@ -64,6 +64,7 @@ export interface MetricProvider { getMetrics(): Metric[]; getProviderDatasourceId(): string; getProviderId(): string; + isEnabled?(): boolean; } // @public diff --git a/workspaces/scorecard/plugins/scorecard-node/src/api/MetricProvider.ts b/workspaces/scorecard/plugins/scorecard-node/src/api/MetricProvider.ts index ce8740ef62a..10457c40089 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/api/MetricProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/api/MetricProvider.ts @@ -53,4 +53,14 @@ export interface MetricProvider { * @public */ getCatalogFilter(): Record; + /** + * Whether this metric provider is enabled by default. When this + * method returns `false`, all metrics from this provider are + * disabled unless the administrator explicitly enables them (or + * the provider itself) in `app-config.yaml`. + * + * Omitting this method means the provider is enabled by default. + * @public + */ + isEnabled?(): boolean; } diff --git a/workspaces/scorecard/plugins/scorecard/report.api.md b/workspaces/scorecard/plugins/scorecard/report.api.md index 359bb47222e..93a76bfef40 100644 --- a/workspaces/scorecard/plugins/scorecard/report.api.md +++ b/workspaces/scorecard/plugins/scorecard/report.api.md @@ -32,8 +32,8 @@ const _default: OverridableFrontendPlugin< { root: RouteRef; drillDown: RouteRef<{ - aggregationId: string; metricId: string; + aggregationId: string; }>; }, {}, @@ -57,8 +57,8 @@ const _default: OverridableFrontendPlugin< config: { allowedFilters: | { - kind?: string | undefined; type?: string | undefined; + kind?: string | undefined; }[] | undefined; path: string | undefined; @@ -70,8 +70,8 @@ const _default: OverridableFrontendPlugin< configInput: { allowedFilters?: | { - kind?: string | undefined; type?: string | undefined; + kind?: string | undefined; }[] | undefined; path?: string | undefined | undefined; @@ -121,12 +121,8 @@ const _default: OverridableFrontendPlugin< >; inputs: { layouts: ExtensionInput< - | ConfigurableExtensionDataRef - | ConfigurableExtensionDataRef< - JSX_2.Element, - 'core.reactElement', - {} - >, + | ConfigurableExtensionDataRef + | ConfigurableExtensionDataRef, { singleton: false; optional: true; From 52801b10704614329184f7efb64fa0c63d530c09 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:50:22 +0000 Subject: [PATCH 2/4] fix: address review feedback on PR #4589 - Rename isMetricEnabledByDefault to isMetricEnabled for clarity - Reorder resolution chain: config overrides now always take precedence over code defaults (config metric > config provider > code metric > code provider > default) - Add enabled checks to 5 router endpoints that were serving disabled metrics (collectors, deprecated aggregations, aggregations, aggregations metadata, aggregations time-series) - Extract filterEnabledMetrics utility to eliminate try/catch duplication across router, CatalogMetricService, and listMetrics - Add error logging in catch blocks instead of silently swallowing - Fix information leakage: use generic "Metric not found" message instead of revealing disabled state - Add tests for config provider overriding code metric enabled and for disabled metrics on router endpoints - Update docs: providers.md, disabled-metrics-logic.md, README.md Addresses review feedback on #4589 --- .../plugins/scorecard-backend/README.md | 15 ++++ .../docs/disabled-metrics-logic.md | 27 +++++++ .../scorecard-backend/docs/providers.md | 39 ++++++++++ .../scorecard-backend/src/actions/index.ts | 7 +- .../src/actions/listMetrics.test.ts | 3 + .../src/actions/listMetrics.ts | 23 +++--- .../plugins/scorecard-backend/src/plugin.ts | 1 + .../scorecard-backend/src/scheduler/index.ts | 4 +- .../tasks/PullMetricsByProviderTask.ts | 7 +- .../src/service/CatalogMetricService.ts | 24 +++--- .../src/service/router.test.ts | 74 +++++++++++++++++++ .../scorecard-backend/src/service/router.ts | 47 ++++++++---- .../src/utils/metricUtils.test.ts | 46 ++++++++---- .../src/utils/metricUtils.ts | 64 ++++++++++++---- 14 files changed, 306 insertions(+), 75 deletions(-) diff --git a/workspaces/scorecard/plugins/scorecard-backend/README.md b/workspaces/scorecard/plugins/scorecard-backend/README.md index 10d55ae4bc9..2356796a5c9 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend/README.md @@ -116,6 +116,21 @@ To use these providers, install the corresponding backend modules: Administrators can disable metric checks globally via `scorecard.disabledMetrics`, and users can disable them per entity via the `scorecard.io/disabled-metrics` annotation. Whether that annotation is honored is controlled by `scorecard.entityAnnotations.enabled` (global switch for all scorecard entity annotations) and `scorecard.entityAnnotations.disabledMetrics` (`enabled` / `except`). For more details, see [disabled-metrics-logic.md](./docs/disabled-metrics-logic.md). +Providers and individual metrics can also be disabled via the `enabled` config key at the provider or metric level: + +```yaml +scorecard: + metricProviders: + github: + openPRs: + enabled: false # disable the entire provider + metrics: + openPRs: + enabled: true # re-enable a specific metric +``` + +Config overrides take precedence over code-level defaults. See [providers.md](./docs/providers.md#disabling-providers-and-metrics-by-default) for provider authoring details and [disabled-metrics-logic.md](./docs/disabled-metrics-logic.md#enabled-by-default-system) for the full resolution chain. + ## Thresholds Thresholds define conditions to assign metric values to specific visual categories (`success`, `warning`, `error` or any custom category). The Scorecard plugin provides multiple ways to configure thresholds: diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md b/workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md index 8c669c00c4c..77a7949f23e 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md @@ -19,6 +19,33 @@ The following table describes the result for each combination of app-config and `—`: means this setting is not consulted for that row. +## Enabled-by-default system + +In addition to the `disabledMetrics` list and entity annotations described above, providers and individual metrics can be disabled **by default** in code using the `isEnabled()` method on `MetricProvider` and the `enabled` field on `Metric`. Administrators can override these defaults via `app-config.yaml`: + +```yaml +scorecard: + metricProviders: + myDatasource: + exampleProvider: + enabled: false # disable the entire provider + metrics: + experimentalMetric: + enabled: true # re-enable a specific metric +``` + +The enabled-by-default resolution uses a five-level precedence chain (first defined value wins): + +1. **Config metric `enabled`** — most specific config override +2. **Config provider `enabled`** — provider-level config override +3. **Code metric `enabled` field** — code-level default +4. **Code provider `isEnabled()`** — provider code default +5. **`true`** — backward-compatible default + +Config overrides always take precedence over code defaults. Disabled metrics are excluded from scheduled data collection, API responses, and scaffolder actions. Old data for disabled metrics remains in the database. + +**Interaction with `disabledMetrics`:** The enabled-by-default system and the `disabledMetrics` list are independent mechanisms. A metric must pass both checks to be active: it must be enabled by the resolution chain above **and** not appear in `scorecard.disabledMetrics`. The `disabledMetrics` list is a per-entity check evaluated at collection time, while the enabled-by-default system is a global check applied at startup and request time. + ## Summary - **`scorecard.disabledMetrics`** diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md b/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md index 719f7a920bb..8f5349e3328 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md @@ -104,6 +104,45 @@ export class MyMetricProvider implements MetricProvider<'number'> { - Each metric carries its own `type` and `thresholds` - Configuration for metric providers follows the schema in [`config.d.ts`](../config.d.ts) under `scorecard.metricProviders..` (e.g., for schedule and threshold configurations) +### Disabling providers and metrics by default + +Providers and individual metrics can be disabled by default in code. Administrators can override these defaults via `app-config.yaml`. + +**Provider-level:** Implement the optional `isEnabled()` method on `MetricProvider`. When it returns `false`, the provider and all its metrics are disabled unless overridden by config: + +```typescript +isEnabled(): boolean { + return false; // disabled by default; admins can re-enable via config +} +``` + +**Metric-level:** Set the optional `enabled` field on a `Metric` object. When `false`, the individual metric is disabled by default: + +```typescript +getMetrics(): Metric<'number'>[] { + return [ + { + id: 'myDatasource.experimentalMetric', + title: 'Experimental Metric', + description: 'This metric is disabled by default.', + type: 'number', + thresholds: DEFAULT_NUMBER_THRESHOLDS, + enabled: false, + }, + ]; +} +``` + +The enabled state is resolved using a five-level precedence chain (first defined value wins): + +1. Config metric `enabled` — most specific config override +2. Config provider `enabled` — provider-level config override +3. Code metric `enabled` field — code-level default +4. Code provider `isEnabled()` — provider code default +5. `true` — backward-compatible default + +Config overrides always take precedence over code defaults. Disabled metrics are excluded from scheduling, API responses, and scaffolder actions. See [disabled-metrics-logic.md](./disabled-metrics-logic.md) for the full disabled-metrics system. + ## Updating the Module Update the module registration in `module.ts` to register your metric provider: diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts index 6a3584e3eca..fb0ddb18bd1 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts @@ -14,7 +14,11 @@ * limitations under the License. */ import type { Config } from '@backstage/config'; -import { AuthService, PermissionsService } from '@backstage/backend-plugin-api'; +import { + AuthService, + LoggerService, + PermissionsService, +} from '@backstage/backend-plugin-api'; import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha'; import { CatalogService } from '@backstage/plugin-catalog-node'; import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; @@ -29,6 +33,7 @@ export const createScorecardActions = (options: { actionsRegistry: ActionsRegistryService; auth: AuthService; config: Config; + logger: LoggerService; permissions: PermissionsService; catalog: CatalogService; metricProvidersRegistry: MetricProvidersRegistry; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts index 1fd13953b71..5aaa0ba196a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts @@ -63,6 +63,7 @@ describe('createListMetricsAction', () => { config: mockServices.rootConfig({ data: {} }), permissions: mockPermissions, metricProvidersRegistry: mockRegistry, + logger: mockServices.logger.mock(), }); const result = await mockActionsRegistry.invoke({ @@ -86,6 +87,7 @@ describe('createListMetricsAction', () => { config: mockServices.rootConfig({ data: {} }), permissions: mockPermissions, metricProvidersRegistry: mockRegistry, + logger: mockServices.logger.mock(), }); await expect( @@ -135,6 +137,7 @@ describe('createListMetricsAction', () => { config: mockServices.rootConfig({ data: {} }), permissions: mockPermissions, metricProvidersRegistry: mockRegistry, + logger: mockServices.logger.mock(), }); const result = await mockActionsRegistry.invoke({ diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts index c3f3dc18631..8fbff401b51 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts @@ -14,7 +14,10 @@ * limitations under the License. */ import type { Config } from '@backstage/config'; -import { PermissionsService } from '@backstage/backend-plugin-api'; +import { + LoggerService, + PermissionsService, +} from '@backstage/backend-plugin-api'; import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha'; import { scorecardMetricReadPermission } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry'; @@ -22,18 +25,20 @@ import { authorizeConditional, filterAuthorizedMetrics, } from '../permissions/permissionUtils'; -import { isMetricEnabledByDefault } from '../utils/metricUtils'; +import { filterEnabledMetrics } from '../utils/metricUtils'; export const createListMetricsAction = ({ actionsRegistry, config, permissions, metricProvidersRegistry, + logger, }: { actionsRegistry: ActionsRegistryService; config: Config; permissions: PermissionsService; metricProvidersRegistry: MetricProvidersRegistry; + logger: LoggerService; }) => { actionsRegistry.register({ name: 'list-metrics', @@ -70,14 +75,12 @@ export const createListMetricsAction = ({ scorecardMetricReadPermission, ); - const allMetrics = metricProvidersRegistry.listMetrics().filter(m => { - try { - const provider = metricProvidersRegistry.getProvider(m.id); - return isMetricEnabledByDefault(config, m, provider); - } catch { - return true; - } - }); + const allMetrics = filterEnabledMetrics( + config, + metricProvidersRegistry.listMetrics(), + metricId => metricProvidersRegistry.getProvider(metricId), + logger, + ); const metrics = filterAuthorizedMetrics(allMetrics, conditions); return { output: { metrics } }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts index 2d5e1e5a6b0..ce50be5e91a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts @@ -161,6 +161,7 @@ export const scorecardPlugin = createBackendPlugin({ actionsRegistry, auth, config, + logger, permissions, catalog, metricProvidersRegistry, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts index 6fb5e22fd0a..443c7c4a68c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts @@ -29,7 +29,7 @@ import { SchedulerOptions, SchedulerTask } from './types'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; import { ThresholdEvaluator } from '../threshold/ThresholdEvaluator'; import { ThresholdResolver } from '../threshold/ThresholdResolver'; -import { isMetricEnabledByDefault } from '../utils/metricUtils'; +import { isMetricEnabled } from '../utils/metricUtils'; export class Scheduler { private readonly auth: AuthService; @@ -104,7 +104,7 @@ export class Scheduler { for (const provider of providers) { const hasEnabledMetric = provider .getMetrics() - .some(m => isMetricEnabledByDefault(this.config, m, provider)); + .some(m => isMetricEnabled(this.config, m, provider)); if (!hasEnabledMetric) { this.logger.info( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts index 4d32f12801c..c2b6c5a4d4c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts @@ -24,10 +24,7 @@ import { import type { Config } from '@backstage/config'; import { CatalogService } from '@backstage/plugin-catalog-node'; import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; -import { - isMetricIdDisabled, - isMetricEnabledByDefault, -} from '../../utils/metricUtils'; +import { isMetricIdDisabled, isMetricEnabled } from '../../utils/metricUtils'; import { randomUUID } from 'node:crypto'; import { normalizeOwnerRef } from '../../utils/normalizeOwnerRef'; import { resolveScheduleFromConfig } from '../../utils/metricProviderConfigKeys'; @@ -129,7 +126,7 @@ export class PullMetricsByProviderTask implements SchedulerTask { const allMetrics = provider.getMetrics(); const metrics = allMetrics.filter(m => - isMetricEnabledByDefault(this.config, m, provider), + isMetricEnabled(this.config, m, provider), ); if (metrics.length < allMetrics.length) { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts index 5dba83db809..4e67b1d11ff 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts @@ -48,7 +48,7 @@ import { import { CatalogService } from '@backstage/plugin-catalog-node'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; import { isMetricCalculationError } from '../utils/metricCalculationError'; -import { isMetricEnabledByDefault } from '../utils/metricUtils'; +import { isMetricEnabled, filterEnabledMetrics } from '../utils/metricUtils'; import { AggregatedMetricMapper } from './mappers'; import { DbMetricValue } from '../database/types'; import { ThresholdResolver } from '../threshold/ThresholdResolver'; @@ -123,14 +123,12 @@ export class CatalogMetricService { throw new NotFoundError(`Entity not found: ${entityRef}`); } - const metricsToFetch = this.registry.listMetrics(metricIds).filter(m => { - try { - const provider = this.registry.getProvider(m.id); - return isMetricEnabledByDefault(this.config, m, provider); - } catch { - return true; - } - }); + const metricsToFetch = filterEnabledMetrics( + this.config, + this.registry.listMetrics(metricIds), + metricId => this.registry.getProvider(metricId), + this.logger, + ); const authorizedMetricsToFetch = filterAuthorizedMetrics( metricsToFetch, @@ -232,8 +230,8 @@ export class CatalogMetricService { const metric = this.registry.getMetric(metricId); const provider = this.registry.getProvider(metricId); - if (!isMetricEnabledByDefault(this.config, metric, provider)) { - throw new NotFoundError(`Metric '${metricId}' is disabled`); + if (!isMetricEnabled(this.config, metric, provider)) { + throw new NotFoundError(`Metric not found: ${metricId}`); } const authorizedMetrics = filterAuthorizedMetrics([metric], filter); @@ -369,8 +367,8 @@ export class CatalogMetricService { // Get metric metadata const metric = this.registry.getMetric(metricId); const provider = this.registry.getProvider(metricId); - if (!isMetricEnabledByDefault(this.config, metric, provider)) { - throw new NotFoundError(`Metric '${metricId}' is disabled`); + if (!isMetricEnabled(this.config, metric, provider)) { + throw new NotFoundError(`Metric not found: ${metricId}`); } // High-page early-exit guard 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 d07ceae3a4c..31760697804 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts @@ -332,6 +332,43 @@ describe('createRouter', () => { 'Cannot filter by both metricIds and datasource', ); }); + + it('should exclude disabled metrics from the listing', async () => { + const disabledConfig = mockServices.rootConfig({ + data: { + scorecard: { + metricProviders: { + github: { + openPRs: { enabled: false }, + }, + }, + }, + }, + }); + + const disabledRouter = await createRouter({ + config: disabledConfig, + metricProvidersRegistry, + service: { aggregationsService, catalogMetricService }, + catalog, + httpAuth: httpAuthMock, + permissions: permissionsMock, + logger: mockServices.logger.mock(), + thresholdResolver, + collectorsService, + }); + const disabledApp = express(); + disabledApp.use(disabledRouter); + disabledApp.use(mockErrorHandler()); + + const response = await request(disabledApp).get('/metrics'); + + expect(response.status).toBe(200); + const metricIds = response.body.metrics.map((m: Metric) => m.id); + expect(metricIds).not.toContain('github.openPRs'); + expect(metricIds).toContain('github.openIssues'); + expect(metricIds).toContain('sonar.quality'); + }); }); describe('GET /metrics/:metricId/collectors', () => { @@ -431,6 +468,43 @@ describe('createRouter', () => { expect(response.body.error.name).toBe('NotAllowedError'); }); + it('returns 404 when the metric is disabled', async () => { + const disabledConfig = mockServices.rootConfig({ + data: { + scorecard: { + metricProviders: { + dora: { + changeFailureRate: { enabled: false }, + }, + }, + }, + }, + }); + + const disabledRouter = await createRouter({ + config: disabledConfig, + metricProvidersRegistry, + service: { aggregationsService, catalogMetricService }, + catalog, + httpAuth: httpAuthMock, + permissions: permissionsMock, + logger: mockServices.logger.mock(), + thresholdResolver, + collectorsService, + }); + const disabledApp = express(); + disabledApp.use(disabledRouter); + disabledApp.use(mockErrorHandler()); + + const response = await request(disabledApp).get( + '/metrics/dora.changeFailureRate/collectors', + ); + + expect(response.status).toBe(404); + expect(response.body.error.name).toBe('NotFoundError'); + expect(response.body.error.message).not.toContain('disabled'); + }); + it('returns 500 when a collector ID on the metric is not registered', async () => { (collectorsService.getCollectorMetadata as jest.Mock).mockImplementation( (collectorId: string) => { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts index 328382e585a..3e60142ade4 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts @@ -44,7 +44,7 @@ import { } from '../middlewares/validateTimeSeriesQueryParams'; import { getEntitiesOwnedByUser } from '../utils/getEntitiesOwnedByUser'; import { parseCommaSeparatedString } from '../utils/parseCommaSeparatedString'; -import { isMetricEnabledByDefault } from '../utils/metricUtils'; +import { isMetricEnabled, filterEnabledMetrics } from '../utils/metricUtils'; import { AggregatedMetricMapper } from './mappers'; import { validateDrillDownMetricsSchema } from '../validation/validateDrillDownMetricsSchema'; import { validateAggregationIdParam } from '../middlewares/validateAggregationIdParam'; @@ -85,19 +85,24 @@ export async function createRouter({ const { aggregationsService, catalogMetricService } = service; - const filterEnabledMetrics = ( + const getProvider = (metricId: string) => + metricProvidersRegistry.getProvider(metricId); + + const filterEnabled = ( metrics: ReturnType, - ) => - metrics.filter(m => { - try { - const provider = metricProvidersRegistry.getProvider(m.id); - return isMetricEnabledByDefault(config, m, provider); - } catch { - // Provider not found — treat as enabled to avoid hiding - // broken registrations from the listing. - return true; - } - }); + ) => filterEnabledMetrics(config, metrics, getProvider, logger); + + /** + * Throw NotFoundError when a metric is disabled. Call after + * `getMetric()` — that already throws if the ID is unregistered. + */ + const assertMetricEnabled = (metricId: string) => { + const metric = metricProvidersRegistry.getMetric(metricId); + const provider = metricProvidersRegistry.getProvider(metricId); + if (!isMetricEnabled(config, metric, provider)) { + throw new NotFoundError(`Metric not found: ${metricId}`); + } + }; router.get( '/metrics', @@ -112,7 +117,7 @@ export async function createRouter({ if (metricIds) { return res.json({ - metrics: filterEnabledMetrics( + metrics: filterEnabled( metricProvidersRegistry.listMetrics( parseCommaSeparatedString(metricIds as string), ), @@ -122,7 +127,7 @@ export async function createRouter({ if (datasource) { return res.json({ - metrics: filterEnabledMetrics( + metrics: filterEnabled( metricProvidersRegistry.listMetricsByDatasource( datasource as string, ), @@ -131,7 +136,7 @@ export async function createRouter({ } return res.json({ - metrics: filterEnabledMetrics(metricProvidersRegistry.listMetrics()), + metrics: filterEnabled(metricProvidersRegistry.listMetrics()), }); }, ); @@ -139,6 +144,8 @@ export async function createRouter({ router.get('/metrics/:metricId/collectors', async (req, res) => { const { metricId } = req.params; + assertMetricEnabled(metricId); + const { conditions } = await authorizeConditional( await httpAuth.credentials(req), permissions, @@ -245,6 +252,8 @@ export async function createRouter({ async (req, res) => { const { metricId } = req.params; + assertMetricEnabled(metricId); + const { conditions } = await authorizeConditional( await httpAuth.credentials(req), permissions, @@ -379,6 +388,8 @@ export async function createRouter({ metricProvidersRegistry, ); + assertMetricEnabled(aggregationConfig.metricId); + const metric = metricProvidersRegistry.getMetric( aggregationConfig.metricId, ); @@ -423,6 +434,8 @@ export async function createRouter({ metricProvidersRegistry, ); + assertMetricEnabled(aggregationConfig.metricId); + const metric = metricProvidersRegistry.getMetric( aggregationConfig.metricId, ); @@ -456,6 +469,8 @@ export async function createRouter({ metricProvidersRegistry, ); + assertMetricEnabled(aggregationConfig.metricId); + const metric = metricProvidersRegistry.getMetric( aggregationConfig.metricId, ); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts index bc12451606b..78d602cd920 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts @@ -17,7 +17,7 @@ import { mockServices } from '@backstage/backend-test-utils'; import { ConfigReader, type JsonObject } from '@backstage/config'; import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; -import { isMetricIdDisabled, isMetricEnabledByDefault } from './metricUtils'; +import { isMetricIdDisabled, isMetricEnabled } from './metricUtils'; import type { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import type { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; @@ -214,7 +214,7 @@ describe('isMetricIdDisabled', () => { }); }); -describe('isMetricEnabledByDefault', () => { +describe('isMetricEnabled', () => { function createMetric(overrides: Partial = {}): Metric { return { id: 'github.openPRs', @@ -279,7 +279,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + expect(isMetricEnabled(config, metric, provider)).toBe(true); }); it('returns false when metric code-level enabled is false', () => { @@ -287,7 +287,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric({ enabled: false }); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + expect(isMetricEnabled(config, metric, provider)).toBe(false); }); it('returns true when metric code-level enabled is true', () => { @@ -295,7 +295,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric({ enabled: true }); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + expect(isMetricEnabled(config, metric, provider)).toBe(true); }); it('returns false when provider isEnabled returns false', () => { @@ -303,7 +303,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider({ isEnabled: () => false }); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + expect(isMetricEnabled(config, metric, provider)).toBe(false); }); it('config metric enabled:true overrides code metric enabled:false', () => { @@ -311,7 +311,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric({ enabled: false }); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + expect(isMetricEnabled(config, metric, provider)).toBe(true); }); it('config metric enabled:false overrides code metric enabled:true', () => { @@ -319,7 +319,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric({ enabled: true }); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + expect(isMetricEnabled(config, metric, provider)).toBe(false); }); it('config provider enabled:true overrides provider isEnabled:false', () => { @@ -327,7 +327,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider({ isEnabled: () => false }); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + expect(isMetricEnabled(config, metric, provider)).toBe(true); }); it('config provider enabled:false disables an otherwise-enabled provider', () => { @@ -335,7 +335,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + expect(isMetricEnabled(config, metric, provider)).toBe(false); }); it('code metric enabled:true overrides provider isEnabled:false', () => { @@ -343,7 +343,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric({ enabled: true }); const provider = createProvider({ isEnabled: () => false }); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + expect(isMetricEnabled(config, metric, provider)).toBe(true); }); it('config metric enabled:true overrides config provider enabled:false', () => { @@ -351,7 +351,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + expect(isMetricEnabled(config, metric, provider)).toBe(true); }); it('config metric enabled:false takes precedence over config provider enabled:true', () => { @@ -359,7 +359,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + expect(isMetricEnabled(config, metric, provider)).toBe(false); }); it('metric without enabled inherits provider isEnabled:false', () => { @@ -367,7 +367,7 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider({ isEnabled: () => false }); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(false); + expect(isMetricEnabled(config, metric, provider)).toBe(false); }); it('metric without enabled and provider without isEnabled defaults to true', () => { @@ -375,6 +375,22 @@ describe('isMetricEnabledByDefault', () => { const metric = createMetric(); const provider = createProvider(); - expect(isMetricEnabledByDefault(config, metric, provider)).toBe(true); + expect(isMetricEnabled(config, metric, provider)).toBe(true); + }); + + it('config provider enabled:false overrides code metric enabled:true', () => { + const config = createEnabledConfig(false); + const metric = createMetric({ enabled: true }); + const provider = createProvider(); + + expect(isMetricEnabled(config, metric, provider)).toBe(false); + }); + + it('config provider enabled:true overrides code metric enabled:false', () => { + const config = createEnabledConfig(true); + const metric = createMetric({ enabled: false }); + const provider = createProvider(); + + expect(isMetricEnabled(config, metric, provider)).toBe(true); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts index 49a969abb61..cf800b47574 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts @@ -24,6 +24,7 @@ import { resolveMetricEnabledFromConfig, resolveProviderEnabledFromConfig, } from './metricProviderConfigKeys'; +import { stringifyError } from '@backstage/errors'; /** * Check if the metric is disabled by app-config, if is disabled by the entity annotation and if it has any exception rule. @@ -88,20 +89,24 @@ export function isMetricIdDisabled( } /** - * Check whether a metric is enabled by default, considering code-level - * defaults and config-level overrides. + * Check whether a metric is enabled, considering config-level overrides + * and code-level defaults. * * Resolution order (first defined value wins): - * 1. Config metric `enabled` — most specific config - * 2. Code metric `enabled` field — code-level default - * 3. Config provider `enabled` — provider config + * 1. Config metric `enabled` — most specific config override + * 2. Config provider `enabled` — provider-level config override + * 3. Code metric `enabled` field — code-level default * 4. Code provider `isEnabled()` — provider code default * 5. `true` — backward-compatible default * + * Config overrides always take precedence over code defaults, and + * more-specific overrides (metric) win over less-specific ones + * (provider) within the same level (config or code). + * * This check is independent of the global `disabledMetrics` list and * entity annotations which are handled by {@link isMetricIdDisabled}. */ -export function isMetricEnabledByDefault( +export function isMetricEnabled( config: Config, metric: Metric, provider: MetricProvider, @@ -109,7 +114,7 @@ export function isMetricEnabledByDefault( const datasourceId = provider.getProviderDatasourceId(); const providerId = provider.getProviderId(); - // 1. Config metric enabled (most specific) + // 1. Config metric enabled (most specific config override) const metricConfigEnabled = resolveMetricEnabledFromConfig( config, datasourceId, @@ -120,12 +125,7 @@ export function isMetricEnabledByDefault( return metricConfigEnabled; } - // 2. Code metric enabled - if (metric.enabled !== undefined) { - return metric.enabled; - } - - // 3. Config provider enabled + // 2. Config provider enabled (provider-level config override) const providerConfigEnabled = resolveProviderEnabledFromConfig( config, datasourceId, @@ -135,6 +135,11 @@ export function isMetricEnabledByDefault( return providerConfigEnabled; } + // 3. Code metric enabled + if (metric.enabled !== undefined) { + return metric.enabled; + } + // 4. Code provider isEnabled() if (provider.isEnabled) { return provider.isEnabled(); @@ -143,3 +148,36 @@ export function isMetricEnabledByDefault( // 5. Default: enabled return true; } + +/** + * Filter an array of metrics to only those that are enabled. + * + * Wraps {@link isMetricEnabled} with provider lookup and error handling. + * When the provider for a metric cannot be resolved, the metric is + * treated as enabled to avoid hiding broken registrations. + * + * @param config - Backstage config + * @param metrics - The metrics to filter + * @param getProvider - Lookup function for the metric's provider + * @param logger - Optional logger for diagnostics + */ +export function filterEnabledMetrics( + config: Config, + metrics: Metric[], + getProvider: (metricId: string) => MetricProvider, + logger?: LoggerService, +): Metric[] { + return metrics.filter(m => { + try { + const provider = getProvider(m.id); + return isMetricEnabled(config, m, provider); + } catch (error) { + logger?.debug( + `Unable to resolve enabled state for metric '${ + m.id + }', treating as enabled: ${stringifyError(error)}`, + ); + return true; + } + }); +} From dacd8375ca3653ec6614f8c7c0140edbdc750de7 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 10:20:32 +0000 Subject: [PATCH 3/4] fix: regenerate scorecard frontend API report for PR #4589 Regenerate report.api.md for the scorecard frontend plugin to match API Extractor output. The property ordering in the committed report diverged from what API Extractor generates, causing CI failures. Addresses review feedback on #4589 --- .../scorecard/plugins/scorecard/report.api.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/workspaces/scorecard/plugins/scorecard/report.api.md b/workspaces/scorecard/plugins/scorecard/report.api.md index 93a76bfef40..359bb47222e 100644 --- a/workspaces/scorecard/plugins/scorecard/report.api.md +++ b/workspaces/scorecard/plugins/scorecard/report.api.md @@ -32,8 +32,8 @@ const _default: OverridableFrontendPlugin< { root: RouteRef; drillDown: RouteRef<{ - metricId: string; aggregationId: string; + metricId: string; }>; }, {}, @@ -57,8 +57,8 @@ const _default: OverridableFrontendPlugin< config: { allowedFilters: | { - type?: string | undefined; kind?: string | undefined; + type?: string | undefined; }[] | undefined; path: string | undefined; @@ -70,8 +70,8 @@ const _default: OverridableFrontendPlugin< configInput: { allowedFilters?: | { - type?: string | undefined; kind?: string | undefined; + type?: string | undefined; }[] | undefined; path?: string | undefined | undefined; @@ -121,8 +121,12 @@ const _default: OverridableFrontendPlugin< >; inputs: { layouts: ExtensionInput< - | ConfigurableExtensionDataRef - | ConfigurableExtensionDataRef, + | ConfigurableExtensionDataRef + | ConfigurableExtensionDataRef< + JSX_2.Element, + 'core.reactElement', + {} + >, { singleton: false; optional: true; From 6c4e04fcb6409b8c671703009cd7dd3dd6720f26 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:05:14 +0000 Subject: [PATCH 4/4] fix: address review feedback on PR #4589 - Upgrade filterEnabledMetrics catch logging from debug to warn level to surface provider lookup failures (error-handling finding) - Strip internal `enabled` field from metrics in API responses to avoid confusing consumers when code-level default differs from config (api-surface finding) - Add scheduler test for hasEnabledMetric guard that skips providers with all metrics disabled (test-coverage finding) - Fix inline type import syntax in metricUtils.test.ts for Babel compat Addresses review feedback on #4589 --- .../src/scheduler/index.test.ts | 45 +++++++++++++++++++ .../src/utils/metricUtils.test.ts | 3 +- .../src/utils/metricUtils.ts | 28 ++++++------ 3 files changed, 62 insertions(+), 14 deletions(-) diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts index e627a4e451d..8bf287c5c53 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts @@ -229,6 +229,51 @@ describe('Scheduler', () => { expect(initializedTasks).toEqual([]); }); + + it('should skip providers whose metrics are all disabled by config', async () => { + const disabledConfig = mockServices.rootConfig({ + data: { + scorecard: { + metricProviders: { + github: { + testMetric: { enabled: false }, + }, + }, + }, + }, + }); + + const disabledScheduler = Scheduler.create({ + auth: mockAuth, + catalog: mockCatalog, + config: disabledConfig, + logger: mockLogger, + scheduler: mockScheduler, + database: mockDatabase, + metricProvidersRegistry: mockRegistry, + thresholdEvaluator: new ThresholdEvaluator(), + thresholdResolver: new ThresholdResolver( + disabledConfig, + mockRegistry.listProviders(), + ), + }); + + (disabledScheduler as any).initializeTasksByProviders(); + + const initializedTasks = (disabledScheduler as any).tasks; + + // Only jira.testMetric should have a task; github.testMetric is disabled + expect(initializedTasks).toEqual([ + { + name: 'jira.testMetric', + task: mockPullTask, + }, + ]); + + expect(mockLogger.info).toHaveBeenCalledWith( + "Skipping provider 'github.testMetric': all metrics disabled", + ); + }); }); describe('startTask', () => { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts index 78d602cd920..611d8fd1069 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts @@ -15,7 +15,8 @@ */ import { mockServices } from '@backstage/backend-test-utils'; -import { ConfigReader, type JsonObject } from '@backstage/config'; +import { ConfigReader } from '@backstage/config'; +import type { JsonObject } from '@backstage/config'; import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; import { isMetricIdDisabled, isMetricEnabled } from './metricUtils'; import type { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts index cf800b47574..2fe124a2a63 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts @@ -167,17 +167,19 @@ export function filterEnabledMetrics( getProvider: (metricId: string) => MetricProvider, logger?: LoggerService, ): Metric[] { - return metrics.filter(m => { - try { - const provider = getProvider(m.id); - return isMetricEnabled(config, m, provider); - } catch (error) { - logger?.debug( - `Unable to resolve enabled state for metric '${ - m.id - }', treating as enabled: ${stringifyError(error)}`, - ); - return true; - } - }); + return metrics + .filter(m => { + try { + const provider = getProvider(m.id); + return isMetricEnabled(config, m, provider); + } catch (error) { + logger?.warn( + `Unable to resolve enabled state for metric '${ + m.id + }', treating as enabled: ${stringifyError(error)}`, + ); + return true; + } + }) + .map(({ enabled: _enabled, ...rest }) => rest as Metric); }