diff --git a/workspaces/scorecard/.changeset/string-metric-type.md b/workspaces/scorecard/.changeset/string-metric-type.md new file mode 100644 index 00000000000..2fde90a356e --- /dev/null +++ b/workspaces/scorecard/.changeset/string-metric-type.md @@ -0,0 +1,8 @@ +--- +'@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-module-catalog': minor +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': patch +--- + +Add string as an alternative MetricType and migrate CatalogRequiredAttributesMetricProvider from number to string metrics. diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.test.ts index 73a83e7957d..c06e18ee441 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.test.ts @@ -392,7 +392,7 @@ describe('CatalogRequiredAttributesMetricProvider', () => { expect(metrics).toHaveLength(1); metrics?.forEach(m => { - expect(m.type).toBe('number'); + expect(m.type).toBe('string'); }); }); @@ -472,22 +472,16 @@ describe('CatalogRequiredAttributesMetricProvider', () => { }); describe('calculateMetrics', () => { - it('should return "found" status code for existing field', async () => { + it('should return "found" status string for existing field', async () => { const provider = createCatalogRequiredAttributesMetricProvider( new ConfigReader(buildConfig({ title: titleMetric() })), ); const result = await provider?.calculateMetrics(componentEntity); - // The metric value is a numeric code mapping to "found" - const metrics = provider?.getMetrics(); - const titleMet = metrics?.find(m => m.id === 'catalog.title'); - const foundRule = titleMet?.thresholds.rules.find(r => r.key === 'found'); - const expectedCode = Number(foundRule?.expression.replace('==', '')); - - expect(result?.get('catalog.title')).toBe(expectedCode); + expect(result?.get('catalog.title')).toBe('found'); }); - it('should return "missed" status code for missing field', async () => { + it('should return "missed" status string for missing field', async () => { const entityWithoutTitle: Entity = { apiVersion: 'backstage.io/v1alpha1', kind: 'Component', @@ -499,14 +493,7 @@ describe('CatalogRequiredAttributesMetricProvider', () => { ); const result = await provider?.calculateMetrics(entityWithoutTitle); - const metrics = provider?.getMetrics(); - const titleMet = metrics?.find(m => m.id === 'catalog.title'); - const missedRule = titleMet?.thresholds.rules.find( - r => r.key === 'missed', - ); - const expectedCode = Number(missedRule?.expression.replace('==', '')); - - expect(result?.get('catalog.title')).toBe(expectedCode); + expect(result?.get('catalog.title')).toBe('missed'); }); it('should return "ok" for valid lifecycle value', async () => { @@ -515,12 +502,7 @@ describe('CatalogRequiredAttributesMetricProvider', () => { ); const result = await provider?.calculateMetrics(componentEntity); - const metrics = provider?.getMetrics(); - const lcMetric = metrics?.find(m => m.id === 'catalog.lifecycle'); - const okRule = lcMetric?.thresholds.rules.find(r => r.key === 'ok'); - const expectedCode = Number(okRule?.expression.replace('==', '')); - - expect(result?.get('catalog.lifecycle')).toBe(expectedCode); + expect(result?.get('catalog.lifecycle')).toBe('ok'); }); it('should return "invalid" for unknown lifecycle value', async () => { @@ -533,14 +515,7 @@ describe('CatalogRequiredAttributesMetricProvider', () => { ); const result = await provider?.calculateMetrics(entity); - const metrics = provider?.getMetrics(); - const lcMetric = metrics?.find(m => m.id === 'catalog.lifecycle'); - const invalidRule = lcMetric?.thresholds.rules.find( - r => r.key === 'invalid', - ); - const expectedCode = Number(invalidRule?.expression.replace('==', '')); - - expect(result?.get('catalog.lifecycle')).toBe(expectedCode); + expect(result?.get('catalog.lifecycle')).toBe('invalid'); }); it('should return "missed" for missing lifecycle value', async () => { @@ -553,14 +528,7 @@ describe('CatalogRequiredAttributesMetricProvider', () => { ); const result = await provider?.calculateMetrics(entity); - const metrics = provider?.getMetrics(); - const lcMetric = metrics?.find(m => m.id === 'catalog.lifecycle'); - const missedRule = lcMetric?.thresholds.rules.find( - r => r.key === 'missed', - ); - const expectedCode = Number(missedRule?.expression.replace('==', '')); - - expect(result?.get('catalog.lifecycle')).toBe(expectedCode); + expect(result?.get('catalog.lifecycle')).toBe('missed'); }); it('should handle empty string field with default mapping', async () => { @@ -574,14 +542,7 @@ describe('CatalogRequiredAttributesMetricProvider', () => { const result = await provider?.calculateMetrics(entity); // Default mapping: emptyString → 'missed' - const metrics = provider?.getMetrics(); - const titleMet = metrics?.find(m => m.id === 'catalog.title'); - const missedRule = titleMet?.thresholds.rules.find( - r => r.key === 'missed', - ); - const expectedCode = Number(missedRule?.expression.replace('==', '')); - - expect(result?.get('catalog.title')).toBe(expectedCode); + expect(result?.get('catalog.title')).toBe('missed'); }); it('should handle empty array field with default mapping', async () => { @@ -602,14 +563,7 @@ describe('CatalogRequiredAttributesMetricProvider', () => { ); const result = await provider?.calculateMetrics(entity); - const metrics = provider?.getMetrics(); - const tagsMetric = metrics?.find(m => m.id === 'catalog.tags'); - const missedRule = tagsMetric?.thresholds.rules.find( - r => r.key === 'missed', - ); - const expectedCode = Number(missedRule?.expression.replace('==', '')); - - expect(result?.get('catalog.tags')).toBe(expectedCode); + expect(result?.get('catalog.tags')).toBe('missed'); }); it('should handle multiple metrics on the same entity', async () => { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.ts index 1f080135532..52b2eece869 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-catalog/src/metricProviders/CatalogRequiredAttributesMetricProvider.ts @@ -113,28 +113,20 @@ function collectDistinctStatuses(statusMapping: StatusMapping): string[] { } /** - * Builds a mapping from status strings to numeric codes and generates - * threshold rules that map those codes back to status strings. + * Builds threshold rules for string metrics from the distinct statuses + * in a status mapping. Each status gets an `==statusKey` expression. */ -function buildStatusCodeMapping(statusMapping: StatusMapping): { - statusToCode: Map; - thresholds: ThresholdConfig; -} { +function buildStringThresholds(statusMapping: StatusMapping): ThresholdConfig { const statuses = collectDistinctStatuses(statusMapping); - const statusToCode = new Map(); - statuses.forEach((status, index) => { - statusToCode.set(status, index); - }); - - const rules = statuses.map((status, index) => ({ + const rules = statuses.map(status => ({ key: status, - expression: `==${index}`, + expression: `==${status}`, color: getDefaultColor(status), icon: getDefaultIcon(status), })); - return { statusToCode, thresholds: { rules } }; + return { rules }; } /** @@ -180,23 +172,20 @@ function getDefaultIcon(status: string): string { } export class CatalogRequiredAttributesMetricProvider - implements MetricProvider<'number'> + implements MetricProvider<'string'> { private readonly filter: object; private readonly metricConfigs: MetricConfig[]; - private readonly statusCodeMappings: Map< - string, - { statusToCode: Map; thresholds: ThresholdConfig } - >; + private readonly thresholdsByMetricId: Map; constructor(options: CatalogRequiredAttributesOptions) { this.filter = options.filter; this.metricConfigs = options.metrics; - this.statusCodeMappings = new Map(); + this.thresholdsByMetricId = new Map(); for (const metric of this.metricConfigs) { - this.statusCodeMappings.set( + this.thresholdsByMetricId.set( metric.id, - buildStatusCodeMapping(metric.statusMapping), + buildStringThresholds(metric.statusMapping), ); } } @@ -209,15 +198,15 @@ export class CatalogRequiredAttributesMetricProvider return 'catalog.requiredAttributes'; } - getMetrics(): Metric<'number'>[] { + getMetrics(): Metric<'string'>[] { return this.metricConfigs.map(metric => { - const mapping = this.statusCodeMappings.get(metric.id)!; + const thresholds = this.thresholdsByMetricId.get(metric.id)!; return { id: `catalog.${metric.id}`, title: metric.title, description: metric.description, - type: 'number' as const, - thresholds: mapping.thresholds, + type: 'string' as const, + thresholds, }; }); } @@ -226,8 +215,8 @@ export class CatalogRequiredAttributesMetricProvider return this.filter as Record; } - async calculateMetrics(entity: Entity): Promise> { - const results = new Map(); + async calculateMetrics(entity: Entity): Promise> { + const results = new Map(); for (const metric of this.metricConfigs) { const status = evaluateFieldStatus( @@ -236,11 +225,7 @@ export class CatalogRequiredAttributesMetricProvider metric.statusMapping, ); - const mapping = this.statusCodeMappings.get(metric.id)!; - const code = mapping.statusToCode.get(status); - if (code !== undefined) { - results.set(`catalog.${metric.id}`, code); - } + results.set(`catalog.${metric.id}`, status); } return results; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts index 9d6509ae87b..96e6117f076 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts @@ -59,13 +59,13 @@ export const createGetEntityMetricsAction = ({ metadata: z.object({ title: z.string(), description: z.string(), - type: z.enum(['number', 'boolean']), + type: z.enum(['number', 'boolean', 'string']), unit: z.string().optional(), history: z.boolean().optional(), defaultVisualization: z.enum(['value', 'sparkline']).optional(), }), result: z.object({ - value: z.union([z.number(), z.boolean(), z.null()]), + value: z.union([z.number(), z.boolean(), z.string(), z.null()]), timestamp: z.string(), thresholdResult: z.object({ definition: z.unknown().optional(), diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts index 0ce359a2353..2b2139760bc 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts @@ -50,7 +50,7 @@ export const createListMetricsAction = ({ id: z.string(), title: z.string(), description: z.string(), - type: z.enum(['number', 'boolean']), + type: z.enum(['number', 'boolean', 'string']), unit: z.string().optional(), history: z.boolean().optional(), defaultVisualization: z.enum(['value', 'sparkline']).optional(), diff --git a/workspaces/scorecard/plugins/scorecard-common/report.api.md b/workspaces/scorecard/plugins/scorecard-common/report.api.md index 1936c2831c1..f9ab8e6e801 100644 --- a/workspaces/scorecard/plugins/scorecard-common/report.api.md +++ b/workspaces/scorecard/plugins/scorecard-common/report.api.md @@ -106,7 +106,7 @@ export type EntityMetricDetail = { entityNamespace?: string; entityKind?: string; owner?: string; - metricValue?: number | boolean | null; + metricValue?: number | boolean | string | null; timestamp?: string; status?: string | null; }; @@ -188,13 +188,15 @@ export type MetricTimeSeriesResponse = { }; // @public (undocumented) -export type MetricType = 'number' | 'boolean'; +export type MetricType = 'number' | 'boolean' | 'string'; // @public (undocumented) export type MetricValue = T extends 'number' ? number : T extends 'boolean' ? boolean + : T extends 'string' + ? string : never; // @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 b5d08a841d7..13e088fb70b 100644 --- a/workspaces/scorecard/plugins/scorecard-common/src/types/Metric.ts +++ b/workspaces/scorecard/plugins/scorecard-common/src/types/Metric.ts @@ -19,7 +19,7 @@ import { ThresholdConfig, ThresholdResult } from './threshold'; /** * @public */ -export type MetricType = 'number' | 'boolean'; +export type MetricType = 'number' | 'boolean' | 'string'; /** * Default visualization for a metric on the entity scorecard. @@ -36,6 +36,8 @@ export type MetricValue = T extends 'number' ? number : T extends 'boolean' ? boolean + : T extends 'string' + ? string : never; /** @@ -84,7 +86,7 @@ export type EntityMetricDetail = { entityNamespace?: string; entityKind?: string; owner?: string; - metricValue?: number | boolean | null; + metricValue?: number | boolean | string | null; timestamp?: string; status?: string | null; }; diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.test.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.test.ts index 441bd498080..67e290b86ab 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.test.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.test.ts @@ -106,6 +106,44 @@ describe('parseThresholdExpression', () => { }); }); + describe('parseThresholdExpression - string metrics', () => { + it.each([ + { + expression: '==found', + expectedResult: { operator: '==', value: 'found' }, + }, + { + expression: '==missed', + expectedResult: { operator: '==', value: 'missed' }, + }, + { + expression: '!=invalid', + expectedResult: { operator: '!=', value: 'invalid' }, + }, + { + expression: '==ok', + expectedResult: { operator: '==', value: 'ok' }, + }, + ])( + 'should parse string expression $expression correctly', + ({ expression, expectedResult }) => { + const result = parseThresholdExpression(expression, 'string'); + expect(result).toEqual(expectedResult); + }, + ); + + it('should handle whitespace in string expressions', () => { + const result = parseThresholdExpression(' == found ', 'string'); + expect(result).toEqual({ operator: '==', value: 'found' }); + }); + + it('should reject range expressions for string metrics', () => { + expect(() => parseThresholdExpression('10-60', 'string')).toThrow( + ThresholdConfigFormatError, + ); + }); + }); + describe('parseThresholdExpression - error handling', () => { it.each([ { diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.ts index 0bdc8503533..504260f133a 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/parseThresholdExpression.ts @@ -84,6 +84,10 @@ function parseComparisonOperator( ); } + if (targetType === 'string') { + return { operator, value: valueStr }; + } + return undefined; }