Merge pull request #282 from midwayjs/fix_metrics_add_name

Fix metrics add name
midwayjs · Dec 11, 2018 · c05d470 · c05d470
2 parents 3004225 + 8a1c955
commit c05d470
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 33 deletions.
diff --git a/packages/metrics/src/MetricsClient.ts b/packages/metrics/src/MetricsClient.ts
@@ -60,17 +60,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> <any> metric);
 
     // Gauge 比较特殊，是实际的类，而服务端才是一个代理，和其他 metric 都不同，不需要 proxy
@@ -100,6 +109,7 @@ export class MetricsClient extends AbstractIndicator {
       this.reportMetric(newName, metric, group);
     }
 
+    return metric;
   }
 
   reportMetric(name: MetricName, metric: Metric, group: string) {
@@ -154,48 +164,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 +195,19 @@ export class MetricsClient extends AbstractIndicator {
     return new MetricsRegistry();
   }
 
+  private createMetricProxy(type: string) {
+    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/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();

diff --git 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);
+  });
 
 });