Fix: 1712 - Validate Instrument meta data (name, unit, description)

api/include/opentelemetry/common/macros.h

@@ -90,6 +90,14 @@
 #  define OPENTELEMETRY_DEPRECATED_MESSAGE(msg)
 #endif
 
+// Regex support
+#if (__GNUC__ == 4 && (__GNUC_MINOR__ == 8 || __GNUC_MINOR__ == 9))
+#  define HAVE_WORKING_REGEX 0
+#else
+#  include <regex>
+#  define HAVE_WORKING_REGEX 1
+#endif
+
 /* clang-format off */
 
 /**

sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h

@@ -0,0 +1,38 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#pragma once
+#ifndef ENABLE_METRICS_PREVIEW
+#  include <string>
+#  include "opentelemetry/common/macros.h"
+#  include "opentelemetry/nostd/string_view.h"
+#  include "opentelemetry/version.h"
+
+#  if HAVE_WORKING_REGEX
+#    include <regex>
+#  endif
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace sdk
+{
+namespace metrics
+{
+class InstrumentMetaDataValidator
+{
+public:
+  InstrumentMetaDataValidator();
+  bool ValidateName(nostd::string_view name) const;
+  bool ValidateUnit(nostd::string_view unit) const;
+  bool ValidateDescription(nostd::string_view description) const;
+
+private:
+#  if HAVE_WORKING_REGEX
+  const std::regex name_reg_key_;
+  const std::regex unit_reg_key_;
+#  endif
+};
+
+}  // namespace metrics
+}  // namespace sdk
+OPENTELEMETRY_END_NAMESPACE
+#endif

sdk/include/opentelemetry/sdk/metrics/meter.h

@@ -5,7 +5,9 @@
 #ifndef ENABLE_METRICS_PREVIEW
 #  include <chrono>
 #  include "opentelemetry/metrics/meter.h"
+#  include "opentelemetry/metrics/noop.h"
 #  include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
+#  include "opentelemetry/sdk/metrics/instrument_metadata_validator.h"
 #  include "opentelemetry/sdk/metrics/instruments.h"
 #  include "opentelemetry/sdk/metrics/meter_context.h"
 #  include "opentelemetry/sdk/metrics/state/async_metric_storage.h"
@@ -121,6 +123,23 @@ class Meter final : public opentelemetry::metrics::Meter
   std::unique_ptr<AsyncWritableMetricStorage> RegisterAsyncMetricStorage(
       InstrumentDescriptor &instrument_descriptor);
   opentelemetry::common::SpinLockMutex storage_lock_;
+  const InstrumentMetaDataValidator instrument_metadata_validator;
+
+  static nostd::shared_ptr<opentelemetry::metrics::ObservableInstrument>
+  GetNoopObservableInsrument()
+  {
+    static nostd::shared_ptr<opentelemetry::metrics::ObservableInstrument> noop_instrument(
+        new opentelemetry::metrics::NoopObservableInstrument("", "", ""));
+    return noop_instrument;
+  }
+  static bool ValidateInstrument(nostd::string_view name,
+                                 nostd::string_view description,
+                                 nostd::string_view unit)
+  {
+    const static InstrumentMetaDataValidator instrument_validator;
+    return instrument_validator.ValidateName(name) && instrument_validator.ValidateUnit(unit) &&
+           instrument_validator.ValidateDescription(description);
+  }
 };
 }  // namespace metrics
 }  // namespace sdk

sdk/include/opentelemetry/sdk/metrics/view/predicate.h

@@ -4,15 +4,9 @@
 #pragma once
 #ifndef ENABLE_METRICS_PREVIEW
 #  include <vector>
-#  if (__GNUC__ == 4 && (__GNUC_MINOR__ == 8 || __GNUC_MINOR__ == 9))
-#    define HAVE_WORKING_REGEX 0
-#    include "opentelemetry/sdk/common/global_log_handler.h"
-#  else
-#    include <regex>
-#    define HAVE_WORKING_REGEX 1
-#  endif
-
+#  include "opentelemetry/common/macros.h"
 #  include "opentelemetry/nostd/string_view.h"
+#  include "opentelemetry/sdk/common/global_log_handler.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace sdk

sdk/src/metrics/CMakeLists.txt

@@ -5,6 +5,7 @@ add_library(
   meter.cc
   meter_context.cc
   metric_reader.cc
+  instrument_metadata_validator.cc
   export/periodic_exporting_metric_reader.cc
   state/metric_collector.cc
   state/observable_registry.cc

sdk/src/metrics/instrument_metadata_validator.cc

@@ -0,0 +1,78 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#ifndef ENABLE_METRICS_PREVIEW
+#  include "opentelemetry/sdk/metrics/instrument_metadata_validator.h"
+#  include "opentelemetry/common/macros.h"
+#  include "opentelemetry/nostd/string_view.h"
+
+#  include <algorithm>
+#  include <iostream>
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace sdk
+{
+namespace metrics
+{
+const size_t kMaxSize = 63;
+// instrument-name = ALPHA 0*62 ("_" / "." / "-" / ALPHA / DIGIT)
+const std::string kInstrumentNamePattern = "[a-zA-Z][-_.a-zA-Z0-9]{0,62}";
+//
+const std::string kInstrumentUnitPattern = "[\x01-\x7F]{0,63}";
+// instrument-unit = It can have a maximum length of 63 ASCII chars
+
+InstrumentMetaDataValidator::InstrumentMetaDataValidator()
+#  if HAVE_WORKING_REGEX
+    : name_reg_key_{kInstrumentNamePattern}, unit_reg_key_
+{
+  kInstrumentUnitPattern
+}
+#  endif
+{}
+
+bool InstrumentMetaDataValidator::ValidateName(nostd::string_view name) const
+{
+#  if HAVE_WORKING_REGEX
+  return std::regex_match(name.data(), name_reg_key_);
+#  else
+  // size atmost 63 chars
+  if (name.size() > kMaxSize)
+  {
+    return false;
+  }
+  // first char should be alphanumeric
+  if (!isalpha(name[0]))
+  {
+    return false;
+  }
+  // subsequent chars should be either of alphabets, underscore, minus, dot
+  return !std::any_of(std::next(name.begin()), name.end(),
+                      [](char c) { return !isalpha(c) && c != '-' && c != '_' && c != '.'; });
+#  endif
+}
+
+bool InstrumentMetaDataValidator::ValidateUnit(nostd::string_view unit) const
+{
+#  if HAVE_WORKING_REGEX
+  return std::regex_match(unit.data(), unit_reg_key_);
+#  else
+  // length atmost 63 chars
+  if (unit.size() > kMaxSize)
+  {
+    return false;
+  }
+  // all should be ascii chars.
+  return !std::any_of(unit.begin(), unit.end(),
+                      [](char c) { return static_cast<unsigned char>(c) > 127; });
+#  endif
+}
+
+bool InstrumentMetaDataValidator::ValidateDescription(nostd::string_view /*description*/) const
+{
+  return true;
+}
+
+}  // namespace metrics
+}  // namespace sdk
+OPENTELEMETRY_END_NAMESPACE
+#endif

