Skip to content

Commit b62578d

Browse files
committed
Fix linting issues in telemetry implementation
- Fix no-plusplus errors: replace ++ and -- with += 1 and -= 1 - Fix global-require errors: move require() calls to imports - Fix no-unused-vars errors: remove or mark unused variables - Fix no-await-in-loop: add eslint-disable for intentional retry logic - Fix no-lonely-if: refactor nested if statements - Fix default-case: add default case to switch statements - Fix no-promise-executor-return: wrap setTimeout in arrow function All linting errors resolved. Only 14 warnings remain for unnamed test functions.
1 parent ffdaa64 commit b62578d

10 files changed

Lines changed: 913 additions & 32 deletions

coverage/coverage-summary.json

Lines changed: 77 additions & 0 deletions
Large diffs are not rendered by default.

generated_task_specs.json

Lines changed: 778 additions & 0 deletions
Large diffs are not rendered by default.

lib/DBSQLClient.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import thrift from 'thrift';
22
import Int64 from 'node-int64';
3+
import os from 'os';
34

45
import { EventEmitter } from 'events';
56
import TCLIService from '../thrift/TCLIService';
@@ -77,10 +78,15 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
7778

7879
// Telemetry components (instance-based, NOT singletons)
7980
private host?: string;
81+
8082
private featureFlagCache?: FeatureFlagCache;
83+
8184
private telemetryClientProvider?: TelemetryClientProvider;
85+
8286
private telemetryEmitter?: TelemetryEventEmitter;
87+
8388
private telemetryAggregator?: MetricsAggregator;
89+
8490
private circuitBreakerRegistry?: CircuitBreakerRegistry;
8591

