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

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

vvraskin 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_r246028508
 
 

 ##########
 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) = {
 
 Review comment:
   It seems that this method is not enough to fill the backward compatibility gap... The dots in the resulting metric name are replaced by underscores, e.g. instead of `x.invoker0.counter.y` we see `x.invoker0.counter_y`. 
   
   One possibility to overcome this is to implement a custom metric name generator - `kamon.statsd.MetricKeyGenerator`, I'm not sure how much overhead that would be. Perhaps there are some other options I'm not aware of.

----------------------------------------------------------------
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