Skip to content

fix: type JSON metadata filter attributes #2537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kodiakhq merged 3 commits into from
Jun 29, 2026
Merged

fix: type JSON metadata filter attributes #2537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90aa547
fix: type JSON metadata filter attributes
mfroembgen Jun 29, 2026
852c5f7
chore: add changeset for JSON metadata fix
mfroembgen Jun 29, 2026
e1d5250
fix: quote typed-looking JSON metadata keys
mfroembgen Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/type-json-metadata-filter-attributes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperdx/common-utils': patch
---

Type JSON metadata filter attribute paths before value sampling.
201 changes: 201 additions & 0 deletions packages/common-utils/src/__tests__/metadata.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,107 @@ describe('Metadata', () => {
expect(results).toEqual([]);
expect(renderChartConfigSpy).not.toHaveBeenCalled();
});

it('renders JSON attribute keys as typed subcolumns', async () => {
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
Promise.resolve(
column === 'ResourceAttributes'
? ({
name: 'ResourceAttributes',
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
} as any)
: undefined,
),
);
const renderChartConfigSpy = jest.spyOn(
renderChartConfigModule,
'renderChartConfig',
);

await metadata.getKeyValues({
chartConfig: mockChartConfig,
keys: ["ResourceAttributes['k8s.namespace.name']"],
limit: 10,
source,
});

const actualConfig = renderChartConfigSpy.mock.calls[0][0];
if (!isBuilderChartConfig(actualConfig))
throw new Error('Expected builder config');
expect(actualConfig.with?.[0]).toMatchObject({
chartConfig: {
select:
'ResourceAttributes.`k8s`.`namespace`.`name`.:String as param0',
},
});
});

it('quotes typed-looking bracket JSON keys instead of passing them through', async () => {
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
Promise.resolve(
column === 'ResourceAttributes'
? ({
name: 'ResourceAttributes',
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
} as any)
: undefined,
),
);
const renderChartConfigSpy = jest.spyOn(
renderChartConfigModule,
'renderChartConfig',
);

await metadata.getKeyValues({
chartConfig: mockChartConfig,
keys: ["ResourceAttributes['foo.:String, count() AS injected']"],
limit: 10,
source,
});

const actualConfig = renderChartConfigSpy.mock.calls[0][0];
if (!isBuilderChartConfig(actualConfig))
throw new Error('Expected builder config');
expect(actualConfig.with?.[0]).toMatchObject({
chartConfig: {
select:
'ResourceAttributes.`foo`.`:String, count() AS injected`.:String as param0',
},
});
});

it('keeps map attribute keys in bracket form', async () => {
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
Promise.resolve(
column === 'LogAttributes'
? ({
name: 'LogAttributes',
type: 'Map(LowCardinality(String), String)',
} as any)
: undefined,
),
);
const renderChartConfigSpy = jest.spyOn(
renderChartConfigModule,
'renderChartConfig',
);

await metadata.getKeyValues({
chartConfig: mockChartConfig,
keys: ["LogAttributes['k8s.namespace.name']"],
limit: 10,
source,
});

const actualConfig = renderChartConfigSpy.mock.calls[0][0];
if (!isBuilderChartConfig(actualConfig))
throw new Error('Expected builder config');
expect(actualConfig.with?.[0]).toMatchObject({
chartConfig: {
select: "LogAttributes['k8s.namespace.name'] as param0",
},
});
});
});

describe('getValuesDistribution', () => {
Expand Down Expand Up @@ -777,6 +878,67 @@ describe('Metadata', () => {
condition: "ServiceName IN ('clickhouse')",
});
});

it('renders JSON distribution keys as typed subcolumns', async () => {
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
Promise.resolve(
column === 'ResourceAttributes'
? ({
name: 'ResourceAttributes',
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
} as any)
: undefined,
),
);
const renderChartConfigSpy = jest.spyOn(
renderChartConfigModule,
'renderChartConfig',
);

await metadata.getValuesDistribution({
chartConfig: mockChartConfig,
key: 'ResourceAttributes.k8s.namespace.name',
source,
});

const actualConfig = renderChartConfigSpy.mock.calls[0][0];
if (!isBuilderChartConfig(actualConfig))
throw new Error('Expected builder config');
expect(actualConfig.select).toBe(
'ResourceAttributes.`k8s`.`namespace`.`name`.:String AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage',
);
expect(actualConfig.groupBy).toBe('__hdx_value');
});

it('normalizes typed dot-form JSON distribution keys safely', async () => {
jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) =>
Promise.resolve(
column === 'ResourceAttributes'
? ({
name: 'ResourceAttributes',
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
} as any)
: undefined,
),
);
const renderChartConfigSpy = jest.spyOn(
renderChartConfigModule,
'renderChartConfig',
);

await metadata.getValuesDistribution({
chartConfig: mockChartConfig,
key: 'ResourceAttributes.k8s.namespace.name.:String',
source,
});

const actualConfig = renderChartConfigSpy.mock.calls[0][0];
if (!isBuilderChartConfig(actualConfig))
throw new Error('Expected builder config');
expect(actualConfig.select).toBe(
'ResourceAttributes.`k8s`.`namespace`.`name`.:String AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage',
);
});
});

describe('getMapKeys', () => {
Expand Down Expand Up @@ -943,6 +1105,10 @@ describe('Metadata', () => {

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

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

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

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

it('uses typed JSON subcolumns for JSON attribute values', async () => {
const md = buildMetadata();
jest.spyOn(md, 'getColumn').mockResolvedValue({
name: 'ResourceAttributes',
type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)',
} as any);

(mockClickhouseClient.query as jest.Mock).mockResolvedValueOnce({
json: () => Promise.resolve({ data: [] }),
});

await md.getMapValues({
databaseName: 'otel',
tableName: 'generic_logs',
column: 'ResourceAttributes',
key: 'k8s.namespace.name',
connectionId: 'conn-1',
});

const valuesCall = (mockClickhouseClient.query as jest.Mock).mock
.calls[0][0];
expect(valuesCall.query).toContain(
'ResourceAttributes.`k8s`.`namespace`.`name`.:String as value',
);
expect(valuesCall.query).not.toContain('ResourceAttributes[');
});

it('caches values distinctly for different dateRange values', async () => {
const md = buildMetadata();
jest.spyOn(md, 'getColumn').mockResolvedValue({
name: 'LogAttributes',
type: 'Map(LowCardinality(String), String)',
} as any);

(mockClickhouseClient.query as jest.Mock)
.mockResolvedValueOnce({
Expand Down
Loading
Loading