diff --git a/.changeset/gauge-count-distinct-value-expression.md b/.changeset/gauge-count-distinct-value-expression.md new file mode 100644 index 0000000000..98ec43087f --- /dev/null +++ b/.changeset/gauge-count-distinct-value-expression.md @@ -0,0 +1,12 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +'@hyperdx/api': patch +--- + +fix: count distinct attribute values on gauge metric charts + +`count_distinct` on a gauge metric ignored its value expression and counted the +distinct metric values instead, so counting `Attributes['name']` on an info +metric returned 1 or 2. It now counts the given expression. The default `Value` +expression and every other aggregation still use the bucketed metric value. diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 2264838536..ed1b95fdfc 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -273,6 +273,67 @@ describe('renderChartConfig', () => { expect(actual).toMatchSnapshot(); }); + describe('count_distinct on a gauge metric', () => { + const renderGaugeSelect = async (select: { + aggFn?: string; + valueExpression: string; + }) => + parameterizedQueryToSql( + await renderChartConfig( + { + ...gaugeConfiguration, + select: [ + { + aggFn: 'count_distinct', + metricName: 'argocd_app_info', + metricType: MetricsDataType.Gauge, + ...select, + }, + ], + }, + mockMetadata, + querySettings, + ), + ); + + it('counts the distinct values of an attribute expression', async () => { + const sql = await renderGaugeSelect({ + valueExpression: "Attributes['name']", + }); + expect(sql).toContain("count(DISTINCT Attributes['name'])"); + expect(sql).not.toContain('count(DISTINCT LastValue)'); + }); + + it('still counts the bucketed metric value for the default Value expression', async () => { + const sql = await renderGaugeSelect({ valueExpression: 'Value' }); + expect(sql).toContain('count(DISTINCT LastValue)'); + }); + + it('still counts the bucketed metric value for an expression over Value', async () => { + const sql = await renderGaugeSelect({ valueExpression: 'Value * 100' }); + expect(sql).toContain('count(DISTINCT LastValue)'); + expect(sql).not.toContain('Value * 100'); + }); + + it('counts an attribute whose key is Value', async () => { + const sql = await renderGaugeSelect({ + valueExpression: "Attributes['Value']", + }); + expect(sql).toContain("count(DISTINCT Attributes['Value'])"); + }); + + it('keeps other aggregations on the bucketed metric value', async () => { + const sql = await renderGaugeSelect({ + aggFn: 'max', + valueExpression: "Attributes['name']", + }); + expect(sql).toMatch( + /SELECT max\(\s*toFloat64OrDefault\(toString\(LastValue\)\)/, + ); + expect(sql).not.toContain("Attributes['name']"); + }); + }); + it('should generate sql for a single sum metric', async () => { const config: ChartConfigWithOptDateRange = { displayType: DisplayType.Line, diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 70dab887c4..39529a4416 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -2101,7 +2101,16 @@ async function translateMetricChartConfig( select: [ { ..._select, - valueExpression: 'LastValue', + // count_distinct may count an attribute, which Bucketed still carries; + // Bucketed has no Value column, only LastValue + valueExpression: + _select.aggFn === 'count_distinct' && + _select.valueExpression && + !/\bValue\b/.test( + _select.valueExpression.replace(/'(?:[^'\\]|\\.)*'/g, "''"), + ) + ? _select.valueExpression + : 'LastValue', aggCondition: '', // clear up the condition since the where clause is already applied at the upstream CTE }, ],