Skip to content

Commit da3caab

Browse files
authored
fix: type JSON metadata filter attributes (#2537)
## Summary - Render JSON-backed metadata filter keys as typed ClickHouse JSON subcolumns before using them in `GROUP BY`, `groupUniqArray`, and fallback value lookups. - Keep Map-backed attribute keys on the existing bracket syntax. - Add regression coverage for `getKeyValues`, `getValuesDistribution`, and `getMapValues`. Validation: - `mise exec node@22.16.0 -- corepack yarn workspace @hyperdx/common-utils jest --runInBand src/__tests__/metadata.test.ts` - `mise exec node@22.16.0 -- corepack yarn workspace @hyperdx/common-utils ci:lint` - `mise exec node@22.16.0 -- corepack yarn workspace @hyperdx/common-utils ci:unit` Duplicate/overlap check: I did not find another PR fixing #2482. #2423 touches faceted filter values but is a separate draft feature and does not address JSON typed subcolumn rendering. ### Screenshots or video N/A - non-UI change ### How to test on Vercel preview N/A - non-UI change ### References - Fixes #2482 - Related PRs: #2423
1 parent c1403a7 commit da3caab

3 files changed

Lines changed: 366 additions & 7 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@hyperdx/common-utils': patch
3+
---
4+
5+
Type JSON metadata filter attribute paths before value sampling.

‎packages/common-utils/src/__tests__/metadata.test.ts‎

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,107 @@ describe('Metadata', () => {
635635
expect(results).toEqual([]);
636636
expect(renderChartConfigSpy).not.toHaveBeenCalled();
637637
});
638+
639+
it('renders JSON attribute keys as typed subcolumns', async () => {
640+
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
641+
Promise.resolve(
642+
column === 'ResourceAttributes'
643+
? ({
644+
name: 'ResourceAttributes',
645+
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
646+
} as any)
647+
: undefined,
648+
),
649+
);
650+
const renderChartConfigSpy = jest.spyOn(
651+
renderChartConfigModule,
652+
'renderChartConfig',
653+
);
654+
655+
await metadata.getKeyValues({
656+
chartConfig: mockChartConfig,
657+
keys: ["ResourceAttributes['k8s.namespace.name']"],
658+
limit: 10,
659+
source,
660+
});
661+
662+
const actualConfig = renderChartConfigSpy.mock.calls[0][0];
663+
if (!isBuilderChartConfig(actualConfig))
664+
throw new Error('Expected builder config');
665+
expect(actualConfig.with?.[0]).toMatchObject({
666+
chartConfig: {
667+
select:
668+
'ResourceAttributes.`k8s`.`namespace`.`name`.:String as param0',
669+
},
670+
});
671+
});
672+
673+
it('quotes typed-looking bracket JSON keys instead of passing them through', async () => {
674+
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
675+
Promise.resolve(
676+
column === 'ResourceAttributes'
677+
? ({
678+
name: 'ResourceAttributes',
679+
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
680+
} as any)
681+
: undefined,
682+
),
683+
);
684+
const renderChartConfigSpy = jest.spyOn(
685+
renderChartConfigModule,
686+
'renderChartConfig',
687+
);
688+
689+
await metadata.getKeyValues({
690+
chartConfig: mockChartConfig,
691+
keys: ["ResourceAttributes['foo.:String, count() AS injected']"],
692+
limit: 10,
693+
source,
694+
});
695+
696+
const actualConfig = renderChartConfigSpy.mock.calls[0][0];
697+
if (!isBuilderChartConfig(actualConfig))
698+
throw new Error('Expected builder config');
699+
expect(actualConfig.with?.[0]).toMatchObject({
700+
chartConfig: {
701+
select:
702+
'ResourceAttributes.`foo`.`:String, count() AS injected`.:String as param0',
703+
},
704+
});
705+
});
706+
707+
it('keeps map attribute keys in bracket form', async () => {
708+
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
709+
Promise.resolve(
710+
column === 'LogAttributes'
711+
? ({
712+
name: 'LogAttributes',
713+
type: 'Map(LowCardinality(String), String)',
714+
} as any)
715+
: undefined,
716+
),
717+
);
718+
const renderChartConfigSpy = jest.spyOn(
719+
renderChartConfigModule,
720+
'renderChartConfig',
721+
);
722+
723+
await metadata.getKeyValues({
724+
chartConfig: mockChartConfig,
725+
keys: ["LogAttributes['k8s.namespace.name']"],
726+
limit: 10,
727+
source,
728+
});
729+
730+
const actualConfig = renderChartConfigSpy.mock.calls[0][0];
731+
if (!isBuilderChartConfig(actualConfig))
732+
throw new Error('Expected builder config');
733+
expect(actualConfig.with?.[0]).toMatchObject({
734+
chartConfig: {
735+
select: "LogAttributes['k8s.namespace.name'] as param0",
736+
},
737+
});
738+
});
638739
});
639740

