Revert "feat: Per-model customization of histogram metric buckets (#405…

…)"

This reverts commit 1d18c2b.

src/backend_model_instance.cc

@@ -192,8 +192,7 @@ TritonModelInstance::TritonModelInstance(
         model_->Server()->ResponseCacheEnabled();
     MetricModelReporter::Create(
         model_->ModelId(), model_->Version(), id, response_cache_enabled,
-        model_->IsDecoupled(), model_->Config().metric_tags(),
-        model_->Config().model_metrics(), &reporter_);
+        model_->IsDecoupled(), model_->Config().metric_tags(), &reporter_);
   }
 #endif  // TRITON_ENABLE_METRICS
 }

src/constants.h

@@ -86,7 +86,6 @@ constexpr char kInitialStateFolder[] = "initial_state";
 // Metric names
 constexpr char kPendingRequestMetric[] = "inf_pending_request_count";
 constexpr char kModelLoadTimeMetric[] = "model_load_time";
-constexpr char kFirstResponseHistogram[] = "first_response_histogram";
 
 constexpr uint64_t NANOS_PER_SECOND = 1000000000;
 constexpr uint64_t NANOS_PER_MILLIS = 1000000;

src/ensemble_scheduler/ensemble_scheduler.cc

@@ -1477,7 +1477,7 @@ EnsembleScheduler::EnsembleScheduler(
     MetricModelReporter::Create(
         model_id, 1 /* model_version */, METRIC_REPORTER_ID_CPU,
         false /* response_cache_enabled */, is_decoupled, config.metric_tags(),
-        config.model_metrics(), &metric_reporter_);
+        &metric_reporter_);
   }
 #endif  // TRITON_ENABLE_METRICS
 

src/infer_response.cc

@@ -317,7 +317,7 @@ InferenceResponse::UpdateResponseMetrics() const
                       .count();
     if (auto reporter = model_->MetricReporter()) {
       reporter->ObserveHistogram(
-          kFirstResponseHistogram,
+          "first_response_histogram",
           (now_ns - infer_start_ns_) / NANOS_PER_MILLIS);
     }
   }

src/metric_model_reporter.cc

