Merge d841f00 into bb017ec

core/include/prometheus/family.h

@@ -88,6 +88,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable {
   /// \param constant_labels Assign a set of key-value pairs (= labels) to the
   /// metric. All these labels are propagated to each time series within the
   /// metric.
+  /// \throw std::runtime_exception on invalid metric or label names.
   Family(const std::string& name, const std::string& help,
          const std::map<std::string, std::string>& constant_labels);
 
@@ -107,6 +108,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable {
   /// Counter, Gauge, Histogram or Summary for required constructor arguments.
   /// \return Return the newly created dimensional data or - if a same set of
   /// labels already exists - the already existing dimensional data.
+  /// \throw std::runtime_exception on invalid label names.
   template <typename... Args>
   T& Add(const std::map<std::string, std::string>& labels, Args&&... args) {
     return Add(labels, detail::make_unique<T>(args...));

core/src/family.cc

@@ -5,13 +5,23 @@
 #include "prometheus/histogram.h"
 #include "prometheus/summary.h"
 
+#include <stdexcept>
+
 namespace prometheus {
 
 template <typename T>
 Family<T>::Family(const std::string& name, const std::string& help,
                   const std::map<std::string, std::string>& constant_labels)
     : name_(name), help_(help), constant_labels_(constant_labels) {
-  assert(CheckMetricName(name_));
+  if (!CheckMetricName(name_)) {
+    throw std::invalid_argument("Invalid metric name");
+  }
+  for (auto& label_pair : constant_labels_) {
+    auto& label_name = label_pair.first;
+    if (!CheckLabelName(label_name)) {
+      throw std::invalid_argument("Invalid label name");
+    }
+  }
 }
 
 template <typename T>
@@ -29,20 +39,20 @@ T& Family<T>::Add(const std::map<std::string, std::string>& labels,
     assert(labels == old_labels);
 #endif
     return *metrics_iter->second;
-  } else {
-#ifndef NDEBUG
-    for (auto& label_pair : labels) {
-      auto& label_name = label_pair.first;
-      assert(CheckLabelName(label_name));
-    }
-#endif
+  }
 
-    auto metric = metrics_.insert(std::make_pair(hash, std::move(object)));
-    assert(metric.second);
-    labels_.insert({hash, labels});
-    labels_reverse_lookup_.insert({metric.first->second.get(), hash});
-    return *(metric.first->second);
+  for (auto& label_pair : labels) {
+    auto& label_name = label_pair.first;
+    if (!CheckLabelName(label_name)) {
+      throw std::invalid_argument("Invalid label name");
+    }
   }
+
+  auto metric = metrics_.insert(std::make_pair(hash, std::move(object)));
+  assert(metric.second);
+  labels_.insert({hash, labels});
+  labels_reverse_lookup_.insert({metric.first->second.get(), hash});
+  return *(metric.first->second);
 }
 
 template <typename T>

core/tests/check_names_test.cc

@@ -12,7 +12,9 @@ TEST(CheckNamesTest, good_metric_name) {
 TEST(CheckNamesTest, reserved_metric_name) {
   EXPECT_FALSE(CheckMetricName("__some_reserved_metric"));
 }
-
+TEST(CheckNamesTest, malformed_metric_name) {
+  EXPECT_FALSE(CheckMetricName("fa mi ly with space in name or |"));
+}
 TEST(CheckNamesTest, empty_label_name) { EXPECT_FALSE(CheckLabelName("")); }
 TEST(CheckNamesTest, good_label_name) { EXPECT_TRUE(CheckLabelName("type")); }
 TEST(CheckNamesTest, reserved_label_name) {

core/tests/family_test.cc

@@ -69,22 +69,28 @@ TEST(FamilyTest, add_twice) {
   ASSERT_EQ(&counter, &counter1);
 }
 
-TEST(FamilyTest, should_assert_on_invalid_metric_name) {
+TEST(FamilyTest, throw_on_invalid_metric_name) {
   auto create_family_with_invalid_name = []() {
     return detail::make_unique<Family<Counter>>(
         "", "empty name", std::map<std::string, std::string>{});
   };
-  EXPECT_DEBUG_DEATH(create_family_with_invalid_name(),
-                     ".*Assertion .*CheckMetricName.*");
+  EXPECT_ANY_THROW(create_family_with_invalid_name());
 }
 
-TEST(FamilyTest, should_assert_on_invalid_labels) {
+TEST(FamilyTest, throw_on_invalid_constant_label_name) {
+  auto create_family_with_invalid_labels = []() {
+    return detail::make_unique<Family<Counter>>(
+        "total_requests", "Counts all requests", std::map<std::string, std::string>{{"__inavlid", "counter1"}});
+  };
+  EXPECT_ANY_THROW(create_family_with_invalid_labels());
+}
+
+TEST(FamilyTest, should_throw_on_invalid_labels) {
   Family<Counter> family{"total_requests", "Counts all requests", {}};
   auto add_metric_with_invalid_label_name = [&family]() {
     family.Add({{"__invalid", "counter1"}});
   };
-  EXPECT_DEBUG_DEATH(add_metric_with_invalid_label_name(),
-                     ".*Assertion .*CheckLabelName.*");
+  EXPECT_ANY_THROW(add_metric_with_invalid_label_name());
 }
 
 }  // namespace