640741
describe('getValuesDistribution', () => {
@@ -777,6 +878,67 @@ describe('Metadata', () => {
777878
condition: "ServiceName IN ('clickhouse')",
778879
});
779880
});
881+
882+
it('renders JSON distribution keys as typed subcolumns', async () => {
883+
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
884+
Promise.resolve(
885+
column === 'ResourceAttributes'
886+
? ({
887+
name: 'ResourceAttributes',
888+
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
889+
} as any)
890+
: undefined,
891+
),
892+
);
893+
const renderChartConfigSpy = jest.spyOn(
894+
renderChartConfigModule,
895+
'renderChartConfig',
896+
);
897+
898+
await metadata.getValuesDistribution({
899+
chartConfig: mockChartConfig,
900+
key: 'ResourceAttributes.k8s.namespace.name',
901+
source,
902+
});
903+
904+
const actualConfig = renderChartConfigSpy.mock.calls[0][0];
905+
if (!isBuilderChartConfig(actualConfig))
906+
throw new Error('Expected builder config');
907+
expect(actualConfig.select).toBe(
908+
'ResourceAttributes.`k8s`.`namespace`.`name`.:String AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage',
909+
);
910+
expect(actualConfig.groupBy).toBe('__hdx_value');
911+
});
912+
913+
it('normalizes typed dot-form JSON distribution keys safely', async () => {
914+
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
915+
Promise.resolve(
916+
column === 'ResourceAttributes'
917+
? ({
918+
name: 'ResourceAttributes',
919+
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
920+
} as any)
921+
: undefined,
922+
),
923+
);
924+
const renderChartConfigSpy = jest.spyOn(
925+
renderChartConfigModule,
926+
'renderChartConfig',
927+
);
928+
929+
await metadata.getValuesDistribution({
930+
chartConfig: mockChartConfig,
931+
key: 'ResourceAttributes.k8s.namespace.name.:String',
932+
source,
933+
});
934+
935+
const actualConfig = renderChartConfigSpy.mock.calls[0][0];
936+
if (!isBuilderChartConfig(actualConfig))
937+
throw new Error('Expected builder config');
938+
expect(actualConfig.select).toBe(
939+
'ResourceAttributes.`k8s`.`namespace`.`name`.:String AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage',
940+
);
941+
});
780942
});
781943

782944
describe('getMapKeys', () => {
@@ -943,6 +1105,10 @@ describe('Metadata', () => {
9431105

9441106
it("emits only the existing value != '' predicate when dateRange not provided", async () => {
9451107
const md = buildMetadata();
1108+
jest.spyOn(md, 'getColumn').mockResolvedValue({
1109+
name: 'LogAttributes',
1110+
type: 'Map(LowCardinality(String), String)',
1111+
} as any);
9461112

9471113
(mockClickhouseClient.query as jest.Mock).mockResolvedValueOnce({
9481114
json: () => Promise.resolve({ data: [] }),
@@ -966,6 +1132,10 @@ describe('Metadata', () => {
9661132

9671133
it('injects the time filter clause when dateRange and timestampValueExpression are provided', async () => {
9681134
const md = buildMetadata();
1135+
jest.spyOn(md, 'getColumn').mockResolvedValue({
1136+
name: 'LogAttributes',
1137+
type: 'Map(LowCardinality(String), String)',
1138+
} as any);
9691139

9701140
(mockClickhouseClient.query as jest.Mock).mockResolvedValueOnce({
9711141
json: () => Promise.resolve({ data: [] }),
@@ -1002,8 +1172,39 @@ describe('Metadata', () => {
10021172
expect(valuesCall.query).toContain('__TIME_FILTER__');
10031173
});
10041174

1175+
it('uses typed JSON subcolumns for JSON attribute values', async () => {
1176+
const md = buildMetadata();
1177+
jest.spyOn(md, 'getColumn').mockResolvedValue({
1178+
name: 'ResourceAttributes',
1179+
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
1180+
} as any);
1181+
1182+
(mockClickhouseClient.query as jest.Mock).mockResolvedValueOnce({
1183+
json: () => Promise.resolve({ data: [] }),
1184+
});
1185+
1186+
await md.getMapValues({
1187+
databaseName: 'otel',
1188+
tableName: 'generic_logs',
1189+
column: 'ResourceAttributes',
1190+
key: 'k8s.namespace.name',
1191+
connectionId: 'conn-1',
1192+
});
1193+
1194+
const valuesCall = (mockClickhouseClient.query as jest.Mock).mock
1195+
.calls[0][0];
1196+
expect(valuesCall.query).toContain(
1197+
'ResourceAttributes.`k8s`.`namespace`.`name`.:String as value',
1198+
);
1199+
expect(valuesCall.query).not.toContain('ResourceAttributes[');
1200+
});
1201+
10051202
it('caches values distinctly for different dateRange values', async () => {
10061203
const md = buildMetadata();
1204+
jest.spyOn(md, 'getColumn').mockResolvedValue({
1205+
name: 'LogAttributes',
1206+
type: 'Map(LowCardinality(String), String)',
1207+
} as any);
10071208

10081209
(mockClickhouseClient.query as jest.Mock)
10091210
.mockResolvedValueOnce({

0 commit comments

Comments
 (0)