8692
private static getDefaultLogger(): IDBSQLLogger {
@@ -197,7 +203,7 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
197203
driverName: '@databricks/sql',
198204
nodeVersion: process.version,
199205
platform: process.platform,
200-
osVersion: require('os').release(),
206+
osVersion: os.release(),
201207

202208
// Feature flags
203209
cloudFetchEnabled: this.config.useCloudFetch ?? false,
@@ -239,7 +245,7 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
239245
this.telemetryEmitter = new TelemetryEventEmitter(this);
240246

241247
// Get or create telemetry client for this host (increments refCount)
242-
const telemetryClient = this.telemetryClientProvider.getOrCreateClient(this.host);
248+
this.telemetryClientProvider.getOrCreateClient(this.host);
243249

244250
// Create circuit breaker registry and exporter
245251
this.circuitBreakerRegistry = new CircuitBreakerRegistry(this);

lib/DBSQLOperation.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { definedOrError } from './utils';
3434
import { OperationChunksIterator, OperationRowsIterator } from './utils/OperationIterator';
3535
import HiveDriverError from './errors/HiveDriverError';
3636
import IClientContext from './contracts/IClientContext';
37+
import ExceptionClassifier from './telemetry/ExceptionClassifier';
3738

3839
interface DBSQLOperationConstructorOptions {
3940
handle: TOperationHandle;
@@ -79,7 +80,9 @@ export default class DBSQLOperation implements IOperation {
7980

8081
// Telemetry tracking fields
8182
private startTime: number = Date.now();
83+
8284
private pollCount: number = 0;
85+
8386
private sessionId?: string;
8487

8588
constructor({ handle, directResults, context, sessionId }: DBSQLOperationConstructorOptions) {
@@ -236,7 +239,7 @@ export default class DBSQLOperation implements IOperation {
236239
}
237240

238241
// Track poll count for telemetry
239-
this.pollCount++;
242+
this.pollCount += 1;
240243

241244
const driver = await this.context.getDriver();
242245
const response = await driver.getOperationStatus({
@@ -504,7 +507,7 @@ export default class DBSQLOperation implements IOperation {
504507
*/
505508
private emitStatementStart(): void {
506509
try {
507-
const telemetryEmitter = (this.context as any).telemetryEmitter;
510+
const {telemetryEmitter} = (this.context as any);
508511
if (!telemetryEmitter) {
509512
return;
510513
}
@@ -525,8 +528,8 @@ export default class DBSQLOperation implements IOperation {
525528
*/
526529
private emitStatementComplete(): void {
527530
try {
528-
const telemetryEmitter = (this.context as any).telemetryEmitter;
529-
const telemetryAggregator = (this.context as any).telemetryAggregator;
531+
const {telemetryEmitter} = (this.context as any);
532+
const {telemetryAggregator} = (this.context as any);
530533
if (!telemetryEmitter || !telemetryAggregator) {
531534
return;
532535
}
@@ -557,13 +560,12 @@ export default class DBSQLOperation implements IOperation {
557560
*/
558561
private emitErrorEvent(error: Error): void {
559562
try {
560-
const telemetryEmitter = (this.context as any).telemetryEmitter;
563+
const {telemetryEmitter} = (this.context as any);
561564
if (!telemetryEmitter) {
562565
return;
563566
}
564567

565-
// Import ExceptionClassifier at runtime to avoid circular dependencies
566-
const ExceptionClassifier = require('./telemetry/ExceptionClassifier').default;
568+
// Classify the exception
567569
const isTerminal = ExceptionClassifier.isTerminal(error);
568570

569571
telemetryEmitter.emitError({

lib/result/CloudFetchResultHandler.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ export default class CloudFetchResultHandler implements IResultsProvider<ArrowBa
113113
this.logDownloadMetrics(link.fileLink, result.byteLength, downloadTimeMs);
114114

115115
// Emit cloudfetch.chunk telemetry event
116-
this.emitCloudFetchChunk(this.chunkIndex++, downloadTimeMs, result.byteLength);
116+
this.emitCloudFetchChunk(this.chunkIndex, downloadTimeMs, result.byteLength);
117+
this.chunkIndex += 1;
117118

118119
return {
119120
batches: [Buffer.from(result)],
@@ -144,7 +145,7 @@ export default class CloudFetchResultHandler implements IResultsProvider<ArrowBa
144145
return;
145146
}
146147

147-
const telemetryEmitter = (this.context as any).telemetryEmitter;
148+
const {telemetryEmitter} = (this.context as any);
148149
if (!telemetryEmitter) {
149150
return;
150151
}

lib/telemetry/CircuitBreaker.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,13 @@ export const DEFAULT_CIRCUIT_BREAKER_CONFIG: CircuitBreakerConfig = {
6161
*/
6262
export class CircuitBreaker {
6363
private state: CircuitBreakerState = CircuitBreakerState.CLOSED;
64+
6465
private failureCount = 0;
66+
6567
private successCount = 0;
68+
6669
private nextAttempt?: Date;
70+
6771
private readonly config: CircuitBreakerConfig;
6872

6973
constructor(
@@ -138,7 +142,7 @@ export class CircuitBreaker {
138142
this.failureCount = 0;
139143

140144
if (this.state === CircuitBreakerState.HALF_OPEN) {
141-
this.successCount++;
145+
this.successCount += 1;
142146
logger.log(
143147
LogLevel.debug,
144148
`Circuit breaker success in HALF_OPEN (${this.successCount}/${this.config.successThreshold})`
@@ -160,7 +164,7 @@ export class CircuitBreaker {
160164
private onFailure(): void {
161165
const logger = this.context.getLogger();
162166

163-
this.failureCount++;
167+
this.failureCount += 1;
164168
this.successCount = 0; // Reset success count on failure
165169

166170
logger.log(

lib/telemetry/DatabricksTelemetryExporter.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414
* limitations under the License.
1515
*/
1616

17+
import fetch, { Response } from 'node-fetch';
1718
import IClientContext from '../contracts/IClientContext';
1819
import { LogLevel } from '../contracts/IDBSQLLogger';
1920
import { TelemetryMetric, DEFAULT_TELEMETRY_CONFIG } from './types';
2021
import { CircuitBreakerRegistry } from './CircuitBreaker';
2122
import ExceptionClassifier from './ExceptionClassifier';
22-
import fetch, { Response } from 'node-fetch';
2323

2424
/**
2525
* Databricks telemetry log format for export.
@@ -77,7 +77,9 @@ interface DatabricksTelemetryPayload {
7777
*/
7878
export default class DatabricksTelemetryExporter {
7979
private circuitBreaker;
80+
8081
private readonly userAgent: string;
82+
8183
private fetchFn: typeof fetch;
8284

8385
constructor(
@@ -90,7 +92,6 @@ export default class DatabricksTelemetryExporter {
9092
this.fetchFn = fetchFunction || fetch;
9193

9294
// Get driver version for user agent
93-
const config = context.getConfig();
9495
this.userAgent = `databricks-sql-nodejs/${this.getDriverVersion()}`;
9596
}
9697

@@ -131,7 +132,8 @@ export default class DatabricksTelemetryExporter {
131132

132133
let lastError: Error | null = null;
133134

134-
for (let attempt = 0; attempt <= maxRetries; attempt++) {
135+
/* eslint-disable no-await-in-loop */
136+
for (let attempt = 0; attempt <= maxRetries; attempt += 1) {
135137
try {
136138
await this.exportInternal(metrics);
137139
return; // Success
@@ -157,7 +159,7 @@ export default class DatabricksTelemetryExporter {
157159
}
158160

159161
// Calculate backoff with exponential + jitter (100ms - 1000ms)
160-
const baseDelay = Math.min(100 * Math.pow(2, attempt), 1000);
162+
const baseDelay = Math.min(100 * 2**attempt, 1000);
161163
const jitter = Math.random() * 100;
162164
const delay = baseDelay + jitter;
163165

@@ -169,6 +171,7 @@ export default class DatabricksTelemetryExporter {
169171
await this.sleep(delay);
170172
}
171173
}
174+
/* eslint-enable no-await-in-loop */
172175

173176
// Should not reach here, but just in case
174177
if (lastError) {
@@ -200,10 +203,8 @@ export default class DatabricksTelemetryExporter {
200203
`Exporting ${metrics.length} telemetry metrics to ${authenticatedExport ? 'authenticated' : 'unauthenticated'} endpoint`
201204
);
202205

203-
// Get connection provider for auth headers (if authenticated)
204-
const connectionProvider = await this.context.getConnectionProvider();
205-
206206
// Make HTTP POST request
207+
// Note: In production, auth headers would be added via connectionProvider
207208
const response: Response = await this.fetchFn(endpoint, {
208209
method: 'POST',
209210
headers: {
@@ -301,6 +302,8 @@ export default class DatabricksTelemetryExporter {
301302
* Sleep for the specified number of milliseconds.
302303
*/
303304
private sleep(ms: number): Promise<void> {
304-
return new Promise((resolve) => setTimeout(resolve, ms));
305+
return new Promise((resolve) => {
306+
setTimeout(resolve, ms);
307+
});
305308
}
306309
}

lib/telemetry/FeatureFlagCache.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ export interface FeatureFlagContext {
3434
*/
3535
export default class FeatureFlagCache {
3636
private contexts: Map<string, FeatureFlagContext>;
37+
3738
private readonly CACHE_DURATION_MS = 15 * 60 * 1000; // 15 minutes
39+
3840
private readonly FEATURE_FLAG_NAME = 'databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForNodeJs';
3941

4042
constructor(private context: IClientContext) {
@@ -54,7 +56,7 @@ export default class FeatureFlagCache {
5456
};
5557
this.contexts.set(host, ctx);
5658
}
57-
ctx.refCount++;
59+
ctx.refCount += 1;
5860
return ctx;
5961
}
6062

@@ -65,7 +67,7 @@ export default class FeatureFlagCache {
6567
releaseContext(host: string): void {
6668
const ctx = this.contexts.get(host);
6769
if (ctx) {
68-
ctx.refCount--;
70+
ctx.refCount -= 1;
6971
if (ctx.refCount <= 0) {
7072
this.contexts.delete(host);
7173
}
@@ -105,8 +107,10 @@ export default class FeatureFlagCache {
105107
* Fetches feature flag from server.
106108
* This is a placeholder implementation that returns false.
107109
* Real implementation would fetch from server using connection provider.
110+
* @param _host The host to fetch feature flag for (unused in placeholder implementation)
108111
*/
109-
private async fetchFeatureFlag(host: string): Promise<boolean> {
112+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
113+
private async fetchFeatureFlag(_host: string): Promise<boolean> {
110114
// Placeholder implementation
111115
// Real implementation would use:
112116
// const connectionProvider = await this.context.getConnectionProvider();

lib/telemetry/MetricsAggregator.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,13 @@ interface StatementTelemetryDetails {
6060
*/
6161
export default class MetricsAggregator {
6262
private statementMetrics: Map<string, StatementTelemetryDetails> = new Map();
63+
6364
private pendingMetrics: TelemetryMetric[] = [];
65+
6466
private flushTimer: NodeJS.Timeout | null = null;
67+
6568
private batchSize: number;
69+
6670
private flushIntervalMs: number;
6771

6872
constructor(
@@ -171,12 +175,10 @@ export default class MetricsAggregator {
171175

172176
// Flush immediately for terminal errors
173177
this.flush();
174-
} else {
178+
} else if (event.statementId) {
175179
// Retryable error - buffer until statement complete
176-
if (event.statementId) {
177-
const details = this.getOrCreateStatementDetails(event);
178-
details.errors.push(event);
179-
}
180+
const details = this.getOrCreateStatementDetails(event);
181+
details.errors.push(event);
180182
}
181183
}
182184

@@ -201,12 +203,16 @@ export default class MetricsAggregator {
201203
break;
202204

203205
case TelemetryEventType.CLOUDFETCH_CHUNK:
204-
details.chunkCount++;
206+
details.chunkCount += 1;
205207
details.bytesDownloaded += event.bytes ?? 0;
206208
if (event.compressed !== undefined) {
207209
details.compressionEnabled = event.compressed;
208210
}
209211
break;
212+
213+
default:
214+
// Unknown event type - ignore
215+
break;
210216
}
211217
}
212218

lib/telemetry/TelemetryClientProvider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class TelemetryClientProvider {
6767
}
6868

6969
// Increment reference count
70-
holder.refCount++;
70+
holder.refCount += 1;
7171
logger.log(
7272
LogLevel.debug,
7373
`TelemetryClient reference count for ${host}: ${holder.refCount}`
@@ -92,7 +92,7 @@ class TelemetryClientProvider {
9292
}
9393

9494
// Decrement reference count
95-
holder.refCount--;
95+
holder.refCount -= 1;
9696
logger.log(
9797
LogLevel.debug,
9898
`TelemetryClient reference count for ${host}: ${holder.refCount}`

0 commit comments

Comments
 (0)