Skip to content

Commit 575ca47

Browse files
committed
Fix telemetry unit tests for extensible feature flags and proto payload format
- Update FeatureFlagCache tests to use new extensible flags Map - Fix DatabricksTelemetryExporter tests to use protoLogs format - Verify telemetry endpoints use correct paths (/telemetry-ext, /telemetry-unauth) - 213 passing, 13 logging assertion tests need investigation
1 parent 74cd522 commit 575ca47

2 files changed

Lines changed: 97 additions & 38 deletions

File tree

tests/unit/telemetry/DatabricksTelemetryExporter.test.ts

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ describe('DatabricksTelemetryExporter', () => {
8787

8888
expect(fetchStub.calledOnce).to.be.true;
8989
const call = fetchStub.getCall(0);
90-
expect(call.args[0]).to.equal('https://test.databricks.com/api/2.0/sql/telemetry-ext');
90+
expect(call.args[0]).to.equal('https://test.databricks.com/telemetry-ext');
9191
});
9292

9393
it('should export to unauthenticated endpoint when disabled', async () => {
@@ -123,7 +123,7 @@ describe('DatabricksTelemetryExporter', () => {
123123

124124
expect(fetchStub.calledOnce).to.be.true;
125125
const call = fetchStub.getCall(0);
126-
expect(call.args[0]).to.equal('https://test.databricks.com/api/2.0/sql/telemetry-unauth');
126+
expect(call.args[0]).to.equal('https://test.databricks.com/telemetry-unauth');
127127
});
128128
});
129129

@@ -170,10 +170,26 @@ describe('DatabricksTelemetryExporter', () => {
170170
const call = fetchStub.getCall(0);
171171
const body = JSON.parse(call.args[1].body);
172172

173-
expect(body.frontend_logs).to.have.lengthOf(1);
174-
expect(body.frontend_logs[0].workspace_id).to.equal('ws-1');
175-
expect(body.frontend_logs[0].entry.sql_driver_log.session_id).to.equal('session-1');
176-
expect(body.frontend_logs[0].entry.sql_driver_log.driver_config).to.deep.equal(metrics[0].driverConfig);
173+
expect(body.protoLogs).to.have.lengthOf(1);
174+
const log = JSON.parse(body.protoLogs[0]);
175+
expect(log.entry.sql_driver_log.session_id).to.equal('session-1');
176+
expect(log.entry.sql_driver_log.system_configuration).to.deep.equal(
177+
metrics[0].driverConfig
178+
? {
179+
driver_version: metrics[0].driverConfig.driverVersion,
180+
driver_name: metrics[0].driverConfig.driverName,
181+
runtime_name: 'Node.js',
182+
runtime_version: metrics[0].driverConfig.nodeVersion,
183+
runtime_vendor: metrics[0].driverConfig.runtimeVendor,
184+
os_name: metrics[0].driverConfig.platform,
185+
os_version: metrics[0].driverConfig.osVersion,
186+
os_arch: metrics[0].driverConfig.osArch,
187+
locale_name: metrics[0].driverConfig.localeName,
188+
char_set_encoding: metrics[0].driverConfig.charSetEncoding,
189+
process_name: metrics[0].driverConfig.processName,
190+
}
191+
: undefined,
192+
);
177193
});
178194

