From aa24d0f6a21c9cec81f2e14db75df3fad20f67bc Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Fri, 15 Mar 2019 10:52:09 -0700 Subject: [PATCH 1/2] opencensus-zpages: Enforce strictNullChecks and noUnusedLocals --- .../page-handlers/rpcz.page-handler.ts | 18 +------------ .../opencensus-exporter-zpages/src/zpages.ts | 6 +++-- .../test/test-zpages.ts | 25 +++++++++---------- .../opencensus-exporter-zpages/tsconfig.json | 3 ++- 4 files changed, 19 insertions(+), 33 deletions(-) diff --git a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/rpcz.page-handler.ts b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/rpcz.page-handler.ts index 88a718978..2270a767c 100644 --- a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/rpcz.page-handler.ts +++ b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/rpcz.page-handler.ts @@ -48,22 +48,6 @@ export interface RpczData { measuresReceived: {[key: string]: ZMeasure}; } -enum DefaultMeasures { - CLIENT_SENT_MESSAGES_PER_RPC = 'grpc.io/client/sent_messages_per_rpc', - CLIENT_SENT_BYTES_PER_RPC = 'grpc.io/client/sent_bytes_per_rpc', - CLIENT_RECEIVED_MESSAGES_PER_RPC = 'grpc.io/client/received_messages_per_rpc', - CLIENT_RECEIVED_BYTES_PER_RPC = 'grpc.io/client/received_bytes_per_rpc', - CLIENT_ROUDTRIP_LATENCY = 'grpc.io/client/roundtrip_latency', - CLIENT_SERVER_LATENCY = 'grpc.io/client/server_latency', - CLIENT_STARTED_RPCS = 'grpc.io/client/started_rpcs', - SERVER_RECEIVED_MESSAGES_PER_RPC = 'grpc.io/server/received_messages_per_rpc', - SERVER_RECEIVED_BYTES_PER_RPC = 'grpc.io/server/received_bytes_per_rpc', - SERVER_SENT_MESSAGES_PER_RPC = 'grpc.io/server/sent_messages_per_rpc', - SERVER_SENT_BYTES_PER_RPC = 'grpc.io/server/sent_bytes_per_rpc', - SERVER_SERVER_LATENCY = 'grpc.io/server/server_latency', - SERVER_STARTED_RPCS = 'grpc.io/server/started_rpcs' -} - enum DefaultViews { CLIENT_SENT_BYTES_PER_RPC = 'grpc.io/client/sent_bytes_per_rpc', CLIENT_RECEIVED_BYTES_PER_RPC = 'grpc.io/client/received_bytes_per_rpc', @@ -129,7 +113,7 @@ export class RpczPageHandler { method = snapshot.tagValues[serverMethodIndex].value; zMeasures = rpczData.measuresReceived; } - if (method) { + if (zMeasures && method) { if (!zMeasures[method]) { zMeasures[method] = this.newEmptyZMeasure(); } diff --git a/packages/opencensus-exporter-zpages/src/zpages.ts b/packages/opencensus-exporter-zpages/src/zpages.ts index f0db5ac7a..71a81671a 100644 --- a/packages/opencensus-exporter-zpages/src/zpages.ts +++ b/packages/opencensus-exporter-zpages/src/zpages.ts @@ -42,7 +42,7 @@ export class ZpagesExporter implements Exporter, StatsEventListener { static readonly defaultOptions = {port: 8080, startServer: true}; private app: express.Application; - private server: http.Server; + private server?: http.Server; private port: number; private traces: Map = new Map(); private logger: Logger; @@ -208,6 +208,8 @@ export class ZpagesExporter implements Exporter, StatsEventListener { * @param callback A function that will be called when the server is stopped. */ stopServer(callback?: () => void) { - this.server.close(callback); + if (this.server) { + this.server.close(callback); + } } } diff --git a/packages/opencensus-exporter-zpages/test/test-zpages.ts b/packages/opencensus-exporter-zpages/test/test-zpages.ts index 1d0246a3f..ea96b505e 100644 --- a/packages/opencensus-exporter-zpages/test/test-zpages.ts +++ b/packages/opencensus-exporter-zpages/test/test-zpages.ts @@ -19,7 +19,6 @@ import * as assert from 'assert'; import axios from 'axios'; import * as http from 'http'; import * as qs from 'querystring'; - import {ZpagesExporter, ZpagesExporterOptions} from '../src/zpages'; import {RpczData} from '../src/zpages-frontend/page-handlers/rpcz.page-handler'; import {StatsViewData, StatszParams} from '../src/zpages-frontend/page-handlers/statsz.page-handler'; @@ -268,7 +267,7 @@ describe('Zpages Exporter', () => { it('should get view information', async () => { const view = globalStats.createView( 'test/CountView', measure, AggregationType.COUNT, tagKeys, - 'A count test', null); + 'A count test'); globalStats.registerView(view); globalStats.record([measurement, measurement2], tagMap); @@ -283,7 +282,7 @@ describe('Zpages Exporter', () => { it('should get stats for view', async () => { const view = globalStats.createView( 'test/CountView', measure, AggregationType.COUNT, tagKeys, - 'A count test', null); + 'A count test'); globalStats.registerView(view); globalStats.record([measurement, measurement2], tagMap); @@ -301,8 +300,8 @@ describe('Zpages Exporter', () => { it('should get view information', async () => { globalStats.registerExporter(zpages); const view = globalStats.createView( - 'test/SumView', measure, AggregationType.SUM, tagKeys, 'A sum test', - null); + 'test/SumView', measure, AggregationType.SUM, tagKeys, + 'A sum test'); globalStats.registerView(view); globalStats.record([measurement, measurement2], tagMap); @@ -317,8 +316,8 @@ describe('Zpages Exporter', () => { it('should get stats for view', async () => { globalStats.registerExporter(zpages); const view = globalStats.createView( - 'test/SumView', measure, AggregationType.SUM, tagKeys, 'A sum test', - null); + 'test/SumView', measure, AggregationType.SUM, tagKeys, + 'A sum test'); globalStats.registerView(view); globalStats.record([measurement, measurement2], tagMap); @@ -337,7 +336,7 @@ describe('Zpages Exporter', () => { globalStats.registerExporter(zpages); const view = globalStats.createView( 'test/LastValueView', measure, AggregationType.LAST_VALUE, tagKeys, - 'A last value test', null); + 'A last value test'); globalStats.registerView(view); globalStats.record([measurement, measurement2], tagMap); @@ -353,7 +352,7 @@ describe('Zpages Exporter', () => { globalStats.registerExporter(zpages); const view = globalStats.createView( 'test/LastValueView', measure, AggregationType.LAST_VALUE, tagKeys, - 'A last value test', null); + 'A last value test'); globalStats.registerView(view); globalStats.record([measurement, measurement2], tagMap); @@ -474,13 +473,13 @@ describe('Zpages Exporter', () => { const view4 = globalStats.createView( 'grpc.io/client/completed_rpcs', measure3, AggregationType.COUNT, [GRPC_CLIENT_METHOD, GRPC_CLIENT_STATUS], - 'Number of completed client RPCs', null); + 'Number of completed client RPCs'); globalStats.registerView(view4); const view5 = globalStats.createView( 'grpc.io/client/started_rpcs', measure4, AggregationType.COUNT, [GRPC_CLIENT_METHOD, GRPC_CLIENT_STATUS], - 'Number of started client RPCs', null); + 'Number of started client RPCs'); globalStats.registerView(view5); const measurement = {measure, value: 22000}; @@ -552,13 +551,13 @@ describe('Zpages Exporter', () => { const view4 = globalStats.createView( 'grpc.io/server/completed_rpcs', measure7, AggregationType.COUNT, [GRPC_SERVER_METHOD, GRPC_SERVER_STATUS], - 'Number of completed client RPCs', null); + 'Number of completed client RPCs'); globalStats.registerView(view4); const view5 = globalStats.createView( 'grpc.io/server/started_rpcs', measure8, AggregationType.COUNT, [GRPC_SERVER_METHOD, GRPC_SERVER_STATUS], - 'Number of started client RPCs', null); + 'Number of started client RPCs'); globalStats.registerView(view5); const measurement6 = {measure: measure5, value: 2200}; diff --git a/packages/opencensus-exporter-zpages/tsconfig.json b/packages/opencensus-exporter-zpages/tsconfig.json index 021d9d61a..ab96b7d4e 100644 --- a/packages/opencensus-exporter-zpages/tsconfig.json +++ b/packages/opencensus-exporter-zpages/tsconfig.json @@ -6,7 +6,8 @@ "pretty": true, "module": "commonjs", "target": "es6", - "strictNullChecks": false + "strictNullChecks": false, + "noUnusedLocals": false }, "include": [ "src/**/*.ts", From b2efd48dd2922a721c7c3876f814715424a262c6 Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Tue, 19 Mar 2019 13:38:25 -0700 Subject: [PATCH 2/2] fix tests --- .../src/zpages-frontend/latency-bucket-boundaries.ts | 6 +++--- .../zpages-frontend/page-handlers/statsz.page-handler.ts | 6 +++--- .../page-handlers/traceconfigz.page-handler.ts | 8 +++----- .../zpages-frontend/page-handlers/tracez.page-handler.ts | 2 +- packages/opencensus-exporter-zpages/tsconfig.json | 4 ++-- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/opencensus-exporter-zpages/src/zpages-frontend/latency-bucket-boundaries.ts b/packages/opencensus-exporter-zpages/src/zpages-frontend/latency-bucket-boundaries.ts index 023f798a3..580a6d566 100644 --- a/packages/opencensus-exporter-zpages/src/zpages-frontend/latency-bucket-boundaries.ts +++ b/packages/opencensus-exporter-zpages/src/zpages-frontend/latency-bucket-boundaries.ts @@ -96,7 +96,7 @@ export class LatencyBucketBoundaries { case LatencyBucketBoundaries.SECONDx100_MAX: return '>100s'; default: - return null; + throw new Error('unknown bucket boundary'); } } @@ -122,7 +122,7 @@ export class LatencyBucketBoundaries { case LatencyBucketBoundaries.SECONDx100_MAX: return 'SECONDx100_MAX'; default: - return null; + throw new Error('unknown bucket boundary'); } } @@ -137,7 +137,7 @@ export class LatencyBucketBoundaries { return latency; } } - return null; + throw new Error('unknown bucket boundary'); } /** diff --git a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/statsz.page-handler.ts b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/statsz.page-handler.ts index 58632dcfa..8d8b95037 100644 --- a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/statsz.page-handler.ts +++ b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/statsz.page-handler.ts @@ -68,11 +68,11 @@ export class StatszPageHandler { /** keeps the folders that belong to the current folder */ const folders: FolderType = {}; /** selected view to show */ - let selectedView: View; + let selectedView: View|undefined; /** keeps HTML table content */ let tableContent: string; /** keeps the stats and view data to load UI */ - let statsViewData: StatsViewData; + let statsViewData: StatsViewData|undefined; // gets the path from user if (params.path) { @@ -129,7 +129,7 @@ export class StatszPageHandler { if (selectedView) { const statsData = this.getStatsData(selectedView); const viewFile = this.loaderFile('statsz-view.ejs'); - let viewContentFile: string; + let viewContentFile: string|undefined; let statsContent: string; switch (selectedView.aggregation) { diff --git a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/traceconfigz.page-handler.ts b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/traceconfigz.page-handler.ts index 4a84c4fa9..0eeaf26c6 100644 --- a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/traceconfigz.page-handler.ts +++ b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/traceconfigz.page-handler.ts @@ -19,8 +19,6 @@ import * as tracing from '@opencensus/nodejs'; import * as ejs from 'ejs'; import * as pkgDir from 'pkg-dir'; -import {ZpagesExporter} from '../../zpages'; - // The directory to search for templates. const templatesDir = `${pkgDir.sync(__dirname)}/templates`; @@ -36,7 +34,7 @@ export interface TraceConfigzData { export class TraceConfigzPageHandler { /** Configuration defaults. Currently just the default sampling rate. */ - private defaultConfig: {samplingRate: number;}; + private defaultConfig?: {samplingRate: number;}; /** * Generate Zpages Trace Config HTML Page @@ -83,7 +81,6 @@ export class TraceConfigzPageHandler { private saveChanges(query: Partial): void { /** restore the config to default */ if (query.change === 'restore_default') { - const exporter = tracing.exporter as ZpagesExporter; tracing.tracer.sampler = SamplerBuilder.getSampler(this.defaultConfig!.samplingRate); return; @@ -107,6 +104,7 @@ export class TraceConfigzPageHandler { } else if (samplingProbability === 'never') { return 0; } - return Number(samplingProbability.match(/\((.*)\)/)[1]); + const probability = samplingProbability.match(/\((.*)\)/); + return probability ? Number(probability[1]) : 1 / 10000; } } diff --git a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/tracez.page-handler.ts b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/tracez.page-handler.ts index 1dd680a53..9bc8ed999 100644 --- a/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/tracez.page-handler.ts +++ b/packages/opencensus-exporter-zpages/src/zpages-frontend/page-handlers/tracez.page-handler.ts @@ -84,7 +84,7 @@ const getCanonicalCode = (status: number) => { case 16: return 'UNAUTHENTICATED'; default: - return null; + return 'UNKNOWN'; } }; diff --git a/packages/opencensus-exporter-zpages/tsconfig.json b/packages/opencensus-exporter-zpages/tsconfig.json index ab96b7d4e..4a7edf10f 100644 --- a/packages/opencensus-exporter-zpages/tsconfig.json +++ b/packages/opencensus-exporter-zpages/tsconfig.json @@ -6,8 +6,8 @@ "pretty": true, "module": "commonjs", "target": "es6", - "strictNullChecks": false, - "noUnusedLocals": false + "strictNullChecks": true, + "noUnusedLocals": true }, "include": [ "src/**/*.ts",