diff --git a/.changeset/metric-series-scope-name.md b/.changeset/metric-series-scope-name.md new file mode 100644 index 0000000000..5afac6de13 --- /dev/null +++ b/.changeset/metric-series-scope-name.md @@ -0,0 +1,14 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +'@hyperdx/api': patch +--- + +fix: keep metric series from different instrumentation scopes apart + +Two instrumentation scopes can report the same metric with the same resource, +scope and point attributes. Metric charts and alerts keyed a series on those +attributes only, so they read the two scopes as one series: a gauge kept one +scope's value, a counter's `increase` and a histogram's count compared one +scope's total with the other's, and a group-by on `ScopeName` showed one scope. +The series key now includes `ScopeName`. diff --git a/packages/api/src/clickhouse/__tests__/renderChartConfig.int.test.ts b/packages/api/src/clickhouse/__tests__/renderChartConfig.int.test.ts index 4f93e19282..aa9850be6c 100644 --- a/packages/api/src/clickhouse/__tests__/renderChartConfig.int.test.ts +++ b/packages/api/src/clickhouse/__tests__/renderChartConfig.int.test.ts @@ -1139,7 +1139,7 @@ describe('renderChartConfig', () => { // Row B: ResourceAttributes={service.name: api}, Attributes={host: h1} // // AttributesHash is now computed as the variadic - // cityHash64(ScopeAttributes, ResourceAttributes, Attributes) + // cityHash64(ScopeName, ScopeAttributes, ResourceAttributes, Attributes) // for all metric schemas (Map and JSON), so the two rows hash // distinctly: each lands in its own series. Both rows are then the // first row of their respective series, the lagInFrame Rate is NULL diff --git a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts index 8a7045229d..24e5fe82c1 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts @@ -7738,7 +7738,7 @@ describe('checkAlerts', () => { // Insert gauge metrics for two different services // Note: ResourceAttributes must differ per service so that - // AttributesHash (variadic cityHash64(ScopeAttributes, ResourceAttributes, Attributes)) + // AttributesHash (variadic cityHash64(ScopeName, ScopeAttributes, ResourceAttributes, Attributes)) // produces distinct hashes. Otherwise, the Bucketed CTE collapses all rows into one group. const gaugePoints = [ // service-a: high CPU values (should trigger alert) diff --git a/packages/common-utils/src/__tests__/metricScopeName.int.test.ts b/packages/common-utils/src/__tests__/metricScopeName.int.test.ts new file mode 100644 index 0000000000..f129e39100 --- /dev/null +++ b/packages/common-utils/src/__tests__/metricScopeName.int.test.ts @@ -0,0 +1,456 @@ +import type { ClickHouseClient } from '@clickhouse/client'; +import { createClient } from '@clickhouse/client'; + +import { ClickhouseClient as HdxClickhouseClient } from '@/clickhouse/node'; +import { Metadata, MetadataCache } from '@/core/metadata'; +import { + ChartConfigWithOptDateRange, + DisplayType, + MetricsDataType, +} from '@/types'; + +// Two instrumentation scopes in one process can report the same metric with +// the same resource, scope and point attributes. The OTel data model makes +// them two streams, so the metric queries must not merge them into one series. +describe('metric series from different scopes (integration)', () => { + let client: ClickHouseClient; + let hdxClient: HdxClickhouseClient; + let metadata: Metadata; + + const DATABASE = 'default'; + const GAUGE_TABLE = 'scope_name_gauge_int_test'; + const SUM_TABLE = 'scope_name_sum_int_test'; + const HIST_TABLE = 'scope_name_histogram_int_test'; + const EXP_HIST_TABLE = 'scope_name_exp_histogram_int_test'; + // Custom tables: a Nullable ScopeName, and a histogram without ScopeName. + const NULLABLE_GAUGE_TABLE = 'scope_name_nullable_gauge_int_test'; + const NO_SCOPE_HIST_TABLE = 'scope_name_no_scope_histogram_int_test'; + + const SCOPE_A = 'scope.a'; + const SCOPE_B = 'scope.b'; + const MINUTES = [0, 1, 2]; + + // Inserts are server-local strings; the CI ClickHouse runs in UTC. + const insertTs = (minute: number) => `2025-04-15 10:0${minute}:00`; + const bucket = (minute: number) => `2025-04-15T10:0${minute}:00Z`; + + // Columns of docker/otel-collector/schema/seed/00003_otel_metrics.sql that + // every metric table shares. + const COMMON_COLUMNS = ` + ResourceAttributes Map(LowCardinality(String), String), + ResourceSchemaUrl String, + ScopeName String, + ScopeVersion String, + ScopeAttributes Map(LowCardinality(String), String), + ScopeDroppedAttrCount UInt32, + ScopeSchemaUrl String, + ServiceName LowCardinality(String), + MetricName LowCardinality(String), + MetricDescription String, + MetricUnit String, + Attributes Map(LowCardinality(String), String), + StartTimeUnix DateTime, + TimeUnix DateTime, + Flags UInt32`; + const ENGINE = `ENGINE = MergeTree PARTITION BY toDate(TimeUnix) + ORDER BY (ServiceName, MetricName, toStartOfHour(TimeUnix), cityHash64(Attributes), TimeUnix)`; + + // Only ScopeName tells the two series apart. + const commonFields = (metricName: string, minute: number, scope: string) => ({ + ResourceAttributes: { 'service.name': 'svc' }, + ResourceSchemaUrl: '', + ScopeName: scope, + ScopeVersion: '', + ScopeAttributes: {}, + ScopeDroppedAttrCount: 0, + ScopeSchemaUrl: '', + ServiceName: 'svc', + MetricName: metricName, + MetricDescription: '', + MetricUnit: '', + Attributes: { 'http.route': '/api' }, + StartTimeUnix: insertTs(0), + TimeUnix: insertTs(minute), + Flags: 0, + }); + + const metricTables = { + gauge: GAUGE_TABLE, + sum: SUM_TABLE, + histogram: HIST_TABLE, + summary: 'unused', + 'exponential histogram': EXP_HIST_TABLE, + }; + + const queryRows = async ( + metricType: MetricsDataType, + metricName: string, + select: { aggFn: string; level?: number }, + { + groupBy, + tables = metricTables, + }: { groupBy?: string; tables?: typeof metricTables } = {}, + ) => { + const result = await hdxClient.queryChartConfig({ + config: { + displayType: DisplayType.Line, + connection: 'test-connection', + metricTables: tables, + from: { databaseName: DATABASE, tableName: '' }, + select: [ + { + ...select, + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + alias: 'Value', + metricName, + metricType, + }, + ], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'TimeUnix', + dateRange: [new Date('2025-04-14'), new Date('2025-04-16')], + granularity: '1 minute', + ...(groupBy ? { groupBy } : {}), + } as ChartConfigWithOptDateRange, + metadata, + querySettings: undefined, + }); + return result.data as Array< + { __hdx_time_bucket: string; Value: unknown } & Record + >; + }; + + const runConfig = async ( + metricType: MetricsDataType, + metricName: string, + select: { aggFn: string; level?: number }, + tables = metricTables, + ) => + new Map( + (await queryRows(metricType, metricName, select, { tables })).map(row => [ + row.__hdx_time_bucket, + Number(row.Value), + ]), + ); + + beforeAll(async () => { + const host = process.env.CLICKHOUSE_HOST || 'http://localhost:8123'; + const username = process.env.CLICKHOUSE_USER || 'default'; + const password = process.env.CLICKHOUSE_PASSWORD || ''; + + client = createClient({ url: host, username, password }); + hdxClient = new HdxClickhouseClient({ host, username, password }); + + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${GAUGE_TABLE} ( + ${COMMON_COLUMNS}, + Value Float64 + ) ${ENGINE}`, + }); + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${SUM_TABLE} ( + ${COMMON_COLUMNS}, + Value Float64, + AggregationTemporality Int32, + IsMonotonic Bool + ) ${ENGINE}`, + }); + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${HIST_TABLE} ( + ${COMMON_COLUMNS}, + Count UInt64, + Sum Float64, + BucketCounts Array(UInt64), + ExplicitBounds Array(Float64), + Min Float64, + Max Float64, + AggregationTemporality Int32 + ) ${ENGINE}`, + }); + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${EXP_HIST_TABLE} ( + ${COMMON_COLUMNS}, + Count UInt64, + Sum Float64, + Scale Int32, + ZeroCount UInt64, + PositiveOffset Int32, + PositiveBucketCounts Array(UInt64), + NegativeOffset Int32, + NegativeBucketCounts Array(UInt64), + Min Float64, + Max Float64, + AggregationTemporality Int32 + ) ${ENGINE}`, + }); + + // Both scopes report at the same time, as one SDK reader does. + await client.insert({ + table: `${DATABASE}.${GAUGE_TABLE}`, + values: MINUTES.flatMap(m => [ + { ...commonFields('gauge.cpu', m, SCOPE_A), Value: 10 }, + { ...commonFields('gauge.cpu', m, SCOPE_B), Value: 20 }, + ]), + format: 'JSONEachRow', + }); + + // Cumulative counters: scope a stays at 1000, scope b goes up by 5 a + // minute. sum.single has the scope b series only. + const sumRow = ( + metricName: string, + minute: number, + scope: string, + value: number, + ) => ({ + ...commonFields(metricName, minute, scope), + Value: value, + AggregationTemporality: 2, + IsMonotonic: true, + }); + await client.insert({ + table: `${DATABASE}.${SUM_TABLE}`, + values: MINUTES.flatMap(m => [ + sumRow('sum.requests', m, SCOPE_A, 1000), + sumRow('sum.requests', m, SCOPE_B, 5 * m), + sumRow('sum.single', m, SCOPE_B, 5 * m), + ]), + format: 'JSONEachRow', + }); + + // Cumulative histograms: scope a holds 1000 old observations in (0, 10], + // scope b gets 5 new observations a minute in (10, 30]. + const histRow = ( + minute: number, + scope: string, + bucketCounts: number[], + ) => ({ + ...commonFields('hist.latency', minute, scope), + Count: bucketCounts.reduce((a, b) => a + b, 0), + Sum: 0, + BucketCounts: bucketCounts, + ExplicitBounds: [10, 30], + Min: 0, + Max: 0, + AggregationTemporality: 2, + }); + await client.insert({ + table: `${DATABASE}.${HIST_TABLE}`, + values: MINUTES.flatMap(m => [ + histRow(m, SCOPE_A, [1000, 0, 0]), + histRow(m, SCOPE_B, [0, 5 * m, 0]), + ]), + format: 'JSONEachRow', + }); + + // Cumulative native histograms at scale 3, where bucket 8 is (2, 2.18] + // and bucket 16 is (4, 4.36]: scope a holds 1000 old observations in + // bucket 8, scope b gets 5 new observations a minute in bucket 16. + const expHistRow = ( + minute: number, + scope: string, + offset: number, + count: number, + ) => ({ + ...commonFields('exphist.latency', minute, scope), + Count: count, + Sum: 0, + Scale: 3, + ZeroCount: 0, + PositiveOffset: offset, + PositiveBucketCounts: [count], + NegativeOffset: 0, + NegativeBucketCounts: [], + Min: 0, + Max: 0, + AggregationTemporality: 2, + }); + await client.insert({ + table: `${DATABASE}.${EXP_HIST_TABLE}`, + values: MINUTES.flatMap(m => [ + expHistRow(m, SCOPE_A, 8, 1000), + expHistRow(m, SCOPE_B, 16, 5 * m), + ]), + format: 'JSONEachRow', + }); + + // Two hosts with a NULL ScopeName: their attributes keep them apart. + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${NULLABLE_GAUGE_TABLE} ( + ${COMMON_COLUMNS.replace('ScopeName String', 'ScopeName Nullable(String)')}, + Value Float64 + ) ${ENGINE}`, + }); + await client.insert({ + table: `${DATABASE}.${NULLABLE_GAUGE_TABLE}`, + values: MINUTES.flatMap(m => + [ + ['a', 10], + ['b', 20], + ].map(([host, value]) => ({ + ...commonFields('gauge.cpu', m, SCOPE_A), + ScopeName: null, + Attributes: { host }, + Value: value, + })), + ), + format: 'JSONEachRow', + }); + + // A histogram table without ScopeName, with one series that gets 5 new + // observations a minute. + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${NO_SCOPE_HIST_TABLE} ( + ${COMMON_COLUMNS.replace('ScopeName String,', '')}, + Count UInt64, + Sum Float64, + BucketCounts Array(UInt64), + ExplicitBounds Array(Float64), + Min Float64, + Max Float64, + AggregationTemporality Int32 + ) ${ENGINE}`, + }); + await client.insert({ + table: `${DATABASE}.${NO_SCOPE_HIST_TABLE}`, + values: MINUTES.map(m => { + const row: Record = histRow(m, SCOPE_B, [0, 5 * m, 0]); + delete row.ScopeName; + return row; + }), + format: 'JSONEachRow', + }); + }); + + beforeEach(() => { + metadata = new Metadata(hdxClient, new MetadataCache()); + }); + + afterAll(async () => { + for (const table of [ + GAUGE_TABLE, + SUM_TABLE, + HIST_TABLE, + EXP_HIST_TABLE, + NULLABLE_GAUGE_TABLE, + NO_SCOPE_HIST_TABLE, + ]) { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${table}`, + }); + } + await hdxClient.close(); + await client.close(); + }); + + it.each([ + ['sum', 30], + ['count', 2], + ])('reads a gauge %s over both scopes', async (aggFn, expected) => { + const values = await runConfig(MetricsDataType.Gauge, 'gauge.cpu', { + aggFn, + }); + for (const m of MINUTES) { + expect(values.get(bucket(m))).toBe(expected); + } + }); + + it('gives each scope its own gauge group', async () => { + const rows = await queryRows( + MetricsDataType.Gauge, + 'gauge.cpu', + { aggFn: 'sum' }, + { groupBy: 'ScopeName' }, + ); + const byScope = Object.fromEntries( + rows + .filter(row => row.__hdx_time_bucket === bucket(1)) + .map(row => [row.ScopeName, Number(row.Value)]), + ); + expect(byScope).toEqual({ [SCOPE_A]: 10, [SCOPE_B]: 20 }); + }); + + it('computes counter increase per scope', async () => { + const values = await runConfig(MetricsDataType.Sum, 'sum.requests', { + aggFn: 'increase', + }); + expect(values.get(bucket(1))).toBe(5); + expect(values.get(bucket(2))).toBe(5); + }); + + it('keeps the increase of a counter in one scope unchanged', async () => { + const values = await runConfig(MetricsDataType.Sum, 'sum.single', { + aggFn: 'increase', + }); + expect(values.get(bucket(1))).toBe(5); + expect(values.get(bucket(2))).toBe(5); + }); + + it('counts histogram observations per scope', async () => { + const counts = await runConfig(MetricsDataType.Histogram, 'hist.latency', { + aggFn: 'count', + }); + expect(counts.get(bucket(1))).toBe(5); + expect(counts.get(bucket(2))).toBe(5); + }); + + it('computes histogram quantiles from the new observations only', async () => { + const p50 = await runConfig(MetricsDataType.Histogram, 'hist.latency', { + aggFn: 'quantile', + level: 0.5, + }); + expect(p50.get(bucket(1))).toBeGreaterThan(10); + expect(p50.get(bucket(1))).toBeLessThanOrEqual(30); + }); + + it('counts exponential histogram observations per scope', async () => { + const counts = await runConfig( + MetricsDataType.ExponentialHistogram, + 'exphist.latency', + { aggFn: 'count' }, + ); + expect(counts.get(bucket(1))).toBe(5); + expect(counts.get(bucket(2))).toBe(5); + }); + + it('computes exponential histogram quantiles from the new observations only', async () => { + const p50 = await runConfig( + MetricsDataType.ExponentialHistogram, + 'exphist.latency', + { aggFn: 'quantile', level: 0.5 }, + ); + expect(p50.get(bucket(1))).toBeGreaterThan(4); + expect(p50.get(bucket(1))).toBeLessThanOrEqual(2 ** (17 / 8)); + }); + + it.each([ + ['sum', 30], + ['count', 2], + ])( + 'keeps series with a NULL ScopeName apart (%s)', + async (aggFn, expected) => { + const values = await runConfig( + MetricsDataType.Gauge, + 'gauge.cpu', + { aggFn }, + { ...metricTables, gauge: NULLABLE_GAUGE_TABLE }, + ); + for (const m of MINUTES) { + expect(values.get(bucket(m))).toBe(expected); + } + }, + ); + + it('reads a histogram table without ScopeName', async () => { + const counts = await runConfig( + MetricsDataType.Histogram, + 'hist.latency', + { aggFn: 'count' }, + { ...metricTables, histogram: NO_SCOPE_HIST_TABLE }, + ); + expect(counts.get(bucket(1))).toBe(5); + expect(counts.get(bucket(2))).toBe(5); + }); +}); diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 44cd80248b..a83132a945 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -987,9 +987,10 @@ describe('queryChartConfig Integration Tests', () => { const OTEL_ORDER_BY = `ORDER BY (ServiceName, MetricName, toStartOfHour(TimeUnix), cityHash64(Attributes), TimeUnix)`; - // The per-series key is cityHash64(ScopeAttributes, ResourceAttributes, - // Attributes); give every service a distinct resource attribute so two - // services never collapse into one attributes-hash series. + // The per-series key is cityHash64(ScopeName, ScopeAttributes, + // ResourceAttributes, Attributes) (see renderMetricSeriesKey); give every + // service a distinct resource attribute so two services never collapse + // into one attributes-hash series. const commonFields = (metricName: string, ts: string, service: string) => ({ ResourceAttributes: { 'service.name': service }, ResourceSchemaUrl: '', diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index c025e687ee..428ce56d02 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -1039,6 +1039,110 @@ describe('renderChartConfig', () => { ); }); + // Two scopes can report the same metric with the same attributes. They are + // two OTel streams, so ScopeName must be part of the series key. + describe('metric series key', () => { + const KEY_WITH_SCOPE = + 'cityHash64(ScopeName, ScopeAttributes, ResourceAttributes, Attributes)'; + const KEY_WITH_NULLABLE_SCOPE = + "cityHash64(ifNull(ScopeName, ''), ScopeAttributes, ResourceAttributes, Attributes)"; + const KEY_WITHOUT_SCOPE = + 'cityHash64(ScopeAttributes, ResourceAttributes, Attributes)'; + + const mockScopeName = (type: string) => { + mockMetadata.getColumn = jest + .fn() + .mockImplementation(async ({ column }) => + column === 'ScopeName' ? { name: 'ScopeName', type } : undefined, + ); + }; + + const renderMetricSql = async ( + metricType: MetricsDataType, + select: { aggFn: 'avg' | 'count' | 'quantile'; level?: number }, + ) => { + const rendered = await renderChartConfig( + { + ...gaugeConfiguration, + select: [ + { + ...select, + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + metricName: 'test.metric', + metricType, + }, + ], + } as ChartConfigWithOptDateRange, + mockMetadata, + querySettings, + ); + return parameterizedQueryToSql(rendered); + }; + + const cases = [ + [MetricsDataType.Gauge, { aggFn: 'avg' }], + [MetricsDataType.Sum, { aggFn: 'avg' }], + [MetricsDataType.Histogram, { aggFn: 'quantile', level: 0.5 }], + [MetricsDataType.Histogram, { aggFn: 'count' }], + [MetricsDataType.ExponentialHistogram, { aggFn: 'quantile', level: 0.5 }], + [MetricsDataType.ExponentialHistogram, { aggFn: 'count' }], + ] as const; + + it.each(cases)( + 'keys each %s series on its ScopeName (%o)', + async (metricType, select) => { + mockScopeName('String'); + const sql = await renderMetricSql(metricType, select); + expect(sql).toContain(KEY_WITH_SCOPE); + expect(sql).not.toContain(KEY_WITHOUT_SCOPE); + }, + ); + + // Gauge and sum queries already read ScopeName, so only histogram tables + // can lack it. + it.each( + cases.filter( + ([metricType]) => + metricType === MetricsDataType.Histogram || + metricType === MetricsDataType.ExponentialHistogram, + ), + )( + 'reads a table without a ScopeName column as before (%s, %o)', + async (metricType, select) => { + const sql = await renderMetricSql(metricType, select); + expect(sql).toContain(KEY_WITHOUT_SCOPE); + expect(sql).not.toContain('ScopeName'); + }, + ); + + it('reads the table as before when the ScopeName lookup fails', async () => { + mockMetadata.getColumn = jest + .fn() + .mockImplementation(async ({ column }) => { + if (column === 'ScopeName') throw new Error('DESCRIBE failed'); + return undefined; + }); + const sql = await renderMetricSql(MetricsDataType.Histogram, { + aggFn: 'count', + }); + expect(sql).toContain(KEY_WITHOUT_SCOPE); + }); + + it.each([ + ['LowCardinality(String)', KEY_WITH_SCOPE], + ['Nullable(String)', KEY_WITH_NULLABLE_SCOPE], + ['LowCardinality(Nullable(String))', KEY_WITH_NULLABLE_SCOPE], + ])('keys on a ScopeName of type %s', async (scopeNameType, expected) => { + mockScopeName(scopeNameType); + const sql = await renderMetricSql(MetricsDataType.Gauge, { + aggFn: 'avg', + }); + expect(sql).toContain(expected); + }); + }); + describe('histogram metric queries', () => { describe('quantile', () => { it('should generate a whole-range query without a time dimension', async () => { @@ -4663,7 +4767,7 @@ describe('renderChartConfig', () => { // The Map-schema vs JSON-schema attrHashExpr distinction was collapsed in // HDX-4466. Both schemas now render the same variadic - // cityHash64(ScopeAttributes, ResourceAttributes, Attributes) expression, + // cityHash64 key (see renderMetricSeriesKey), // which works for both Map(LowCardinality(String), String) and JSON // attribute columns. Coverage of the variadic form lives in the regenerated // gauge / sum / histogram snapshots earlier in this file plus the diff --git a/packages/common-utils/src/core/histogram.ts b/packages/common-utils/src/core/histogram.ts index 81d0fdb623..edc70f0b50 100644 --- a/packages/common-utils/src/core/histogram.ts +++ b/packages/common-utils/src/core/histogram.ts @@ -11,6 +11,8 @@ type HistogramCountInput = { from: TemplatedInput; where: TemplatedInput; valueAlias: TemplatedInput; + // Per-series key, from renderMetricSeriesKey in renderChartConfig.ts. + attrHashExpr: string; }; export const GROUP_ALIAS = 'group'; @@ -53,14 +55,6 @@ const renderDimensionGroupBy = ({ return rendered.sql.length > 0 ? chSql`GROUP BY ${rendered}` : chSql``; }; -// SQL expression for hashing metric attributes into a per-series key. -// Variadic cityHash64 over the three attribute scopes — works for both -// Map(LowCardinality(String), String) and JSON columns. Prior to HDX-4466 -// the Map-schema path wrapped the three maps in mapConcat() before hashing; -// see the ticket for the cross-scope same-key implications of the switch. -const ATTR_HASH_EXPR = - 'cityHash64(ScopeAttributes, ResourceAttributes, Attributes)'; - export const translateHistogram = ({ select, ...rest @@ -71,6 +65,7 @@ export const translateHistogram = ({ from: TemplatedInput; where: TemplatedInput; valueAlias: TemplatedInput; + attrHashExpr: string; }) => { if (select.aggFn === 'quantile') { if (!('level' in select) || select.level === null) @@ -92,6 +87,7 @@ const translateHistogramCount = ({ from, where, valueAlias, + attrHashExpr, }: HistogramCountInput): WithClauses => [ { name: 'source', @@ -101,7 +97,7 @@ const translateHistogramCount = ({ AggregationTemporality, ${renderTimeBucketProjection(timeBucketSelect)} ${groupBy ? chSql`[${groupBy}] AS ${GROUP_ALIAS},` : ''} - ${ATTR_HASH_EXPR} AS attr_hash, + ${attrHashExpr} AS attr_hash, cityHash64(ExplicitBounds) AS bounds_hash, toInt64(Count) AS current_count, lagInFrame(toNullable(current_count), 1, NULL) OVER ( @@ -140,6 +136,7 @@ const translateHistogramQuantile = ({ from, where, valueAlias, + attrHashExpr, level, }: { timeBucketSelect?: TemplatedInput; @@ -147,6 +144,7 @@ const translateHistogramQuantile = ({ from: TemplatedInput; where: TemplatedInput; valueAlias: TemplatedInput; + attrHashExpr: string; level: number; }): WithClauses => [ { @@ -189,7 +187,7 @@ const translateHistogramQuantile = ({ ExplicitBounds, ResourceAttributes, Attributes,${groupBy ? chSql`[${groupBy}] as ${GROUP_ALIAS},` : ''} - ${ATTR_HASH_EXPR} AS attr_hash, + ${attrHashExpr} AS attr_hash, cityHash64(ExplicitBounds) AS bounds_hash, CAST(BucketCounts AS Array(Int64)) counts FROM ${from} @@ -259,6 +257,7 @@ export const translateExponentialHistogram = ({ from: TemplatedInput; where: TemplatedInput; valueAlias: TemplatedInput; + attrHashExpr: string; }) => { if (select.aggFn === 'quantile') { if (!('level' in select) || select.level === null) @@ -282,6 +281,7 @@ const translateExponentialHistogramCount = ({ from, where, valueAlias, + attrHashExpr, }: HistogramCountInput): WithClauses => [ { name: 'source', @@ -293,7 +293,7 @@ const translateExponentialHistogramCount = ({ AggregationTemporality, ${renderTimeBucketProjection(timeBucketSelect)} ${groupBy ? chSql`[${groupBy}] AS ${GROUP_ALIAS},` : ''} - ${ATTR_HASH_EXPR} AS attr_hash, + ${attrHashExpr} AS attr_hash, toInt64(Count) AS current_count, count() OVER prev_row = 0 AS is_first_series_point, toInt64(any(Count) OVER prev_row) AS previous_count, @@ -335,6 +335,7 @@ const translateExponentialHistogramQuantile = ({ from, where, valueAlias, + attrHashExpr, level, }: { timeBucketSelect?: TemplatedInput; @@ -342,6 +343,7 @@ const translateExponentialHistogramQuantile = ({ from: TemplatedInput; where: TemplatedInput; valueAlias: TemplatedInput; + attrHashExpr: string; level: number; }): WithClauses => [ // Filter for the relevant source data @@ -354,7 +356,7 @@ const translateExponentialHistogramQuantile = ({ StartTimeUnix, AggregationTemporality, Scale, - ${ATTR_HASH_EXPR} AS attr_hash, + ${attrHashExpr} AS attr_hash, ${groupBy ? chSql`[${groupBy}] as ${GROUP_ALIAS},` : ''} ZeroCount, PositiveOffset, diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index a38d8e6567..2c7bd0d8ab 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -2055,6 +2055,37 @@ async function renderMetricWhere( : where; } +/** + * Per-series key of a metric source CTE (AttributesHash / attr_hash). The + * variadic cityHash64 works for both Map(LowCardinality(String), String) and + * JSON attribute columns (HDX-4466). ScopeName is in the key because two + * instrumentation scopes can report the same metric with the same attributes, + * and OTel makes them two streams. Every metric type uses this key, but + * HyperDX does not require ScopeName on metric tables. The gauge and sum + * Bucketed CTEs select ScopeName anyway; the histogram queries did not read + * it before. So a table without it (or a failed column lookup) keeps the key + * without it, which is the query from before ScopeName was added. + * A NULL argument makes cityHash64 return NULL, which would merge every series + * with a NULL ScopeName, so a Nullable ScopeName is hashed as ''. + */ +async function renderMetricSeriesKey( + metadata: Metadata, + table: { databaseName: string; tableName: string; connectionId: string }, +): Promise { + let scopeName = ''; + try { + const column = await metadata.getColumn({ ...table, column: 'ScopeName' }); + if (column != null) { + scopeName = column.type.includes('Nullable(') + ? "ifNull(ScopeName, ''), " + : 'ScopeName, '; + } + } catch (e) { + console.warn('Failed to look up the ScopeName column of a metric table', e); + } + return `cityHash64(${scopeName}ScopeAttributes, ResourceAttributes, Attributes)`; +} + async function translateMetricChartConfig( chartConfig: BuilderChartConfigWithOptDateRangeEx, metadata: Metadata, @@ -2090,9 +2121,7 @@ async function translateMetricChartConfig( MetricsDataType.Histogram in metricTables && metricTables[MetricsDataType.Histogram]; - // AttributesHash is computed inline with a variadic cityHash64 call - // (HDX-4466). This works for both Map(LowCardinality(String), String) and - // JSON attribute columns, so no schema detection round-trip is needed. + // AttributesHash comes from renderMetricSeriesKey (see its comment). if ( metricType === MetricsDataType.Gauge && @@ -2143,6 +2172,11 @@ async function translateMetricChartConfig( bucketedColumns: GAUGE_BUCKETED_COLUMNS, groupBy: chartConfig.groupBy, }); + const seriesKey = await renderMetricSeriesKey(metadata, { + databaseName: from.databaseName, + tableName: metricTables[MetricsDataType.Gauge], + connectionId: chartConfig.connection, + }); return { ...restChartConfig, @@ -2152,7 +2186,7 @@ async function translateMetricChartConfig( sql: chSql` SELECT *, - cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash${{ UNSAFE_RAW_SQL: computedColumns.select }} + ${{ UNSAFE_RAW_SQL: seriesKey }} AS AttributesHash${{ UNSAFE_RAW_SQL: computedColumns.select }} FROM ${renderFrom({ from: { ...from, tableName: metricTables[MetricsDataType.Gauge] }, isRenderingRawSqlTemplate: chartConfig.isRenderingRawSqlTemplate, metricType: MetricsDataType.Gauge })} WHERE ${where} `, @@ -2255,6 +2289,11 @@ async function translateMetricChartConfig( bucketedColumns: SUM_BUCKETED_COLUMNS, groupBy: chartConfig.groupBy, }); + const seriesKey = await renderMetricSeriesKey(metadata, { + databaseName: from.databaseName, + tableName: metricTables[MetricsDataType.Sum], + connectionId: chartConfig.connection, + }); /** * See: https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/metrics/v1/metrics.proto @@ -2281,7 +2320,7 @@ async function translateMetricChartConfig( sql: chSql` SELECT *, - cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash, + ${{ UNSAFE_RAW_SQL: seriesKey }} AS AttributesHash, IF( AggregationTemporality = 1, Value, -- DELTA: Value is already the per-interval increase @@ -2545,6 +2584,12 @@ async function translateMetricChartConfig( }), where, valueAlias, + attrHashExpr: await renderMetricSeriesKey(metadata, { + databaseName: from.databaseName, + // eslint-disable-next-line security/detect-object-injection + tableName: metricTables[metricType], + connectionId: chartConfig.connection, + }), }; // The translated scope only projects [bucket?, group, value] — the