[GitHub] rabbah commented on a change in pull request #4165: Update to Kamon 1.1.3 from 0.6 series

[GitBox <gi...@apache.org>]

rabbah commented on a change in pull request #4165: Update to Kamon 1.1.3 from 0.6 series
URL: https://github.com/apache/incubator-openwhisk/pull/4165#discussion_r245491258
 
 

 ##########
 File path: common/scala/src/main/scala/org/apache/openwhisk/common/Logging.scala
 ##########
 @@ -210,32 +210,43 @@ object LogMarkerToken {
 }
 
 object MetricEmitter {
-
-  val metrics = Kamon.metrics
+  import kamon.metric.InstrumentFactory.InstrumentTypes._
+  import kamon.metric.InstrumentFactory._
 
   def emitCounterMetric(token: LogMarkerToken, times: Long = 1): Unit = {
     if (TransactionId.metricsKamon) {
       if (TransactionId.metricsKamonTags) {
-        metrics
-          .counter(token.toString, token.tags)
+        Kamon
+          .counter(createName(token.toString, Counter))
+          .refine(token.tags)
           .increment(times)
       } else {
-        metrics.counter(token.toStringWithSubAction).increment(times)
+        Kamon.counter(createName(token.toStringWithSubAction, Counter)).increment(times)
       }
     }
   }
 
   def emitHistogramMetric(token: LogMarkerToken, value: Long): Unit = {
     if (TransactionId.metricsKamon) {
       if (TransactionId.metricsKamonTags) {
-        metrics
-          .histogram(token.toString, token.tags)
+        Kamon
+          .histogram(createName(token.toString, Histogram))
+          .refine(token.tags)
           .record(value)
       } else {
-        metrics.histogram(token.toStringWithSubAction).record(value)
+        Kamon.histogram(createName(token.toStringWithSubAction, Histogram)).record(value)
       }
     }
   }
+
+  /**
+   * Kamon 1.0 onwards does not include the metric type in the metric name which cause issue
+   * for us as we use same metric name for counter and histogram. So to be backward compatible we
+   * need to prefix the name with type
+   */
+  private def createName(name: String, metricType: InstrumentType) = {
+    s"${metricType.name.toLowerCase}.$name"
 
 Review comment:
   can/should the name be lower cases once when con'sing  the `metricType` instance? (looks like this can be lowercased repeatedly here otherwise from a cursory glance.)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services