From 8d3b5a9a8c905031d773d9661cadea66be5bf653 Mon Sep 17 00:00:00 2001 From: Chris Barber Date: Fri, 22 May 2026 19:13:46 -0500 Subject: [PATCH 1/2] fix(analytics): wrap time bounds as compound-key prefixes for get_analytics filter The hdb_analytics table stores its primary key as the compound array [timestamp, nodeId], but get_analytics was passing scalar timestamps for start_time/end_time/customWindow filters. Harper's key encoding sorts scalar numbers and arrays into different ranges, so the range iterator landed past all rows and returned zero results. The response had been hiding this by post-processing result.id down to result.id[0] before returning to the client. Wrap each bound as a single-element array so the comparison stays inside the array key space ([X] sorts before [X, anything]). Also collapsed the start && end branching to two independent ifs and switched the truthiness check to != null so a 0 timestamp is treated as valid. Co-Authored-By: Claude Opus 4.7 (1M context) --- resources/analytics/read.ts | 35 ++++---- unitTests/resources/analytics/read.test.js | 92 +++++++++++++++++++++- 2 files changed, 104 insertions(+), 23 deletions(-) diff --git a/resources/analytics/read.ts b/resources/analytics/read.ts index 3a2032e16..be9881825 100644 --- a/resources/analytics/read.ts +++ b/resources/analytics/read.ts @@ -97,27 +97,22 @@ export async function get(metric: string, opts?: GetAnalyticsOpts): Promise { diff --git a/unitTests/resources/analytics/read.test.js b/unitTests/resources/analytics/read.test.js index 2724ab017..0451cd88b 100644 --- a/unitTests/resources/analytics/read.test.js +++ b/unitTests/resources/analytics/read.test.js @@ -4,7 +4,87 @@ const { expect } = require('chai'); const { describe, it } = require('mocha'); const sinon = require('sinon'); const { METRIC } = require('#src/resources/analytics/metadata'); -const { listMetrics, describeMetric /* collectDistinctValues */ } = require('#src/resources/analytics/read'); +const { get, listMetrics, describeMetric /* collectDistinctValues */ } = require('#src/resources/analytics/read'); + +describe('get', () => { + let searchStub; + let mockAsyncIterable; + + beforeEach(() => { + mockAsyncIterable = { + [Symbol.asyncIterator]: async function* () {}, + map: function () { + return this; + }, + }; + + global.databases = { + system: { + hdb_analytics: { + search: sinon.stub().resolves(mockAsyncIterable), + }, + }, + }; + global.server = { hostname: 'test-host' }; + + searchStub = global.databases.system.hdb_analytics.search; + }); + + afterEach(() => { + sinon.restore(); + delete global.databases; + delete global.server; + }); + + function findIdCondition(conditions, comparator) { + return conditions.find((c) => c.attribute === 'id' && c.comparator === comparator); + } + + it('should not add any id condition when neither startTime nor endTime is provided', async () => { + await get('cpu-usage'); + const { conditions } = searchStub.firstCall.args[0]; + expect(conditions.some((c) => c.attribute === 'id')).to.be.false; + }); + + it('should wrap startTime as a compound-key prefix for greater_than_equal', async () => { + const startTime = 1779488703216; + await get('cpu-usage', { startTime }); + const { conditions } = searchStub.firstCall.args[0]; + const cond = findIdCondition(conditions, 'greater_than_equal'); + expect(cond, 'expected a greater_than_equal condition on id').to.exist; + // id is stored as [timestamp, nodeId]; a scalar bound would land outside + // the array key range and return zero rows, so the bound must be wrapped. + expect(cond.value).to.deep.equal([startTime]); + }); + + it('should wrap endTime as a compound-key prefix for less_than', async () => { + const endTime = 1779488793222; + await get('cpu-usage', { endTime }); + const { conditions } = searchStub.firstCall.args[0]; + const cond = findIdCondition(conditions, 'less_than'); + expect(cond, 'expected a less_than condition on id').to.exist; + expect(cond.value).to.deep.equal([endTime]); + }); + + it('should emit both wrapped bounds when startTime and endTime are provided', async () => { + const startTime = 1779488703216; + const endTime = 1779488793222; + await get('cpu-usage', { startTime, endTime }); + const { conditions } = searchStub.firstCall.args[0]; + const ge = findIdCondition(conditions, 'greater_than_equal'); + const lt = findIdCondition(conditions, 'less_than'); + expect(ge.value).to.deep.equal([startTime]); + expect(lt.value).to.deep.equal([endTime]); + }); + + it('should treat startTime of 0 as a valid bound (not falsy)', async () => { + await get('cpu-usage', { startTime: 0 }); + const { conditions } = searchStub.firstCall.args[0]; + const cond = findIdCondition(conditions, 'greater_than_equal'); + expect(cond).to.exist; + expect(cond.value).to.deep.equal([0]); + }); +}); describe('listMetrics', () => { let searchStub; @@ -137,7 +217,9 @@ describe('listMetrics', () => { const firstCondition = searchStub.firstCall.args[0].conditions[0]; expect(firstCondition.attribute).to.be.equal('id'); expect(firstCondition.comparator).to.be.equal('greater_than'); - expect(firstCondition.value).to.be.approximately(weekAgo, 1000); + // id is a compound key, so the bound is wrapped in an array + expect(firstCondition.value).to.be.an('array').with.lengthOf(1); + expect(firstCondition.value[0]).to.be.approximately(weekAgo, 1000); }); it('should use the given metric time window when provided', async () => { @@ -146,7 +228,8 @@ describe('listMetrics', () => { const firstCondition = searchStub.firstCall.args[0].conditions[0]; expect(firstCondition.attribute).to.be.equal('id'); expect(firstCondition.comparator).to.be.equal('greater_than'); - expect(firstCondition.value).to.be.approximately(Date.now() - twoDays, 1000); + expect(firstCondition.value).to.be.an('array').with.lengthOf(1); + expect(firstCondition.value[0]).to.be.approximately(Date.now() - twoDays, 1000); }); it('should return empty array when no metric types are requested', async () => { @@ -191,7 +274,8 @@ describe('listMetrics', () => { // Should set a time window cutoff as the first condition expect(searchParams.conditions[0].attribute).to.equal('id'); expect(searchParams.conditions[0].comparator).to.equal('greater_than'); - expect(searchParams.conditions[0].value).to.be.lessThan(Date.now()); + expect(searchParams.conditions[0].value).to.be.an('array').with.lengthOf(1); + expect(searchParams.conditions[0].value[0]).to.be.lessThan(Date.now()); // Each condition should be checking "not equal" to a built-in metric for (let i = 0; i < builtins.length; i++) { From 60161ca2fe89b81b6582b550cb3d2715c089af2a Mon Sep 17 00:00:00 2001 From: Chris Barber Date: Sat, 23 May 2026 01:21:03 -0500 Subject: [PATCH 2/2] Remove server mock --- unitTests/resources/analytics/read.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/unitTests/resources/analytics/read.test.js b/unitTests/resources/analytics/read.test.js index 0451cd88b..89f565af3 100644 --- a/unitTests/resources/analytics/read.test.js +++ b/unitTests/resources/analytics/read.test.js @@ -25,7 +25,6 @@ describe('get', () => { }, }, }; - global.server = { hostname: 'test-host' }; searchStub = global.databases.system.hdb_analytics.search; }); @@ -33,7 +32,6 @@ describe('get', () => { afterEach(() => { sinon.restore(); delete global.databases; - delete global.server; }); function findIdCondition(conditions, comparator) {