sdk/src/metrics/meter.cc

@@ -12,6 +12,7 @@
 #  include "opentelemetry/sdk/metrics/state/observable_registry.h"
 #  include "opentelemetry/sdk/metrics/state/sync_metric_storage.h"
 
+#  include "opentelemetry/sdk/common/global_log_handler.h"
 #  include "opentelemetry/sdk/metrics/sync_instruments.h"
 #  include "opentelemetry/sdk_config.h"
 
@@ -39,6 +40,14 @@ nostd::unique_ptr<metrics::Counter<uint64_t>> Meter::CreateUInt64Counter(
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateUInt64Counter - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return nostd::unique_ptr<metrics::Counter<uint64_t>>(
+        new metrics::NoopCounter<uint64_t>(name, description, unit));
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kLong};
@@ -52,6 +61,14 @@ nostd::unique_ptr<metrics::Counter<double>> Meter::CreateDoubleCounter(
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateDoubleCounter - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return nostd::unique_ptr<metrics::Counter<double>>(
+        new metrics::NoopCounter<double>(name, description, unit));
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kCounter,
@@ -66,6 +83,13 @@ nostd::shared_ptr<opentelemetry::metrics::ObservableInstrument> Meter::CreateInt
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateInt64ObservableCounter - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return GetNoopObservableInsrument();
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter,
@@ -80,6 +104,13 @@ Meter::CreateDoubleObservableCounter(nostd::string_view name,
                                      nostd::string_view description,
                                      nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateDoubleObservableCounter - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return GetNoopObservableInsrument();
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kObservableCounter,
@@ -94,6 +125,14 @@ nostd::unique_ptr<metrics::Histogram<uint64_t>> Meter::CreateUInt64Histogram(
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateUInt64Histogram - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return nostd::unique_ptr<metrics::Histogram<uint64_t>>(
+        new metrics::NoopHistogram<uint64_t>(name, description, unit));
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kHistogram,
@@ -108,6 +147,14 @@ nostd::unique_ptr<metrics::Histogram<double>> Meter::CreateDoubleHistogram(
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateDoubleHistogram - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return nostd::unique_ptr<metrics::Histogram<double>>(
+        new metrics::NoopHistogram<double>(name, description, unit));
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kHistogram,
@@ -122,6 +169,13 @@ nostd::shared_ptr<opentelemetry::metrics::ObservableInstrument> Meter::CreateInt
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateInt64ObservableGauge - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return GetNoopObservableInsrument();
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge,
@@ -136,6 +190,13 @@ nostd::shared_ptr<opentelemetry::metrics::ObservableInstrument> Meter::CreateDou
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateDoubleObservableGauge - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return GetNoopObservableInsrument();
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kObservableGauge,
@@ -150,6 +211,14 @@ nostd::unique_ptr<metrics::UpDownCounter<int64_t>> Meter::CreateInt64UpDownCount
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateInt64UpDownCounter - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return nostd::unique_ptr<metrics::UpDownCounter<int64_t>>(
+        new metrics::NoopUpDownCounter<int64_t>(name, description, unit));
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter,
@@ -164,6 +233,14 @@ nostd::unique_ptr<metrics::UpDownCounter<double>> Meter::CreateDoubleUpDownCount
     nostd::string_view description,
     nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR("Meter::CreateDoubleUpDownCounter - failed. Invalid parameters."
+                            << name << " " << description << " " << unit
+                            << ". Measurements won't be recorded.");
+    return nostd::unique_ptr<metrics::UpDownCounter<double>>(
+        new metrics::NoopUpDownCounter<double>(name, description, unit));
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter,
@@ -178,6 +255,13 @@ Meter::CreateInt64ObservableUpDownCounter(nostd::string_view name,
                                           nostd::string_view description,
                                           nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR(
+        "Meter::CreateInt64ObservableUpDownCounter - failed. Invalid parameters -"
+        << name << " " << description << " " << unit << ". Measurements won't be recorded.");
+    return GetNoopObservableInsrument();
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter,
@@ -192,6 +276,13 @@ Meter::CreateDoubleObservableUpDownCounter(nostd::string_view name,
                                            nostd::string_view description,
                                            nostd::string_view unit) noexcept
 {
+  if (!ValidateInstrument(name, description, unit))
+  {
+    OTEL_INTERNAL_LOG_ERROR(
+        "Meter::CreateDoubleObservableUpDownCounter - failed. Invalid parameters."
+        << name << " " << description << " " << unit << ". Measurements won't be recorded.");
+    return GetNoopObservableInsrument();
+  }
   InstrumentDescriptor instrument_descriptor = {
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kObservableUpDownCounter,

sdk/test/metrics/CMakeLists.txt

@@ -16,7 +16,8 @@ foreach(
   async_instruments_test
   metric_reader_test
   observable_registry_test
-  periodic_exporting_metric_reader_test)
+  periodic_exporting_metric_reader_test
+  instrument_metadata_validator_test)
   add_executable(${testname} "${testname}.cc")
   target_link_libraries(
     ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}