179195
it('should format statement metric correctly', async () => {
@@ -203,15 +219,14 @@ describe('DatabricksTelemetryExporter', () => {
203219
const call = fetchStub.getCall(0);
204220
const body = JSON.parse(call.args[1].body);
205221

206-
expect(body.frontend_logs).to.have.lengthOf(1);
207-
const log = body.frontend_logs[0];
208-
expect(log.workspace_id).to.equal('ws-1');
222+
expect(body.protoLogs).to.have.lengthOf(1);
223+
const log = JSON.parse(body.protoLogs[0]);
209224
expect(log.entry.sql_driver_log.session_id).to.equal('session-1');
210225
expect(log.entry.sql_driver_log.sql_statement_id).to.equal('stmt-1');
211226
expect(log.entry.sql_driver_log.operation_latency_ms).to.equal(1500);
212-
expect(log.entry.sql_driver_log.sql_operation.execution_result_format).to.equal('cloudfetch');
213-
expect(log.entry.sql_driver_log.sql_operation.chunk_details.chunk_count).to.equal(5);
214-
expect(log.entry.sql_driver_log.sql_operation.chunk_details.total_bytes).to.equal(1024000);
227+
expect(log.entry.sql_driver_log.sql_operation.execution_result).to.equal('cloudfetch');
228+
expect(log.entry.sql_driver_log.sql_operation.chunk_details.total_chunks_present).to.equal(5);
229+
expect(log.entry.sql_driver_log.sql_operation.chunk_details.total_chunks_iterated).to.equal(5);
215230
});
216231

217232
it('should format error metric correctly', async () => {
@@ -239,8 +254,8 @@ describe('DatabricksTelemetryExporter', () => {
239254
const call = fetchStub.getCall(0);
240255
const body = JSON.parse(call.args[1].body);
241256

242-
expect(body.frontend_logs).to.have.lengthOf(1);
243-
const log = body.frontend_logs[0];
257+
expect(body.protoLogs).to.have.lengthOf(1);
258+
const log = JSON.parse(body.protoLogs[0]);
244259
expect(log.entry.sql_driver_log.error_info.error_name).to.equal('AuthenticationError');
245260
expect(log.entry.sql_driver_log.error_info.stack_trace).to.equal('Invalid credentials');
246261
});
@@ -267,9 +282,8 @@ describe('DatabricksTelemetryExporter', () => {
267282

268283
const call = fetchStub.getCall(0);
269284
const body = JSON.parse(call.args[1].body);
270-
const log = body.frontend_logs[0];
285+
const log = JSON.parse(body.protoLogs[0]);
271286

272-
expect(log.workspace_id).to.equal('ws-789');
273287
expect(log.entry.sql_driver_log.session_id).to.equal('session-123');
274288
expect(log.entry.sql_driver_log.sql_statement_id).to.equal('stmt-456');
275289
});

tests/unit/telemetry/FeatureFlagCache.test.ts

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('FeatureFlagCache', () => {
4242
expect(ctx).to.not.be.undefined;
4343
expect(ctx.refCount).to.equal(1);
4444
expect(ctx.cacheDuration).to.equal(15 * 60 * 1000); // 15 minutes
45-
expect(ctx.telemetryEnabled).to.be.undefined;
45+
expect(ctx.flags.size).to.equal(0); // Empty flags map initially
4646
expect(ctx.lastFetched).to.be.undefined;
4747
});
4848

@@ -137,8 +137,16 @@ describe('FeatureFlagCache', () => {
137137
const cache = new FeatureFlagCache(context);
138138
const host = 'test-host.databricks.com';
139139

140-
// Stub the private fetchFeatureFlag method
141-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag').resolves(true);
140+
// Stub the private fetchFeatureFlags method to populate flags Map
141+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').callsFake(async () => {
142+
const ctx = (cache as any).contexts.get(host);
143+
if (ctx) {
144+
ctx.flags.set(
145+
'databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForNodeJs',
146+
'true',
147+
);
148+
}
149+
});
142150

143151
cache.getOrCreateContext(host);
144152
const enabled = await cache.isTelemetryEnabled(host);
@@ -155,7 +163,15 @@ describe('FeatureFlagCache', () => {
155163
const cache = new FeatureFlagCache(context);
156164
const host = 'test-host.databricks.com';
157165

158-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag').resolves(true);
166+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').callsFake(async () => {
167+
const ctx = (cache as any).contexts.get(host);
168+
if (ctx) {
169+
ctx.flags.set(
170+
'databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForNodeJs',
171+
'true',
172+
);
173+
}
174+
});
159175

160176
cache.getOrCreateContext(host);
161177

@@ -179,9 +195,18 @@ describe('FeatureFlagCache', () => {
179195
const cache = new FeatureFlagCache(context);
180196
const host = 'test-host.databricks.com';
181197

182-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag');
183-
fetchStub.onFirstCall().resolves(true);
184-
fetchStub.onSecondCall().resolves(false);
198+
let callCount = 0;
199+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').callsFake(async () => {
200+
const ctx = (cache as any).contexts.get(host);
201+
if (ctx) {
202+
callCount++;
203+
// First call returns true, second returns false
204+
ctx.flags.set(
205+
'databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForNodeJs',
206+
callCount === 1 ? 'true' : 'false',
207+
);
208+
}
209+
});
185210

186211
cache.getOrCreateContext(host);
187212

@@ -207,24 +232,24 @@ describe('FeatureFlagCache', () => {
207232
const cache = new FeatureFlagCache(context);
208233
const host = 'test-host.databricks.com';
209234

210-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag').rejects(new Error('Network error'));
235+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').rejects(new Error('Network error'));
211236

212237
cache.getOrCreateContext(host);
213238
const enabled = await cache.isTelemetryEnabled(host);
214239

215240
expect(enabled).to.be.false;
216-
expect(logSpy.calledWith(LogLevel.debug, 'Error fetching feature flag: Network error')).to.be.true;
241+
expect(logSpy.calledWith(LogLevel.debug, 'Error fetching feature flags: Network error')).to.be.true;
217242

218243
fetchStub.restore();
219244
logSpy.restore();
220245
});
221246

222-
it('should not propagate exceptions from fetchFeatureFlag', async () => {
247+
it('should not propagate exceptions from fetchFeatureFlags', async () => {
223248
const context = new ClientContextStub();
224249
const cache = new FeatureFlagCache(context);
225250
const host = 'test-host.databricks.com';
226251

227-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag').rejects(new Error('Network error'));
252+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').rejects(new Error('Network error'));
228253

229254
cache.getOrCreateContext(host);
230255

@@ -235,12 +260,13 @@ describe('FeatureFlagCache', () => {
235260
fetchStub.restore();
236261
});
237262

238-
it('should return false when telemetryEnabled is undefined', async () => {
263+
it('should return false when flag is not set in the map', async () => {
239264
const context = new ClientContextStub();
240265
const cache = new FeatureFlagCache(context);
241266
const host = 'test-host.databricks.com';
242267

243-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag').resolves(undefined);
268+
// Stub fetchFeatureFlags to do nothing (leaves flags map empty)
269+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').resolves();
244270

245271
cache.getOrCreateContext(host);
246272
const enabled = await cache.isTelemetryEnabled(host);
@@ -251,15 +277,19 @@ describe('FeatureFlagCache', () => {
251277
});
252278
});
253279

254-
describe('fetchFeatureFlag', () => {
255-
it('should return false as placeholder implementation', async () => {
280+
describe('fetchFeatureFlags', () => {
281+
it('should handle fetch errors gracefully', async () => {
256282
const context = new ClientContextStub();
257283
const cache = new FeatureFlagCache(context);
258284
const host = 'test-host.databricks.com';
259285

260-
// Access private method through any cast
261-
const result = await (cache as any).fetchFeatureFlag(host);
262-
expect(result).to.be.false;
286+
cache.getOrCreateContext(host);
287+
288+
// Access private method through any cast - should not throw even if fetch fails
289+
await (cache as any).fetchFeatureFlags(host);
290+
291+
// Should log at debug level but not throw
292+
// Exact behavior depends on network conditions, this just verifies no exception
263293
});
264294
});
265295

@@ -269,7 +299,15 @@ describe('FeatureFlagCache', () => {
269299
const cache = new FeatureFlagCache(context);
270300
const host = 'test-host.databricks.com';
271301

272-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag').resolves(true);
302+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').callsFake(async () => {
303+
const ctx = (cache as any).contexts.get(host);
304+
if (ctx) {
305+
ctx.flags.set(
306+
'databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForNodeJs',
307+
'true',
308+
);
309+
}
310+
});
273311

274312
// Simulate 3 connections to same host
275313
cache.getOrCreateContext(host);
@@ -301,9 +339,16 @@ describe('FeatureFlagCache', () => {
301339
const host1 = 'host1.databricks.com';
302340
const host2 = 'host2.databricks.com';
303341

304-
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlag');
305-
fetchStub.withArgs(host1).resolves(true);
306-
fetchStub.withArgs(host2).resolves(false);
342+
const fetchStub = sinon.stub(cache as any, 'fetchFeatureFlags').callsFake(async (host: string) => {
343+
const ctx = (cache as any).contexts.get(host);
344+
if (ctx) {
345+
// host1 returns true, host2 returns false
346+
ctx.flags.set(
347+
'databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForNodeJs',
348+
host === host1 ? 'true' : 'false',
349+
);
350+
}
351+
});
307352

308353
cache.getOrCreateContext(host1);
309354
cache.getOrCreateContext(host2);

0 commit comments

Comments
 (0)