Derived metrics should be aware of tags (#105)

* Make derived metrics aware of tags

* Adapt example

* Extend derived test in order to cover usecase with different tags

Author: awegrzyn

examples/4-RateDerivedMetric.cxx

@@ -5,17 +5,16 @@
 
 #include "Monitoring/MonitoringFactory.h"
 
-using Monitoring = o2::monitoring::MonitoringFactory;
-using DerivedMetricMode =  o2::monitoring::DerivedMetricMode;
+using namespace o2::monitoring;
 
 int main() {
   // Configure monitoring
   // Pass string with list of URLs as parameter
-  auto monitoring = Monitoring::Get("stdout://");
+  auto monitoring = MonitoringFactory::Get("stdout://");
 
   // now send at least two metrics to see the result
   for (int i = 0; i < 101; i += 10) {
-    monitoring->send({i, "myMetric"}, DerivedMetricMode::RATE);
-    std::this_thread::sleep_for(std::chrono::milliseconds(250));
+    monitoring->send(Metric{i, "myMetric"}.addTags({tags::Subsystem::Readout}), DerivedMetricMode::RATE);
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
   }
 }

include/Monitoring/Metric.h

@@ -75,7 +75,7 @@ class Metric
     /// Add user defined tags
     /// \param tags      r-value to vector of tags
     /// \return          r-value to "this" - to be able to chain methods
-    Metric&& addTags(std::initializer_list<unsigned int>&& tags);
+    Metric&& addTags(std::vector<unsigned int>&& tags);
 
     /// Generetes current timestamp
     /// return          timestamp as std::chrono::system_clock

src/DerivedMetrics.cxx

@@ -25,16 +25,17 @@ namespace monitoring
 Metric DerivedMetrics::rate(Metric& metric)
 {
   // disallow string
-  std::string name = metric.getName();
+  auto tags = metric.getTags();
+  std::string key = metric.getName() + std::string(tags.begin(), tags.end());
   if (metric.getType() == MetricType::STRING) {
     throw MonitoringException("DerivedMetrics/ProcessMetric", "Not able to process string values");
   }
 
   // search for previous value
-  auto search = mStorage.find(name);
+  auto search = mStorage.find(key);
   if (search == mStorage.end()) {
-    mStorage.insert(std::make_pair(name, metric));
-    return Metric{(double) 0.0, name + "Rate"};
+    mStorage.insert(std::make_pair(key, metric));
+    return Metric{(double) 0.0, metric.getName() + "Rate"}.addTags(std::move(tags));
   }
 
   auto timestampDifference = std::chrono::duration_cast<std::chrono::milliseconds>(
@@ -52,25 +53,25 @@ Metric DerivedMetrics::rate(Metric& metric)
   auto rate =  boost::apply_visitor(VariantVisitorRate(timestampCount), current, previous);
 
   // swap metrics
-  mStorage.erase(name);
-  mStorage.insert(std::make_pair(name, metric));
-
-  return Metric{rate, name + "Rate"};
+  mStorage.erase(key);
+  mStorage.insert(std::make_pair(key, metric));
+  return Metric{rate, metric.getName() + "Rate"}.addTags(std::move(tags));
 }
 
 Metric DerivedMetrics::increment(Metric& metric) {
-  std::string name = metric.getName();
-  auto search = mStorage.find(name);
+  auto tags = metric.getTags();
+  std::string key = metric.getName() + std::string(tags.begin(), tags.end());
+  auto search = mStorage.find(key);
   if (search != mStorage.end()) {
     auto currentValue = metric.getValue();
     auto storedValue = search->second.getValue();
     auto value = boost::apply_visitor(VariantVisitorAdd(), currentValue, storedValue);
     mStorage.erase(search);
-    Metric result = Metric{value, name};
-    mStorage.insert(std::make_pair(name, result));
+    Metric result = Metric{value, metric.getName()}.addTags(std::move(tags));
+    mStorage.insert(std::make_pair(key, result));
     return result;
   }
-  mStorage.insert(std::make_pair(name, metric));
+  mStorage.insert(std::make_pair(key, metric));
   return metric;
 }
 

src/Metric.cxx

@@ -56,7 +56,7 @@ boost::variant< int, std::string, double, uint64_t > Metric::getValue() const
   return mValue;
 }
 
-Metric&& Metric::addTags(std::initializer_list<unsigned int>&& tags)
+Metric&& Metric::addTags(std::vector<unsigned int>&& tags)
 {
   mTags = std::move(tags);
   return std::move(*this);

test/testDerived.cxx

@@ -44,16 +44,20 @@ BOOST_AUTO_TEST_CASE(derivedRateDouble) {
     double rate;
   };
   std::vector<RateResults> results = {{1.2, 0 }, {11.2, 100}, {21.2, 100}, {41.2, 200}, {51.2, 100}, {61, 98}};
+  std::vector<RateResults> resultsTagged = {{0.5, 0 }, {5.5, 50}, {10.5, 50}, {20.5, 100}, {40.5, 200}, {45.5, 50}};
   o2::monitoring::DerivedMetrics derivedHandler;
   std::string name("metricDouble");
-
-  for (auto const result : results) {
+  for(std::size_t i = 0; i < results.size(); i++) {
     try {
       std::this_thread::sleep_for(std::chrono::milliseconds(100));
-      o2::monitoring::Metric metric(result.value, name);
+      o2::monitoring::Metric metric(results[i].value, name);
+      o2::monitoring::Metric metricTagged = Metric{resultsTagged[i].value, name}.addTags({o2::monitoring::tags::Subsystem::Readout});
       o2::monitoring::Metric derived = derivedHandler.rate(metric);
+      o2::monitoring::Metric derivedTagged = derivedHandler.rate(metricTagged);
       BOOST_CHECK_EQUAL(derived.getName(), "metricDoubleRate");
-      BOOST_WARN_CLOSE(boost::get<double>(derived.getValue()), result.rate, 1.0);
+      BOOST_CHECK_EQUAL(derivedTagged.getName(), "metricDoubleRate");
+      BOOST_WARN_CLOSE(boost::get<double>(derived.getValue()), results[i].rate, 5.0);
+      BOOST_WARN_CLOSE(boost::get<double>(derivedTagged.getValue()), resultsTagged[i].rate, 5.0);
     } catch(MonitoringException &e) {
       BOOST_CHECK_EQUAL(e.what(), std::string("Not enough values"));
     }