@@ -42,8 +42,7 @@ namespace triton { namespace core {
 //
 void
 MetricReporterConfig::ParseConfig(
-    bool response_cache_enabled, bool is_decoupled,
-    const inference::ModelMetrics& model_metrics)
+    bool response_cache_enabled, bool is_decoupled)
 {
   // Global config only for now in config map
   auto metrics_config_map = Metrics::ConfigMap();
@@ -75,26 +74,6 @@ MetricReporterConfig::ParseConfig(
   // Set flag to signal to stats aggregator if caching is enabled or not
   cache_enabled_ = response_cache_enabled;
   is_decoupled_ = is_decoupled;
-
-  // Override default histogram options if set in model_metrics.
-  for (const auto& metric_control : model_metrics.metric_control()) {
-    const std::string& family_name =
-        metric_control.metric_identifier().family();
-
-    // If family name exists, override with new options.
-    if (metric_map_.find(family_name) != metric_map_.end()) {
-      // Copy protobuf RepeatedField to std::vector
-      const auto& buckets_proto = metric_control.histogram_options().buckets();
-      const prometheus::Histogram::BucketBoundaries buckets(
-          buckets_proto.begin(), buckets_proto.end());
-      histogram_options_[metric_map_.at(family_name)] = buckets;
-    } else {
-      // metric_control config may be extended to support backend metrics.
-      LOG_WARNING << "Metric family '" << family_name
-                  << "' in 'metric_identifier' is not a customizable metric in "
-                     "Triton core.";
-    }
-  }
 }
 
 prometheus::Summary::Quantiles
@@ -141,7 +120,6 @@ MetricModelReporter::Create(
     const ModelIdentifier& model_id, const int64_t model_version,
     const int device, bool response_cache_enabled, bool is_decoupled,
     const triton::common::MetricTagsMap& model_tags,
-    const inference::ModelMetrics& model_metrics,
     std::shared_ptr<MetricModelReporter>* metric_model_reporter)
 {
   static std::mutex mtx;
@@ -170,22 +148,21 @@ MetricModelReporter::Create(
 
   metric_model_reporter->reset(new MetricModelReporter(
       model_id, model_version, device, response_cache_enabled, is_decoupled,
-      model_tags, model_metrics));
+      model_tags));
   reporter_map.insert({hash_labels, *metric_model_reporter});
   return Status::Success;
 }
 
 MetricModelReporter::MetricModelReporter(
     const ModelIdentifier& model_id, const int64_t model_version,
     const int device, bool response_cache_enabled, bool is_decoupled,
-    const triton::common::MetricTagsMap& model_tags,
-    const inference::ModelMetrics& model_metrics)
+    const triton::common::MetricTagsMap& model_tags)
 {
   std::map<std::string, std::string> labels;
   GetMetricLabels(&labels, model_id, model_version, device, model_tags);
 
   // Parse metrics config to control metric setup and behavior
-  config_.ParseConfig(response_cache_enabled, is_decoupled, model_metrics);
+  config_.ParseConfig(response_cache_enabled, is_decoupled);
 
   // Initialize families and metrics
   InitializeCounters(labels);
@@ -305,11 +282,10 @@ void
 MetricModelReporter::InitializeHistograms(
     const std::map<std::string, std::string>& labels)
 {
-  // Update MetricReporterConfig::metric_map_ for new histograms.
   // Only create response metrics if decoupled model to reduce metric output
   if (config_.latency_histograms_enabled_) {
     if (config_.is_decoupled_) {
-      histogram_families_[kFirstResponseHistogram] =
+      histogram_families_["first_response_histogram"] =
           &Metrics::FamilyFirstResponseDuration();
     }
   }
@@ -318,9 +294,8 @@ MetricModelReporter::InitializeHistograms(
     const auto& name = iter.first;
     auto family_ptr = iter.second;
     if (family_ptr) {
-      const auto& buckets = config_.histogram_options_[name];
-      histograms_[name] =
-          CreateMetric<prometheus::Histogram>(*family_ptr, labels, buckets);
+      histograms_[name] = CreateMetric<prometheus::Histogram>(
+          *family_ptr, labels, config_.buckets_);
     }
   }
 }

src/metric_model_reporter.h

@@ -46,9 +46,7 @@ struct ModelIdentifier;
 struct MetricReporterConfig {
 #ifdef TRITON_ENABLE_METRICS
   // Parses Metrics::ConfigMap and sets fields if specified
-  void ParseConfig(
-      bool response_cache_enabled, bool is_decoupled,
-      const inference::ModelMetrics& model_metrics);
+  void ParseConfig(bool response_cache_enabled, bool is_decoupled);
   // Parses pairs of quantiles "quantile1:error1, quantile2:error2, ..."
   // and overwrites quantiles_ field if successful.
   prometheus::Summary::Quantiles ParseQuantiles(std::string options);
@@ -59,12 +57,10 @@ struct MetricReporterConfig {
   bool latency_histograms_enabled_ = false;
   // Create and use Summaries for per-model latency related metrics
   bool latency_summaries_enabled_ = false;
-  // Default bucket boundaries used for each histogram metric. Each value
-  // represents a boundary. For example, {100, 500, 2000, 5000} are latencies.
+  // Buckets used for any histogram metrics. Each value represents
+  // a bucket boundary. For example, {100, 500, 2000, 5000} are latencies
   // in milliseconds in first_response_histogram.
-  std::unordered_map<std::string, prometheus::Histogram::BucketBoundaries>
-      histogram_options_ = {{kFirstResponseHistogram, {100, 500, 2000, 5000}}};
-
+  prometheus::Histogram::BucketBoundaries buckets_ = {100, 500, 2000, 5000};
   // Quantiles used for any summary metrics. Each pair of values represents
   // { quantile, error }. For example, {0.90, 0.01} means to compute the
   // 90th percentile with 1% error on either side, so the approximate 90th
@@ -77,14 +73,6 @@ struct MetricReporterConfig {
   bool cache_enabled_ = false;
 
   bool is_decoupled_ = false;
-
- private:
-  // Maps the metric family fullname to its lookup key. This field is required
-  // because the users are expected to configure metric configuration
-  // "ModelMetrics" with the full name displayed from metrics reporting while a
-  // different name is used internally. All new histograms must update the map.
-  const std::unordered_map<std::string, std::string> metric_map_ = {
-      {"nv_inference_first_response_histogram_ms", kFirstResponseHistogram}};
 #endif  // TRITON_ENABLE_METRICS
 };
 
@@ -98,9 +86,7 @@ class MetricModelReporter {
       const triton::core::ModelIdentifier& model_id,
       const int64_t model_version, const int device,
       bool response_cache_enabled, bool is_decoupled,
-      // FIXME: [DLIS-7497] Merge model_tags with model_metrics
       const triton::common::MetricTagsMap& model_tags,
-      const inference::ModelMetrics& model_metrics,
       std::shared_ptr<MetricModelReporter>* metric_model_reporter);
 
   ~MetricModelReporter();
@@ -126,8 +112,7 @@ class MetricModelReporter {
   MetricModelReporter(
       const ModelIdentifier& model_id, const int64_t model_version,
       const int device, bool response_cache_enabled, bool is_decoupled,
-      const triton::common::MetricTagsMap& model_tags,
-      const inference::ModelMetrics& model_metrics);
+      const triton::common::MetricTagsMap& model_tags);
 
   static void GetMetricLabels(
       std::map<std::string, std::string>* labels,

src/metrics.cc

@@ -109,6 +109,12 @@ Metrics::Metrics()
                     "execution per-model.")
               .Register(*registry_)),
 
+      inf_first_response_histogram_ms_family_(
+          prometheus::BuildHistogram()
+              .Name("nv_inference_first_response_histogram_ms")
+              .Help("Duration from request to first response in milliseconds")
+              .Register(*registry_)),
+
       model_load_time_family_(prometheus::BuildGauge()
                                   .Name("nv_model_load_duration_secs")
                                   .Help("Model load time in seconds")
@@ -149,14 +155,6 @@ Metrics::Metrics()
                     "microseconds")
               .Register(*registry_)),
 
-      // Histograms
-      // New histograms must be added to MetricReporterConfig.metric_map_
-      inf_first_response_histogram_ms_family_(
-          prometheus::BuildHistogram()
-              .Name("nv_inference_first_response_histogram_ms")
-              .Help("Duration from request to first response in milliseconds")
-              .Register(*registry_)),
-
       // Summaries
       inf_request_summary_us_family_(
           prometheus::BuildSummary()

src/metrics.h

@@ -312,6 +312,8 @@ class Metrics {
   prometheus::Family<prometheus::Counter>&
       inf_compute_output_duration_us_family_;
   prometheus::Family<prometheus::Gauge>& inf_pending_request_count_family_;
+  prometheus::Family<prometheus::Histogram>&
+      inf_first_response_histogram_ms_family_;
   prometheus::Family<prometheus::Gauge>& model_load_time_family_;
 
   prometheus::Family<prometheus::Gauge>& pinned_memory_pool_total_family_;
@@ -328,10 +330,6 @@ class Metrics {
   prometheus::Family<prometheus::Counter>& cache_num_misses_model_family_;
   prometheus::Family<prometheus::Counter>& cache_miss_duration_us_model_family_;
 
-  // Histograms
-  prometheus::Family<prometheus::Histogram>&
-      inf_first_response_histogram_ms_family_;
-
   // Summaries
   prometheus::Family<prometheus::Summary>& inf_request_summary_us_family_;
   prometheus::Family<prometheus::Summary>& inf_queue_summary_us_family_;

src/model.cc

@@ -135,8 +135,7 @@ Model::Init(const bool is_config_provided)
 #ifdef TRITON_ENABLE_METRICS
   MetricModelReporter::Create(
       ModelId(), Version(), METRIC_REPORTER_ID_UTILITY, ResponseCacheEnabled(),
-      IsDecoupled(), Config().metric_tags(), Config().model_metrics(),
-      &reporter_);
+      IsDecoupled(), Config().metric_tags(), &reporter_);
 #endif  // TRITON_ENABLE_METRICS
 
   return Status::Success;

src/model_config_utils.cc

@@ -446,43 +446,6 @@ ValidateNonLinearFormatIO(
   return Status::Success;
 }
 
-#ifdef TRITON_ENABLE_METRICS
-
-// Helper function to validate that model_metrics contains all required data.
-Status
-ValidateModelMetrics(const inference::ModelMetrics& model_metrics)
-{
-  for (const auto& metric_control : model_metrics.metric_control()) {
-    if (!metric_control.has_metric_identifier()) {
-      return Status(
-          Status::Code::INVALID_ARG,
-          "metric control must specify 'metric_identifier'");
-    }
-
-    if (metric_control.metric_identifier().family().empty()) {
-      return Status(
-          Status::Code::INVALID_ARG,
-          "metric identifier must specify non-empty 'family'");
-    }
-
-    if (!metric_control.has_histogram_options()) {
-      return Status(
-          Status::Code::INVALID_ARG,
-          "metric control must specify 'histogram_options'");
-    }
-
-    if (metric_control.histogram_options().buckets_size() == 0) {
-      return Status(
-          Status::Code::INVALID_ARG,
-          "histogram options must specify non-empty 'buckets'");
-    }
-  }
-
-  return Status::Success;
-}
-
-#endif  // TRITON_ENABLE_METRICS
-
 }  // namespace
 
 Status
@@ -1628,7 +1591,7 @@ ValidateModelConfig(
     }
   }
 
-  // If ensemble scheduling is specified, validate it. Otherwise,
+  // If ensemble scheduling is specified, validate it.  Otherwise,
   // must validate platform and instance_group
   if (config.has_ensemble_scheduling()) {
 #ifdef TRITON_ENABLE_ENSEMBLE
@@ -1657,14 +1620,6 @@ ValidateModelConfig(
             " cache.");
   }
 
-  // If model_metric is specified, validate it.
-  if (config.has_model_metrics()) {
-#ifdef TRITON_ENABLE_METRICS
-    RETURN_IF_ERROR(ValidateModelMetrics(config.model_metrics()));
-#else
-    return Status(Status::Code::INVALID_ARG, "metrics not supported");
-#endif  // TRITON_ENABLE_METRICS
-  }
   return Status::Success;
 }