From dc820aa28fa2b9bcb7ad11e2a356f5097ed57ddd Mon Sep 17 00:00:00 2001 From: Harry Chen Date: Tue, 11 Dec 2018 17:34:40 +0800 Subject: [PATCH 1/3] fix: fix match with app name bug --- packages/metrics/src/MetricsClient.ts | 65 ++++++++++--------- .../test/unit/MetricsServerManager.test.ts | 22 +++++++ 2 files changed, 55 insertions(+), 32 deletions(-) diff --git a/packages/metrics/src/MetricsClient.ts b/packages/metrics/src/MetricsClient.ts index e3677055..027e7f65 100644 --- a/packages/metrics/src/MetricsClient.ts +++ b/packages/metrics/src/MetricsClient.ts @@ -5,6 +5,7 @@ import { Environment, EnvironmentUtil } from 'pandora-env'; import { MetricsMessengerClient } from './util/MessengerUtil'; import { Counter, FastCompass, Gauge, Histogram, Meter, Proxiable, Timer } from './client'; import { AbstractIndicator } from './indicator/AbstractIndicator'; +import { type } from 'os'; export class MetricsClient extends AbstractIndicator { @@ -60,17 +61,26 @@ export class MetricsClient extends AbstractIndicator { * @param {} name * @param {Proxiable} metric */ - register(group: string, name: MetricName | string, metric: Proxiable | Metric) { + register(group: string, name: MetricName | string, metric: Proxiable | Metric | string) { this.debug(`Register: wait register a metrics name = ${name}`); let newName = this.buildName(name); // 把应用名加上 newName = newName.tagged('appName', this.getAppName()); + if(typeof metric === 'string') { + let newMetric = this.allMetricsRegistry.getMetric(newName); + if(newMetric) { + // 去重，并返回原指标 + return newMetric; + } else { + metric = this.createMetricProxy(metric); + } + } + if (!metric.type) { metric.type = MetricType.GAUGE; } - // 这边暂时不做去重 this.allMetricsRegistry.register(newName, metric); // Gauge 比较特殊，是实际的类，而服务端才是一个代理，和其他 metric 都不同，不需要 proxy @@ -100,6 +110,7 @@ export class MetricsClient extends AbstractIndicator { this.reportMetric(newName, metric, group); } + return metric; } reportMetric(name: MetricName, metric: Metric, group: string) { @@ -154,48 +165,23 @@ export class MetricsClient extends AbstractIndicator { } getCounter(group: string, name: MetricName | string) { - let counter = this.allMetricsRegistry.getMetric(this.buildName(name)); - if (!counter) { - counter = new Counter(); - this.register(group, name, counter); - } - return counter; + return this.register(group, name, 'COUNTER'); } getTimer(group: string, name: MetricName | string) { - let timer = this.allMetricsRegistry.getMetric(this.buildName(name)); - if (!timer) { - timer = new Timer(); - this.register(group, name, timer); - } - return timer; + return this.register(group, name, 'TIMER'); } getMeter(group: string, name: MetricName | string) { - let meter = this.allMetricsRegistry.getMetric(this.buildName(name)); - if (!meter) { - meter = new Meter(); - this.register(group, name, meter); - } - return meter; + return this.register(group, name, 'METER'); } getHistogram(group: string, name: MetricName | string) { - let histogram = this.allMetricsRegistry.getMetric(this.buildName(name)); - if (!histogram) { - histogram = new Histogram(); - this.register(group, name, histogram); - } - return histogram; + return this.register(group, name, 'HISTOGRAM'); } getFastCompass(group: string, name: MetricName | string) { - let fastCompass = this.allMetricsRegistry.getMetric(this.buildName(name)); - if (!fastCompass) { - fastCompass = new FastCompass(); - this.register(group, name, fastCompass); - } - return fastCompass; + return this.register(group, name, 'FASTCOMPASS'); } private buildName(name: MetricName | string): MetricName { @@ -210,4 +196,19 @@ export class MetricsClient extends AbstractIndicator { return new MetricsRegistry(); } + private createMetricProxy(type) { + switch (type) { + case 'COUNTER': + return new Counter(); + case 'METER': + return new Meter(); + case 'TIMER': + return new Timer(); + case 'HISTOGRAM': + return new Histogram(); + case 'FASTCOMPASS': + return new FastCompass(); + } + } + } diff --git a/packages/metrics/test/unit/MetricsServerManager.test.ts b/packages/metrics/test/unit/MetricsServerManager.test.ts index e43290ac..ecc1666a 100644 --- a/packages/metrics/test/unit/MetricsServerManager.test.ts +++ b/packages/metrics/test/unit/MetricsServerManager.test.ts @@ -4,6 +4,7 @@ import {expect} from 'chai'; import {Counter as CounterProxy, Gauge as GaugeProxy, Timer as TimerProxy, Histogram as HistogramProxy, Meter as MeterProxy, FastCompass as FastCompassProxy} from '../../src/client/index'; import {MetricName, BaseCounter, BaseGauge, BaseHistogram, BaseMeter, BaseTimer, BaseFastCompass} from '../../src/common/index'; import {MetricsConstants} from '../../src/MetricsConstants'; +import * as assert from 'assert'; describe('/test/unit/MetricsServerManager.test.ts', () => { @@ -136,5 +137,26 @@ describe('/test/unit/MetricsServerManager.test.ts', () => { expect(server.getAllCategoryMetrics().size).to.equal(6); }); + it('should test get method', () => { + const counter1 = client.getCounter('getter', 'getter.test.counter'); + const counter2 = client.getCounter('getter', 'getter.test.counter'); + assert(counter1 === counter2); + + const meter1 = client.getMeter('getter', 'getter.test.meter'); + const meter2 = client.getMeter('getter', 'getter.test.meter'); + assert(meter1 === meter2); + + const timer1 = client.getTimer('getter', 'getter.test.timer'); + const timer2 = client.getTimer('getter', 'getter.test.timer'); + assert(timer1 === timer2); + + const fastCompass1 = client.getFastCompass('getter', 'getter.test.fastCompass'); + const fastCompass2 = client.getFastCompass('getter', 'getter.test.fastCompass'); + assert(fastCompass1 === fastCompass2); + + const histogram1 = client.getHistogram('getter', 'getter.test.histogram'); + const histogram2 = client.getHistogram('getter', 'getter.test.histogram'); + assert(histogram1 === histogram2); + }); }); From 19617c1ad9775e23ee56749c4a3620a3e6f6d207 Mon Sep 17 00:00:00 2001 From: GuangWong Date: Tue, 11 Dec 2018 17:35:01 +0800 Subject: [PATCH 2/3] fix: add hasOwnProperty check --- packages/metrics/src/common/MetricName.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/metrics/src/common/MetricName.ts b/packages/metrics/src/common/MetricName.ts index eee3852b..86463e63 100644 --- a/packages/metrics/src/common/MetricName.ts +++ b/packages/metrics/src/common/MetricName.ts @@ -176,7 +176,9 @@ export class MetricName { let tagsArr = []; for(let key in this.tags) { - tagsArr.push(key + MetricName.TAGS_CONCAT + this.tags[key]); + if(this.tags.hasOwnProperty(key)) { + tagsArr.push(key + MetricName.TAGS_CONCAT + this.tags[key]); + } } tagsArr.sort(); From 8a1c9556705435b208e1a3b220e4e6a55b766630 Mon Sep 17 00:00:00 2001 From: Harry Chen Date: Tue, 11 Dec 2018 17:38:33 +0800 Subject: [PATCH 3/3] chore: remove wrong import --- packages/metrics/src/MetricsClient.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/metrics/src/MetricsClient.ts b/packages/metrics/src/MetricsClient.ts index 027e7f65..cef51ae6 100644 --- a/packages/metrics/src/MetricsClient.ts +++ b/packages/metrics/src/MetricsClient.ts @@ -5,7 +5,6 @@ import { Environment, EnvironmentUtil } from 'pandora-env'; import { MetricsMessengerClient } from './util/MessengerUtil'; import { Counter, FastCompass, Gauge, Histogram, Meter, Proxiable, Timer } from './client'; import { AbstractIndicator } from './indicator/AbstractIndicator'; -import { type } from 'os'; export class MetricsClient extends AbstractIndicator { @@ -196,7 +195,7 @@ export class MetricsClient extends AbstractIndicator { return new MetricsRegistry(); } - private createMetricProxy(type) { + private createMetricProxy(type: string) { switch (type) { case 'COUNTER': return new Counter();