From a8d83d090902a61c284aefb4c4583c6edacf08db Mon Sep 17 00:00:00 2001 From: Yingge He Date: Thu, 1 Aug 2024 04:56:02 -0700 Subject: [PATCH 01/10] Add histogram metric type --- include/triton/core/tritonserver.h | 27 ++++-- python/tritonserver/_c/triton_bindings.pyi | 2 + python/tritonserver/_c/tritonserver_pybind.cc | 19 +++-- src/metric_family.cc | 83 ++++++++++++++++++- src/metric_family.h | 8 +- src/metrics.h | 1 + src/tritonserver.cc | 19 ++++- src/tritonserver_stub.cc | 5 ++ 8 files changed, 145 insertions(+), 19 deletions(-) diff --git a/include/triton/core/tritonserver.h b/include/triton/core/tritonserver.h index ef5a45d6a..edaf554b2 100644 --- a/include/triton/core/tritonserver.h +++ b/include/triton/core/tritonserver.h @@ -91,7 +91,7 @@ struct TRITONSERVER_MetricFamily; /// } /// #define TRITONSERVER_API_VERSION_MAJOR 1 -#define TRITONSERVER_API_VERSION_MINOR 33 +#define TRITONSERVER_API_VERSION_MINOR 34 /// Get the TRITONBACKEND API version supported by the Triton shared /// library. This value can be compared against the @@ -2615,7 +2615,8 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_ServerInferAsync( /// typedef enum TRITONSERVER_metrickind_enum { TRITONSERVER_METRIC_KIND_COUNTER, - TRITONSERVER_METRIC_KIND_GAUGE + TRITONSERVER_METRIC_KIND_GAUGE, + TRITONSERVER_METRIC_KIND_HISTOGRAM } TRITONSERVER_MetricKind; /// Create a new metric family object. The caller takes ownership of the @@ -2655,11 +2656,14 @@ TRITONSERVER_MetricFamilyDelete(struct TRITONSERVER_MetricFamily* family); /// \param family The metric family to add this new metric to. /// \param labels The array of labels to associate with this new metric. /// \param label_count The number of labels. +/// \param buckets Monotonically increasing values representing the +/// bucket boundaries. For histogram only. /// \return a TRITONSERVER_Error indicating success or failure. TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricNew( struct TRITONSERVER_Metric** metric, struct TRITONSERVER_MetricFamily* family, - const struct TRITONSERVER_Parameter** labels, const uint64_t label_count); + const struct TRITONSERVER_Parameter** labels, const uint64_t label_count, + const void* buckets = nullptr); /// Delete a metric object. /// All TRITONSERVER_Metric* objects should be deleted BEFORE their @@ -2672,9 +2676,10 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricDelete( struct TRITONSERVER_Metric* metric); /// Get the current value of a metric object. -/// Supports metrics of kind TRITONSERVER_METRIC_KIND_COUNTER -/// and TRITONSERVER_METRIC_KIND_GAUGE, and returns -/// TRITONSERVER_ERROR_UNSUPPORTED for unsupported TRITONSERVER_MetricKind. +/// Supports metrics of kind TRITONSERVER_METRIC_KIND_COUNTER, +/// TRITONSERVER_METRIC_KIND_GAUGE, TRITONSERVER_METRIC_KIND_HISTOGRAM, and +/// returns TRITONSERVER_ERROR_UNSUPPORTED for unsupported +/// TRITONSERVER_MetricKind. /// /// \param metric The metric object to query. /// \param value Returns the current value of the metric object. @@ -2705,6 +2710,16 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricIncrement( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricSet( struct TRITONSERVER_Metric* metric, double value); +/// Observe the current value of metric to value. +/// Supports metrics of kind TRITONSERVER_METRIC_KIND_HISTOGRAM and returns +/// TRITONSERVER_ERROR_UNSUPPORTED for unsupported TRITONSERVER_MetricKind. +/// +/// \param metric The metric object to update. +/// \param value The amount to observe metric's value to. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricObserve( + struct TRITONSERVER_Metric* metric, double value); + /// Get the TRITONSERVER_MetricKind of metric and its corresponding family. /// /// \param metric The metric object to query. diff --git a/python/tritonserver/_c/triton_bindings.pyi b/python/tritonserver/_c/triton_bindings.pyi index 71deaba6b..15e6b9004 100644 --- a/python/tritonserver/_c/triton_bindings.pyi +++ b/python/tritonserver/_c/triton_bindings.pyi @@ -357,6 +357,7 @@ class TRITONSERVER_Metric: ) -> None: ... def increment(self, arg0: float) -> None: ... def set_value(self, arg0: float) -> None: ... + def observe(self, arg0: float) -> None: ... @property def kind(self) -> TRITONSERVER_MetricKind: ... @property @@ -384,6 +385,7 @@ class TRITONSERVER_MetricKind: __members__: ClassVar[dict] = ... # read-only COUNTER: ClassVar[TRITONSERVER_MetricKind] = ... GAUGE: ClassVar[TRITONSERVER_MetricKind] = ... + HISTOGRAM: ClassVar[TRITONSERVER_MetricKind] = ... __entries: ClassVar[dict] = ... def __init__(self, value: int) -> None: ... def __eq__(self, other: object) -> bool: ... diff --git a/python/tritonserver/_c/tritonserver_pybind.cc b/python/tritonserver/_c/tritonserver_pybind.cc index 6017b3d7e..79c277657 100644 --- a/python/tritonserver/_c/tritonserver_pybind.cc +++ b/python/tritonserver/_c/tritonserver_pybind.cc @@ -1672,14 +1672,16 @@ class PyMetric : public PyWrapper { DESTRUCTOR_WITH_LOG(PyMetric, TRITONSERVER_MetricDelete); PyMetric( PyMetricFamily& family, - const std::vector>& labels) + const std::vector>& labels, + const std::vector* buckets) { std::vector params; for (const auto& label : labels) { params.emplace_back(label->Ptr()); } ThrowIfError(TRITONSERVER_MetricNew( - &triton_object_, family.Ptr(), params.data(), params.size())); + &triton_object_, family.Ptr(), params.data(), params.size(), + reinterpret_cast(buckets))); owned_ = true; } @@ -1700,6 +1702,11 @@ class PyMetric : public PyWrapper { ThrowIfError(TRITONSERVER_MetricSet(triton_object_, val)); } + void Observe(double val) const + { + ThrowIfError(TRITONSERVER_MetricObserve(triton_object_, val)); + } + TRITONSERVER_MetricKind Kind() const { TRITONSERVER_MetricKind val = TRITONSERVER_METRIC_KIND_COUNTER; @@ -2140,7 +2147,8 @@ PYBIND11_MODULE(triton_bindings, m) // TRITONSERVER_MetricKind py::enum_(m, "TRITONSERVER_MetricKind") .value("COUNTER", TRITONSERVER_METRIC_KIND_COUNTER) - .value("GAUGE", TRITONSERVER_METRIC_KIND_GAUGE); + .value("GAUGE", TRITONSERVER_METRIC_KIND_GAUGE) + .value("HISTOGRAM", TRITONSERVER_METRIC_KIND_HISTOGRAM); // TRITONSERVER_MetricFamily py::class_(m, "TRITONSERVER_MetricFamily") .def(py::init< @@ -2149,12 +2157,13 @@ PYBIND11_MODULE(triton_bindings, m) py::class_(m, "TRITONSERVER_Metric") .def( py::init< - PyMetricFamily&, - const std::vector>&>(), + PyMetricFamily&, const std::vector>&, + const std::vector*>(), py::keep_alive<1, 2>()) .def_property_readonly("value", &PyMetric::Value) .def("increment", &PyMetric::Increment) .def("set_value", &PyMetric::SetValue) + .def("observe", &PyMetric::Observe) .def_property_readonly("kind", &PyMetric::Kind); } diff --git a/src/metric_family.cc b/src/metric_family.cc index 8132eab25..a685514ce 100644 --- a/src/metric_family.cc +++ b/src/metric_family.cc @@ -54,6 +54,12 @@ MetricFamily::MetricFamily( .Help(description) .Register(*registry)); break; + case TRITONSERVER_METRIC_KIND_HISTOGRAM: + family_ = reinterpret_cast(&prometheus::BuildHistogram() + .Name(name) + .Help(description) + .Register(*registry)); + break; default: throw std::invalid_argument( "Unsupported kind passed to MetricFamily constructor."); @@ -63,7 +69,9 @@ MetricFamily::MetricFamily( } void* -MetricFamily::Add(std::map label_map, Metric* metric) +MetricFamily::Add( + std::map label_map, Metric* metric, + const std::vector* buckets) { void* prom_metric = nullptr; switch (kind_) { @@ -81,6 +89,17 @@ MetricFamily::Add(std::map label_map, Metric* metric) prom_metric = reinterpret_cast(gauge_ptr); break; } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + if (buckets == nullptr) { + throw std::invalid_argument( + "Histogram must be constructed with bucket boundaries."); + } + auto histogram_family_ptr = + reinterpret_cast*>(family_); + auto histogram_ptr = &histogram_family_ptr->Add(label_map, *buckets); + prom_metric = reinterpret_cast(histogram_ptr); + break; + } default: throw std::invalid_argument( "Unsupported family kind passed to Metric constructor."); @@ -134,6 +153,14 @@ MetricFamily::Remove(void* prom_metric, Metric* metric) gauge_family_ptr->Remove(gauge_ptr); break; } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + auto histogram_family_ptr = + reinterpret_cast*>(family_); + auto histogram_ptr = + reinterpret_cast(prom_metric); + histogram_family_ptr->Remove(histogram_ptr); + break; + } default: // Invalid kind should be caught in constructor LOG_ERROR << "Unsupported kind in Metric destructor."; @@ -169,7 +196,8 @@ MetricFamily::~MetricFamily() // Metric::Metric( TRITONSERVER_MetricFamily* family, - std::vector labels) + std::vector labels, + const std::vector* buckets) { family_ = reinterpret_cast(family); kind_ = family_->Kind(); @@ -188,7 +216,7 @@ Metric::Metric( std::string(reinterpret_cast(param->ValuePointer())); } - metric_ = family_->Add(label_map, this); + metric_ = family_->Add(label_map, this, buckets); } Metric::~Metric() @@ -235,6 +263,11 @@ Metric::Value(double* value) *value = gauge_ptr->Value(); break; } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_HISTOGRAM does not support Value"); + } default: return TRITONSERVER_ErrorNew( TRITONSERVER_ERROR_UNSUPPORTED, @@ -279,6 +312,11 @@ Metric::Increment(double value) } break; } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_HISTOGRAM does not support Increment"); + } default: return TRITONSERVER_ErrorNew( TRITONSERVER_ERROR_UNSUPPORTED, @@ -308,6 +346,45 @@ Metric::Set(double value) gauge_ptr->Set(value); break; } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_HISTOGRAM does not support Set"); + } + default: + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "Unsupported TRITONSERVER_MetricKind"); + } + + return nullptr; // Success +} + +TRITONSERVER_Error* +Metric::Observe(double value) +{ + if (metric_ == nullptr) { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_INTERNAL, + "Could not set metric value. Metric has been invalidated."); + } + + switch (kind_) { + case TRITONSERVER_METRIC_KIND_COUNTER: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_COUNTER does not support Observe"); + } + case TRITONSERVER_METRIC_KIND_GAUGE: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_GAUGE does not support Observe"); + } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + auto histogram_ptr = reinterpret_cast(metric_); + histogram_ptr->Observe(value); + break; + } default: return TRITONSERVER_ErrorNew( TRITONSERVER_ERROR_UNSUPPORTED, diff --git a/src/metric_family.h b/src/metric_family.h index b5d09d864..d3e305c76 100644 --- a/src/metric_family.h +++ b/src/metric_family.h @@ -50,7 +50,9 @@ class MetricFamily { void* Family() const { return family_; } TRITONSERVER_MetricKind Kind() const { return kind_; } - void* Add(std::map label_map, Metric* metric); + void* Add( + std::map label_map, Metric* metric, + const std::vector* buckets = nullptr); void Remove(void* prom_metric, Metric* metric); int NumMetrics() @@ -86,7 +88,8 @@ class Metric { public: Metric( TRITONSERVER_MetricFamily* family, - std::vector labels); + std::vector labels, + const std::vector* buckets = nullptr); ~Metric(); MetricFamily* Family() const { return family_; } @@ -95,6 +98,7 @@ class Metric { TRITONSERVER_Error* Value(double* value); TRITONSERVER_Error* Increment(double value); TRITONSERVER_Error* Set(double value); + TRITONSERVER_Error* Observe(double value); // If a MetricFamily is deleted before its dependent Metric, we want to // invalidate the references so we don't access invalid memory. diff --git a/src/metrics.h b/src/metrics.h index 2abec1cf3..90a030d80 100644 --- a/src/metrics.h +++ b/src/metrics.h @@ -35,6 +35,7 @@ #include "prometheus/counter.h" #include "prometheus/gauge.h" +#include "prometheus/histogram.h" #include "prometheus/registry.h" #include "prometheus/serializer.h" #include "prometheus/summary.h" diff --git a/src/tritonserver.cc b/src/tritonserver.cc index 82642d5dc..a9d0d2f6d 100644 --- a/src/tritonserver.cc +++ b/src/tritonserver.cc @@ -3371,7 +3371,8 @@ TRITONSERVER_MetricFamilyDelete(TRITONSERVER_MetricFamily* family) TRITONSERVER_Error* TRITONSERVER_MetricNew( TRITONSERVER_Metric** metric, TRITONSERVER_MetricFamily* family, - const TRITONSERVER_Parameter** labels, const uint64_t label_count) + const TRITONSERVER_Parameter** labels, const uint64_t label_count, + const void* buckets) { #ifdef TRITON_ENABLE_METRICS std::vector labels_vec; @@ -3381,8 +3382,9 @@ TRITONSERVER_MetricNew( } try { - *metric = reinterpret_cast( - new tc::Metric(family, labels_vec)); + *metric = reinterpret_cast(new tc::Metric( + family, labels_vec, + reinterpret_cast*>(buckets))); } catch (std::invalid_argument const& ex) { // Catch invalid kinds passed to constructor @@ -3450,6 +3452,17 @@ TRITONSERVER_MetricSet(TRITONSERVER_Metric* metric, double value) #endif // TRITON_ENABLE_METRICS } +TRITONSERVER_Error* +TRITONSERVER_MetricObserve(TRITONSERVER_Metric* metric, double value) +{ +#ifdef TRITON_ENABLE_METRICS + return reinterpret_cast(metric)->Observe(value); +#else + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + TRITONSERVER_Error* TRITONSERVER_GetMetricKind( TRITONSERVER_Metric* metric, TRITONSERVER_MetricKind* kind) diff --git a/src/tritonserver_stub.cc b/src/tritonserver_stub.cc index cd1e03e15..302e56f64 100644 --- a/src/tritonserver_stub.cc +++ b/src/tritonserver_stub.cc @@ -1100,6 +1100,11 @@ TRITONSERVER_MetricSet() { } +TRITONAPI_DECLSPEC void +TRITONSERVER_MetricObserve() +{ +} + TRITONAPI_DECLSPEC void TRITONSERVER_GetMetricKind() { From d560f64079440f121deca6f97cca32884ed411a2 Mon Sep 17 00:00:00 2001 From: Yingge He Date: Tue, 6 Aug 2024 19:07:45 -0700 Subject: [PATCH 02/10] Add collect api for metrics --- include/triton/core/tritonserver.h | 12 +++++ src/metric_family.cc | 44 +++++++++++++++++- src/metric_family.h | 1 + src/test/metrics_api_test.cc | 72 ++++++++++++++++++++++++++++++ src/tritonserver.cc | 12 +++++ 5 files changed, 140 insertions(+), 1 deletion(-) diff --git a/include/triton/core/tritonserver.h b/include/triton/core/tritonserver.h index edaf554b2..baa757128 100644 --- a/include/triton/core/tritonserver.h +++ b/include/triton/core/tritonserver.h @@ -2720,6 +2720,18 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricSet( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricObserve( struct TRITONSERVER_Metric* metric, double value); +/// Collect metrics. +/// Supports metrics of kind TRITONSERVER_METRIC_KIND_COUNTER, +/// TRITONSERVER_METRIC_KIND_GAUGE, TRITONSERVER_METRIC_KIND_HISTOGRAM and +/// returns TRITONSERVER_ERROR_UNSUPPORTED for unsupported +/// TRITONSERVER_MetricKind. +/// +/// \param metric The metric object to collect. +/// \param value Returns the current value of the metric object. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricCollect( + struct TRITONSERVER_Metric* metric, void* value); + /// Get the TRITONSERVER_MetricKind of metric and its corresponding family. /// /// \param metric The metric object to query. diff --git a/src/metric_family.cc b/src/metric_family.cc index a685514ce..17380ddf0 100644 --- a/src/metric_family.cc +++ b/src/metric_family.cc @@ -76,6 +76,10 @@ MetricFamily::Add( void* prom_metric = nullptr; switch (kind_) { case TRITONSERVER_METRIC_KIND_COUNTER: { + if (buckets != nullptr) { + throw std::invalid_argument( + "Unexpected buckets found in counter Metric constructor."); + } auto counter_family_ptr = reinterpret_cast*>(family_); auto counter_ptr = &counter_family_ptr->Add(label_map); @@ -83,6 +87,10 @@ MetricFamily::Add( break; } case TRITONSERVER_METRIC_KIND_GAUGE: { + if (buckets != nullptr) { + throw std::invalid_argument( + "Unexpected buckets found in gauge Metric constructor."); + } auto gauge_family_ptr = reinterpret_cast*>(family_); auto gauge_ptr = &gauge_family_ptr->Add(label_map); @@ -92,7 +100,7 @@ MetricFamily::Add( case TRITONSERVER_METRIC_KIND_HISTOGRAM: { if (buckets == nullptr) { throw std::invalid_argument( - "Histogram must be constructed with bucket boundaries."); + "Missing required buckets in histogram Metric constructor."); } auto histogram_family_ptr = reinterpret_cast*>(family_); @@ -394,6 +402,40 @@ Metric::Observe(double value) return nullptr; // Success } +TRITONSERVER_Error* +Metric::Collect(prometheus::ClientMetric* value) +{ + if (metric_ == nullptr) { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_INTERNAL, + "Could not collect metric value. Metric has been invalidated."); + } + + switch (kind_) { + case TRITONSERVER_METRIC_KIND_COUNTER: { + auto counter_ptr = reinterpret_cast(metric_); + *value = counter_ptr->Collect(); + break; + } + case TRITONSERVER_METRIC_KIND_GAUGE: { + auto gauge_ptr = reinterpret_cast(metric_); + *value = gauge_ptr->Collect(); + break; + } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + auto histogram_ptr = reinterpret_cast(metric_); + *value = histogram_ptr->Collect(); + break; + } + default: + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "Unsupported TRITONSERVER_MetricKind"); + } + + return nullptr; // Success +} + }} // namespace triton::core #endif // TRITON_ENABLE_METRICS diff --git a/src/metric_family.h b/src/metric_family.h index d3e305c76..3565d8fec 100644 --- a/src/metric_family.h +++ b/src/metric_family.h @@ -99,6 +99,7 @@ class Metric { TRITONSERVER_Error* Increment(double value); TRITONSERVER_Error* Set(double value); TRITONSERVER_Error* Observe(double value); + TRITONSERVER_Error* Collect(prometheus::ClientMetric* value); // If a MetricFamily is deleted before its dependent Metric, we want to // invalidate the references so we don't access invalid memory. diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index 3356493a3..35f6b27b5 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -232,6 +232,32 @@ MetricAPIHelper(TRITONSERVER_Metric* metric, TRITONSERVER_MetricKind kind) TRITONSERVER_ErrorDelete(err); } +void +HistogramAPIHelper(TRITONSERVER_Metric* metric) +{ + // Observe + std::vector data{0.05, 1.5, 6.0}; + std::vector cumulative_counts = {1, 1, 2, 2, 3, 3}; + double sum = 0.0; + for (auto datum : data) { + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricObserve(metric, datum), "observe metric value"); + sum += datum; + } + + // Collect + prometheus::ClientMetric value; + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricCollect(metric, &value), + "query metric value after observe"); + auto hist = value.histogram; + ASSERT_EQ(hist.sample_count, data.size()); + ASSERT_EQ(hist.sample_sum, sum); + ASSERT_EQ(hist.bucket.size(), cumulative_counts.size()); + for (uint64_t i = 0; i < hist.bucket.size(); ++i) { + ASSERT_EQ(hist.bucket[i].cumulative_count, cumulative_counts[i]); + } +} // Test Fixture class MetricsApiTest : public ::testing::Test { @@ -364,6 +390,52 @@ TEST_F(MetricsApiTest, TestGaugeEndToEnd) ASSERT_EQ(NumMetricMatches(server_, description), 0); } +// Test end-to-end flow of Generic Metrics API for Histogram metric +TEST_F(MetricsApiTest, TestHistogramEndToEnd) +{ + // Create metric family + TRITONSERVER_MetricFamily* family; + TRITONSERVER_MetricKind kind = TRITONSERVER_METRIC_KIND_HISTOGRAM; + const char* name = "custom_histogram_example"; + const char* description = + "this is an example histogram metric added via API."; + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricFamilyNew(&family, kind, name, description), + "Creating new metric family"); + + // Create metric + TRITONSERVER_Metric* metric; + std::vector labels; + labels.emplace_back(TRITONSERVER_ParameterNew( + "example1", TRITONSERVER_PARAMETER_STRING, "histogram_label1")); + labels.emplace_back(TRITONSERVER_ParameterNew( + "example2", TRITONSERVER_PARAMETER_STRING, "histogram_label2")); + std::vector buckets = {0.1, 1.0, 2.5, 5.0, 10.0}; + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricNew( + &metric, family, labels.data(), labels.size(), + reinterpret_cast(&buckets)), + "Creating new metric"); + for (const auto label : labels) { + TRITONSERVER_ParameterDelete(const_cast(label)); + } + + // Run through metric APIs and assert correctness + HistogramAPIHelper(metric); + + // Assert custom metric is reported and found in output + ASSERT_EQ(NumMetricMatches(server_, description), 1); + + // Cleanup + FAIL_TEST_IF_ERR(TRITONSERVER_MetricDelete(metric), "delete metric"); + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricFamilyDelete(family), "delete metric family"); + + // Assert custom metric/family is unregistered and no longer in output + ASSERT_EQ(NumMetricMatches(server_, description), 0); +} + + // Test that a duplicate metric family can't be added // with a conflicting type/kind TEST_F(MetricsApiTest, TestDupeMetricFamilyDiffKind) diff --git a/src/tritonserver.cc b/src/tritonserver.cc index a9d0d2f6d..ac36c2445 100644 --- a/src/tritonserver.cc +++ b/src/tritonserver.cc @@ -3463,6 +3463,18 @@ TRITONSERVER_MetricObserve(TRITONSERVER_Metric* metric, double value) #endif // TRITON_ENABLE_METRICS } +TRITONSERVER_Error* +TRITONSERVER_MetricCollect(TRITONSERVER_Metric* metric, void* value) +{ +#ifdef TRITON_ENABLE_METRICS + return reinterpret_cast(metric)->Collect( + reinterpret_cast(value)); +#else + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + TRITONSERVER_Error* TRITONSERVER_GetMetricKind( TRITONSERVER_Metric* metric, TRITONSERVER_MetricKind* kind) From fd5c44bae3fb4eaef8b0ac277d68142a8eb11e03 Mon Sep 17 00:00:00 2001 From: Yingge He Date: Tue, 6 Aug 2024 22:04:18 -0700 Subject: [PATCH 03/10] Update copyrights --- src/metric_family.cc | 3 ++- src/metric_family.h | 3 ++- src/metrics.h | 2 +- src/test/metrics_api_test.cc | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/metric_family.cc b/src/metric_family.cc index 17380ddf0..70cc23cc3 100644 --- a/src/metric_family.cc +++ b/src/metric_family.cc @@ -1,4 +1,5 @@ -// Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// Copyright (c) 2022-2024, NVIDIA CORPORATION & AFFILIATES. All rights +// reserved. // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions diff --git a/src/metric_family.h b/src/metric_family.h index 3565d8fec..ad08f81c3 100644 --- a/src/metric_family.h +++ b/src/metric_family.h @@ -1,4 +1,5 @@ -// Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// Copyright (c) 2022-2024, NVIDIA CORPORATION & AFFILIATES. All rights +// reserved. // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions diff --git a/src/metrics.h b/src/metrics.h index 90a030d80..6d08ad168 100644 --- a/src/metrics.h +++ b/src/metrics.h @@ -1,4 +1,4 @@ -// Copyright 2018-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// Copyright 2018-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index 35f6b27b5..9a6fc33ea 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -1,4 +1,5 @@ -// Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// Copyright (c) 2022-2024, NVIDIA CORPORATION & AFFILIATES. All rights +// reserved. // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions From edb05338a26b4ec347639d77060aefdeed6ba23b Mon Sep 17 00:00:00 2001 From: Yingge He Date: Mon, 12 Aug 2024 19:30:47 -0700 Subject: [PATCH 04/10] Update C API --- include/triton/core/tritonserver.h | 77 +++++++++++++--- python/tritonserver/_c/triton_bindings.pyi | 2 - python/tritonserver/_c/tritonserver_pybind.cc | 19 ++-- src/metric_family.cc | 55 +++--------- src/metric_family.h | 29 +++++- src/test/metrics_api_test.cc | 18 ++-- src/tritonserver.cc | 88 ++++++++++++++++--- src/tritonserver_stub.cc | 10 +-- 8 files changed, 199 insertions(+), 99 deletions(-) diff --git a/include/triton/core/tritonserver.h b/include/triton/core/tritonserver.h index baa757128..78f13bf85 100644 --- a/include/triton/core/tritonserver.h +++ b/include/triton/core/tritonserver.h @@ -64,6 +64,7 @@ struct TRITONSERVER_Server; struct TRITONSERVER_ServerOptions; struct TRITONSERVER_Metric; struct TRITONSERVER_MetricFamily; +struct TRITONSERVER_MetricArgs; /// /// TRITONSERVER API Version @@ -2645,6 +2646,42 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricFamilyNew( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricFamilyDelete(struct TRITONSERVER_MetricFamily* family); +/// Get the TRITONSERVER_MetricKind of the metric family. +/// +/// \param metric The metric family object to query. +/// \param kind Returns the TRITONSERVER_MetricKind of metric. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* +TRITONSERVER_GetMetricFamilyKind( + struct TRITONSERVER_MetricFamily* family, TRITONSERVER_MetricKind* kind); + +/// Create a new metric args object. The caller takes ownership of the +/// TRITONSERVER_MetricArgs object and must call TRITONSERVER_MetricArgsDelete +/// to release the object. +/// +/// \param args Returns the new metric args object. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricArgsNew( + struct TRITONSERVER_MetricArgs** args); + +/// Set metric args with prometheus histogram metric parameter. +/// +/// \param args The metric args object to set. +/// \param buckets The array of bucket boundaries. +/// \param buckets_count The number of bucket boundaries. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* +TRITONSERVER_MetricArgsSetHistogram( + struct TRITONSERVER_MetricArgs* args, const double* buckets, + const uint64_t buckets_count); + +/// Delete a metric args object. +/// +/// \param args The metric args object. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricArgsDelete( + struct TRITONSERVER_MetricArgs* args); + /// Create a new metric object. The caller takes ownership of the /// TRITONSERVER_Metric object and must call /// TRITONSERVER_MetricDelete to release the object. The caller is also @@ -2660,10 +2697,31 @@ TRITONSERVER_MetricFamilyDelete(struct TRITONSERVER_MetricFamily* family); /// bucket boundaries. For histogram only. /// \return a TRITONSERVER_Error indicating success or failure. TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricNew( + struct TRITONSERVER_Metric** metric, + struct TRITONSERVER_MetricFamily* family, + const struct TRITONSERVER_Parameter** labels, const uint64_t label_count); + +/// Create a new metric object. The caller takes ownership of the +/// TRITONSERVER_Metric object and must call +/// TRITONSERVER_MetricDelete to release the object. The caller is also +/// responsible for ownership of the labels passed in. +/// Each label can be deleted immediately after creating the metric with +/// TRITONSERVER_ParameterDelete if not re-using the labels. +/// Metric args can be deleted immediately after creating the metric with +/// TRITONSERVER_MetricArgsDelete if not re-using the metric args. +/// +/// \param metric Returns the new metric object. +/// \param family The metric family to add this new metric to. +/// \param labels The array of labels to associate with this new metric. +/// \param label_count The number of labels. +/// \param args Metric args that store additional arguments to construct +/// particular metric types, e.g. histogram. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricNewWithArgs( struct TRITONSERVER_Metric** metric, struct TRITONSERVER_MetricFamily* family, const struct TRITONSERVER_Parameter** labels, const uint64_t label_count, - const void* buckets = nullptr); + const struct TRITONSERVER_MetricArgs* args); /// Delete a metric object. /// All TRITONSERVER_Metric* objects should be deleted BEFORE their @@ -2700,8 +2758,9 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricValue( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricIncrement( struct TRITONSERVER_Metric* metric, double value); -/// Set the current value of metric to value. -/// Supports metrics of kind TRITONSERVER_METRIC_KIND_GAUGE and returns +/// Set the current value of metric to value or observe the value to metric. +/// Supports metrics of kind TRITONSERVER_METRIC_KIND_GAUGE and +/// TRITONSERVER_METRIC_KIND_HISTOGRAM. Returns /// TRITONSERVER_ERROR_UNSUPPORTED for unsupported TRITONSERVER_MetricKind. /// /// \param metric The metric object to update. @@ -2710,16 +2769,6 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricIncrement( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricSet( struct TRITONSERVER_Metric* metric, double value); -/// Observe the current value of metric to value. -/// Supports metrics of kind TRITONSERVER_METRIC_KIND_HISTOGRAM and returns -/// TRITONSERVER_ERROR_UNSUPPORTED for unsupported TRITONSERVER_MetricKind. -/// -/// \param metric The metric object to update. -/// \param value The amount to observe metric's value to. -/// \return a TRITONSERVER_Error indicating success or failure. -TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricObserve( - struct TRITONSERVER_Metric* metric, double value); - /// Collect metrics. /// Supports metrics of kind TRITONSERVER_METRIC_KIND_COUNTER, /// TRITONSERVER_METRIC_KIND_GAUGE, TRITONSERVER_METRIC_KIND_HISTOGRAM and @@ -2732,7 +2781,7 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricObserve( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricCollect( struct TRITONSERVER_Metric* metric, void* value); -/// Get the TRITONSERVER_MetricKind of metric and its corresponding family. +/// Get the TRITONSERVER_MetricKind of metric of its corresponding family. /// /// \param metric The metric object to query. /// \param kind Returns the TRITONSERVER_MetricKind of metric. diff --git a/python/tritonserver/_c/triton_bindings.pyi b/python/tritonserver/_c/triton_bindings.pyi index 15e6b9004..71deaba6b 100644 --- a/python/tritonserver/_c/triton_bindings.pyi +++ b/python/tritonserver/_c/triton_bindings.pyi @@ -357,7 +357,6 @@ class TRITONSERVER_Metric: ) -> None: ... def increment(self, arg0: float) -> None: ... def set_value(self, arg0: float) -> None: ... - def observe(self, arg0: float) -> None: ... @property def kind(self) -> TRITONSERVER_MetricKind: ... @property @@ -385,7 +384,6 @@ class TRITONSERVER_MetricKind: __members__: ClassVar[dict] = ... # read-only COUNTER: ClassVar[TRITONSERVER_MetricKind] = ... GAUGE: ClassVar[TRITONSERVER_MetricKind] = ... - HISTOGRAM: ClassVar[TRITONSERVER_MetricKind] = ... __entries: ClassVar[dict] = ... def __init__(self, value: int) -> None: ... def __eq__(self, other: object) -> bool: ... diff --git a/python/tritonserver/_c/tritonserver_pybind.cc b/python/tritonserver/_c/tritonserver_pybind.cc index 79c277657..6017b3d7e 100644 --- a/python/tritonserver/_c/tritonserver_pybind.cc +++ b/python/tritonserver/_c/tritonserver_pybind.cc @@ -1672,16 +1672,14 @@ class PyMetric : public PyWrapper { DESTRUCTOR_WITH_LOG(PyMetric, TRITONSERVER_MetricDelete); PyMetric( PyMetricFamily& family, - const std::vector>& labels, - const std::vector* buckets) + const std::vector>& labels) { std::vector params; for (const auto& label : labels) { params.emplace_back(label->Ptr()); } ThrowIfError(TRITONSERVER_MetricNew( - &triton_object_, family.Ptr(), params.data(), params.size(), - reinterpret_cast(buckets))); + &triton_object_, family.Ptr(), params.data(), params.size())); owned_ = true; } @@ -1702,11 +1700,6 @@ class PyMetric : public PyWrapper { ThrowIfError(TRITONSERVER_MetricSet(triton_object_, val)); } - void Observe(double val) const - { - ThrowIfError(TRITONSERVER_MetricObserve(triton_object_, val)); - } - TRITONSERVER_MetricKind Kind() const { TRITONSERVER_MetricKind val = TRITONSERVER_METRIC_KIND_COUNTER; @@ -2147,8 +2140,7 @@ PYBIND11_MODULE(triton_bindings, m) // TRITONSERVER_MetricKind py::enum_(m, "TRITONSERVER_MetricKind") .value("COUNTER", TRITONSERVER_METRIC_KIND_COUNTER) - .value("GAUGE", TRITONSERVER_METRIC_KIND_GAUGE) - .value("HISTOGRAM", TRITONSERVER_METRIC_KIND_HISTOGRAM); + .value("GAUGE", TRITONSERVER_METRIC_KIND_GAUGE); // TRITONSERVER_MetricFamily py::class_(m, "TRITONSERVER_MetricFamily") .def(py::init< @@ -2157,13 +2149,12 @@ PYBIND11_MODULE(triton_bindings, m) py::class_(m, "TRITONSERVER_Metric") .def( py::init< - PyMetricFamily&, const std::vector>&, - const std::vector*>(), + PyMetricFamily&, + const std::vector>&>(), py::keep_alive<1, 2>()) .def_property_readonly("value", &PyMetric::Value) .def("increment", &PyMetric::Increment) .def("set_value", &PyMetric::SetValue) - .def("observe", &PyMetric::Observe) .def_property_readonly("kind", &PyMetric::Kind); } diff --git a/src/metric_family.cc b/src/metric_family.cc index 70cc23cc3..0a145b80c 100644 --- a/src/metric_family.cc +++ b/src/metric_family.cc @@ -72,14 +72,14 @@ MetricFamily::MetricFamily( void* MetricFamily::Add( std::map label_map, Metric* metric, - const std::vector* buckets) + const TritonServerMetricArgs* args) { void* prom_metric = nullptr; switch (kind_) { case TRITONSERVER_METRIC_KIND_COUNTER: { - if (buckets != nullptr) { + if (args != nullptr) { throw std::invalid_argument( - "Unexpected buckets found in counter Metric constructor."); + "Unexpected args found in counter Metric constructor."); } auto counter_family_ptr = reinterpret_cast*>(family_); @@ -88,9 +88,9 @@ MetricFamily::Add( break; } case TRITONSERVER_METRIC_KIND_GAUGE: { - if (buckets != nullptr) { + if (args != nullptr) { throw std::invalid_argument( - "Unexpected buckets found in gauge Metric constructor."); + "Unexpected args found in gauge Metric constructor."); } auto gauge_family_ptr = reinterpret_cast*>(family_); @@ -99,13 +99,14 @@ MetricFamily::Add( break; } case TRITONSERVER_METRIC_KIND_HISTOGRAM: { - if (buckets == nullptr) { + if (args == nullptr) { throw std::invalid_argument( - "Missing required buckets in histogram Metric constructor."); + "Bucket boundaries not found in Metric constructor args."); } auto histogram_family_ptr = reinterpret_cast*>(family_); - auto histogram_ptr = &histogram_family_ptr->Add(label_map, *buckets); + auto histogram_ptr = + &histogram_family_ptr->Add(label_map, args->buckets()); prom_metric = reinterpret_cast(histogram_ptr); break; } @@ -206,7 +207,7 @@ MetricFamily::~MetricFamily() Metric::Metric( TRITONSERVER_MetricFamily* family, std::vector labels, - const std::vector* buckets) + const TritonServerMetricArgs* args) { family_ = reinterpret_cast(family); kind_ = family_->Kind(); @@ -225,7 +226,7 @@ Metric::Metric( std::string(reinterpret_cast(param->ValuePointer())); } - metric_ = family_->Add(label_map, this, buckets); + metric_ = family_->Add(label_map, this, args); } Metric::~Metric() @@ -355,40 +356,6 @@ Metric::Set(double value) gauge_ptr->Set(value); break; } - case TRITONSERVER_METRIC_KIND_HISTOGRAM: { - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_UNSUPPORTED, - "TRITONSERVER_METRIC_KIND_HISTOGRAM does not support Set"); - } - default: - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_UNSUPPORTED, - "Unsupported TRITONSERVER_MetricKind"); - } - - return nullptr; // Success -} - -TRITONSERVER_Error* -Metric::Observe(double value) -{ - if (metric_ == nullptr) { - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_INTERNAL, - "Could not set metric value. Metric has been invalidated."); - } - - switch (kind_) { - case TRITONSERVER_METRIC_KIND_COUNTER: { - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_UNSUPPORTED, - "TRITONSERVER_METRIC_KIND_COUNTER does not support Observe"); - } - case TRITONSERVER_METRIC_KIND_GAUGE: { - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_UNSUPPORTED, - "TRITONSERVER_METRIC_KIND_GAUGE does not support Observe"); - } case TRITONSERVER_METRIC_KIND_HISTOGRAM: { auto histogram_ptr = reinterpret_cast(metric_); histogram_ptr->Observe(value); diff --git a/src/metric_family.h b/src/metric_family.h index ad08f81c3..1062541c5 100644 --- a/src/metric_family.h +++ b/src/metric_family.h @@ -28,6 +28,7 @@ #ifdef TRITON_ENABLE_METRICS +#include #include #include #include @@ -38,6 +39,30 @@ namespace triton { namespace core { +// +// TritonServerMetricArgs +// +// Implementation for TRITONSERVER_MetricArgs. +// +class TritonServerMetricArgs { + public: + TritonServerMetricArgs() = default; + + void* SetHistogramArgs(const double* buckets, uint64_t bucket_count) + { + kind_ = TRITONSERVER_MetricKind::TRITONSERVER_METRIC_KIND_HISTOGRAM; + buckets_.resize(bucket_count); + std::memcpy(buckets_.data(), buckets, sizeof(double) * bucket_count); + return nullptr; + } + + const std::vector& buckets() const { return buckets_; } + + private: + TRITONSERVER_MetricKind kind_; + std::vector buckets_; +}; + // // Implementation for TRITONSERVER_MetricFamily. // @@ -53,7 +78,7 @@ class MetricFamily { void* Add( std::map label_map, Metric* metric, - const std::vector* buckets = nullptr); + const TritonServerMetricArgs* buckets); void Remove(void* prom_metric, Metric* metric); int NumMetrics() @@ -90,7 +115,7 @@ class Metric { Metric( TRITONSERVER_MetricFamily* family, std::vector labels, - const std::vector* buckets = nullptr); + const TritonServerMetricArgs* args); ~Metric(); MetricFamily* Family() const { return family_; } diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index 9a6fc33ea..af32b6f8e 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -242,7 +242,7 @@ HistogramAPIHelper(TRITONSERVER_Metric* metric) double sum = 0.0; for (auto datum : data) { FAIL_TEST_IF_ERR( - TRITONSERVER_MetricObserve(metric, datum), "observe metric value"); + TRITONSERVER_MetricSet(metric, datum), "observe metric value"); sum += datum; } @@ -406,20 +406,29 @@ TEST_F(MetricsApiTest, TestHistogramEndToEnd) // Create metric TRITONSERVER_Metric* metric; + // Create labels std::vector labels; labels.emplace_back(TRITONSERVER_ParameterNew( "example1", TRITONSERVER_PARAMETER_STRING, "histogram_label1")); labels.emplace_back(TRITONSERVER_ParameterNew( "example2", TRITONSERVER_PARAMETER_STRING, "histogram_label2")); + // Create metric args std::vector buckets = {0.1, 1.0, 2.5, 5.0, 10.0}; + TRITONSERVER_MetricArgs* args; FAIL_TEST_IF_ERR( - TRITONSERVER_MetricNew( - &metric, family, labels.data(), labels.size(), - reinterpret_cast(&buckets)), + TRITONSERVER_MetricArgsNew(&args), "Creating new metric args"); + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricArgsSetHistogram(args, buckets.data(), buckets.size()), + "setting histogram metric args"); + + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricNewWithArgs( + &metric, family, labels.data(), labels.size(), args), "Creating new metric"); for (const auto label : labels) { TRITONSERVER_ParameterDelete(const_cast(label)); } + FAIL_TEST_IF_ERR(TRITONSERVER_MetricArgsDelete(args), "delete metric args"); // Run through metric APIs and assert correctness HistogramAPIHelper(metric); @@ -436,7 +445,6 @@ TEST_F(MetricsApiTest, TestHistogramEndToEnd) ASSERT_EQ(NumMetricMatches(server_, description), 0); } - // Test that a duplicate metric family can't be added // with a conflicting type/kind TEST_F(MetricsApiTest, TestDupeMetricFamilyDiffKind) diff --git a/src/tritonserver.cc b/src/tritonserver.cc index ac36c2445..06714d522 100644 --- a/src/tritonserver.cc +++ b/src/tritonserver.cc @@ -3365,14 +3365,87 @@ TRITONSERVER_MetricFamilyDelete(TRITONSERVER_MetricFamily* family) #endif // TRITON_ENABLE_METRICS } +TRITONSERVER_Error* +TRITONSERVER_GetMetricFamilyKind( + TRITONSERVER_MetricFamily* family, TRITONSERVER_MetricKind* kind) +{ +#ifdef TRITON_ENABLE_METRICS + *kind = reinterpret_cast(family)->Kind(); + return nullptr; // Success +#else + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + +TRITONSERVER_Error* +TRITONSERVER_MetricArgsNew(TRITONSERVER_MetricArgs** args) +{ +#ifdef TRITON_ENABLE_METRICS + tc::TritonServerMetricArgs* largs = new tc::TritonServerMetricArgs(); + *args = reinterpret_cast(largs); + return nullptr; // Success +#else + *metrics = nullptr; + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + +TRITONSERVER_Error* +TRITONSERVER_MetricArgsSetHistogram( + TRITONSERVER_MetricArgs* args, const double* buckets, + const uint64_t buckets_count) +{ +#ifdef TRITON_ENABLE_METRICS + auto largs = reinterpret_cast(args); + largs->SetHistogramArgs(buckets, buckets_count); + return nullptr; // Success +#else + *metrics = nullptr; + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + +TRITONSERVER_Error* +TRITONSERVER_MetricArgsDelete(TRITONSERVER_MetricArgs* args) +{ +#ifdef TRITON_ENABLE_METRICS + auto largs = reinterpret_cast(args); + delete largs; + return nullptr; // success +#else + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + // // TRITONSERVER_Metric // TRITONSERVER_Error* TRITONSERVER_MetricNew( + TRITONSERVER_Metric** metric, TRITONSERVER_MetricFamily* family, + const TRITONSERVER_Parameter** labels, const uint64_t label_count) +{ +#ifdef TRITON_ENABLE_METRICS + return TRITONSERVER_MetricNewWithArgs( + metric, family, labels, label_count, nullptr); +#else + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + +// +// TRITONSERVER_MetricGeneric +// +TRITONSERVER_Error* +TRITONSERVER_MetricNewWithArgs( TRITONSERVER_Metric** metric, TRITONSERVER_MetricFamily* family, const TRITONSERVER_Parameter** labels, const uint64_t label_count, - const void* buckets) + const TRITONSERVER_MetricArgs* args) { #ifdef TRITON_ENABLE_METRICS std::vector labels_vec; @@ -3384,7 +3457,7 @@ TRITONSERVER_MetricNew( try { *metric = reinterpret_cast(new tc::Metric( family, labels_vec, - reinterpret_cast*>(buckets))); + reinterpret_cast(args))); } catch (std::invalid_argument const& ex) { // Catch invalid kinds passed to constructor @@ -3452,17 +3525,6 @@ TRITONSERVER_MetricSet(TRITONSERVER_Metric* metric, double value) #endif // TRITON_ENABLE_METRICS } -TRITONSERVER_Error* -TRITONSERVER_MetricObserve(TRITONSERVER_Metric* metric, double value) -{ -#ifdef TRITON_ENABLE_METRICS - return reinterpret_cast(metric)->Observe(value); -#else - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); -#endif // TRITON_ENABLE_METRICS -} - TRITONSERVER_Error* TRITONSERVER_MetricCollect(TRITONSERVER_Metric* metric, void* value) { diff --git a/src/tritonserver_stub.cc b/src/tritonserver_stub.cc index 302e56f64..f6fbe6933 100644 --- a/src/tritonserver_stub.cc +++ b/src/tritonserver_stub.cc @@ -1081,27 +1081,27 @@ TRITONSERVER_MetricNew() } TRITONAPI_DECLSPEC void -TRITONSERVER_MetricDelete() +TRITONSERVER_MetricNewWithArgs() { } TRITONAPI_DECLSPEC void -TRITONSERVER_MetricValue() +TRITONSERVER_MetricDelete() { } TRITONAPI_DECLSPEC void -TRITONSERVER_MetricIncrement() +TRITONSERVER_MetricValue() { } TRITONAPI_DECLSPEC void -TRITONSERVER_MetricSet() +TRITONSERVER_MetricIncrement() { } TRITONAPI_DECLSPEC void -TRITONSERVER_MetricObserve() +TRITONSERVER_MetricSet() { } From 667858eaada0171f7cf03ea7efcfde6b5e983dcb Mon Sep 17 00:00:00 2001 From: Yingge He Date: Tue, 13 Aug 2024 06:29:41 -0700 Subject: [PATCH 05/10] Remove TRITONSERVER_MetricCollect API --- include/triton/core/tritonserver.h | 13 ------ src/metric_family.cc | 40 +++---------------- src/metric_family.h | 7 ++-- src/test/metrics_api_test.cc | 64 +++++++++++++++++++++++------- src/tritonserver.cc | 12 ------ 5 files changed, 57 insertions(+), 79 deletions(-) diff --git a/include/triton/core/tritonserver.h b/include/triton/core/tritonserver.h index 78f13bf85..5831f8827 100644 --- a/include/triton/core/tritonserver.h +++ b/include/triton/core/tritonserver.h @@ -2693,7 +2693,6 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricArgsDelete( /// \param family The metric family to add this new metric to. /// \param labels The array of labels to associate with this new metric. /// \param label_count The number of labels. -/// \param buckets Monotonically increasing values representing the /// bucket boundaries. For histogram only. /// \return a TRITONSERVER_Error indicating success or failure. TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricNew( @@ -2769,18 +2768,6 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricIncrement( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricSet( struct TRITONSERVER_Metric* metric, double value); -/// Collect metrics. -/// Supports metrics of kind TRITONSERVER_METRIC_KIND_COUNTER, -/// TRITONSERVER_METRIC_KIND_GAUGE, TRITONSERVER_METRIC_KIND_HISTOGRAM and -/// returns TRITONSERVER_ERROR_UNSUPPORTED for unsupported -/// TRITONSERVER_MetricKind. -/// -/// \param metric The metric object to collect. -/// \param value Returns the current value of the metric object. -/// \return a TRITONSERVER_Error indicating success or failure. -TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricCollect( - struct TRITONSERVER_Metric* metric, void* value); - /// Get the TRITONSERVER_MetricKind of metric of its corresponding family. /// /// \param metric The metric object to query. diff --git a/src/metric_family.cc b/src/metric_family.cc index 0a145b80c..edd00064e 100644 --- a/src/metric_family.cc +++ b/src/metric_family.cc @@ -101,7 +101,11 @@ MetricFamily::Add( case TRITONSERVER_METRIC_KIND_HISTOGRAM: { if (args == nullptr) { throw std::invalid_argument( - "Bucket boundaries not found in Metric constructor args."); + "Bucket boundaries not found in Metric args."); + } + if (args->kind() != TRITONSERVER_METRIC_KIND_HISTOGRAM) { + throw std::invalid_argument( + "Incorrect Metric args kind in histogram Metric constructor."); } auto histogram_family_ptr = reinterpret_cast*>(family_); @@ -370,40 +374,6 @@ Metric::Set(double value) return nullptr; // Success } -TRITONSERVER_Error* -Metric::Collect(prometheus::ClientMetric* value) -{ - if (metric_ == nullptr) { - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_INTERNAL, - "Could not collect metric value. Metric has been invalidated."); - } - - switch (kind_) { - case TRITONSERVER_METRIC_KIND_COUNTER: { - auto counter_ptr = reinterpret_cast(metric_); - *value = counter_ptr->Collect(); - break; - } - case TRITONSERVER_METRIC_KIND_GAUGE: { - auto gauge_ptr = reinterpret_cast(metric_); - *value = gauge_ptr->Collect(); - break; - } - case TRITONSERVER_METRIC_KIND_HISTOGRAM: { - auto histogram_ptr = reinterpret_cast(metric_); - *value = histogram_ptr->Collect(); - break; - } - default: - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_UNSUPPORTED, - "Unsupported TRITONSERVER_MetricKind"); - } - - return nullptr; // Success -} - }} // namespace triton::core #endif // TRITON_ENABLE_METRICS diff --git a/src/metric_family.h b/src/metric_family.h index 1062541c5..3953d6f53 100644 --- a/src/metric_family.h +++ b/src/metric_family.h @@ -50,12 +50,12 @@ class TritonServerMetricArgs { void* SetHistogramArgs(const double* buckets, uint64_t bucket_count) { - kind_ = TRITONSERVER_MetricKind::TRITONSERVER_METRIC_KIND_HISTOGRAM; + kind_ = TRITONSERVER_METRIC_KIND_HISTOGRAM; buckets_.resize(bucket_count); std::memcpy(buckets_.data(), buckets, sizeof(double) * bucket_count); return nullptr; } - + TRITONSERVER_MetricKind kind() const { return kind_; } const std::vector& buckets() const { return buckets_; } private: @@ -78,7 +78,7 @@ class MetricFamily { void* Add( std::map label_map, Metric* metric, - const TritonServerMetricArgs* buckets); + const TritonServerMetricArgs* args); void Remove(void* prom_metric, Metric* metric); int NumMetrics() @@ -125,7 +125,6 @@ class Metric { TRITONSERVER_Error* Increment(double value); TRITONSERVER_Error* Set(double value); TRITONSERVER_Error* Observe(double value); - TRITONSERVER_Error* Collect(prometheus::ClientMetric* value); // If a MetricFamily is deleted before its dependent Metric, we want to // invalidate the references so we don't access invalid memory. diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index af32b6f8e..7a15f17d3 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -234,29 +234,61 @@ MetricAPIHelper(TRITONSERVER_Metric* metric, TRITONSERVER_MetricKind kind) } void -HistogramAPIHelper(TRITONSERVER_Metric* metric) +HistogramAPIHelper( + TRITONSERVER_Server* server, TRITONSERVER_Metric* metric, + std::vector buckets, std::string metric_name, + std::string labels_str) { - // Observe + // Observe data std::vector data{0.05, 1.5, 6.0}; - std::vector cumulative_counts = {1, 1, 2, 2, 3, 3}; double sum = 0.0; for (auto datum : data) { FAIL_TEST_IF_ERR( TRITONSERVER_MetricSet(metric, datum), "observe metric value"); sum += datum; } + std::vector cumulative_counts = {1, 1, 2, 2, 3, 3}; + ASSERT_EQ(buckets.size() + 1, cumulative_counts.size()); + + // Collect formatted output + std::string metrics_str; + GetMetrics(server, &metrics_str); - // Collect - prometheus::ClientMetric value; - FAIL_TEST_IF_ERR( - TRITONSERVER_MetricCollect(metric, &value), - "query metric value after observe"); - auto hist = value.histogram; - ASSERT_EQ(hist.sample_count, data.size()); - ASSERT_EQ(hist.sample_sum, sum); - ASSERT_EQ(hist.bucket.size(), cumulative_counts.size()); - for (uint64_t i = 0; i < hist.bucket.size(); ++i) { - ASSERT_EQ(hist.bucket[i].cumulative_count, cumulative_counts[i]); + // All strings in this function are generated from ostringstream to make sure + // there is no trailing zeros. For example, std::to_string(7.55) is "7.550000" + // which is inconsistent with prometheus text_serializer output "7.55". + + // custom_histogram_example_count{example1="histogram_label1",example2="histogram_label2"} + // 3 + std::ostringstream count_ss; + count_ss << metric_name << "_count{" << labels_str << "} " << data.size(); + ASSERT_EQ(NumMetricMatches(server, count_ss.str()), 1); + + // custom_histogram_example_sum{example1="histogram_label1",example2="histogram_label2"} + // 7.55 + std::ostringstream sum_ss; + sum_ss << metric_name << "_sum{" << labels_str << "} " << sum; + ASSERT_EQ(NumMetricMatches(server, sum_ss.str()), 1); + + // custom_histogram_example_bucket{example1="histogram_label1",example2="histogram_label2" + std::ostringstream bucket_partial_ss; + bucket_partial_ss << metric_name << "_bucket{" << labels_str; + ASSERT_EQ( + NumMetricMatches(server, bucket_partial_ss.str()), + cumulative_counts.size()); + for (uint64_t i = 0; i < cumulative_counts.size(); ++i) { + // custom_histogram_example_bucket{example1="histogram_label1",example2="histogram_label2",le="0.1"} + // 1 + std::ostringstream le_ss; + if (i < buckets.size()) { + le_ss << "\"" << buckets[i] << "\""; + } else { + le_ss << "\"+Inf\""; + } + std::ostringstream bucket_ss; + bucket_ss << metric_name << "_bucket{" << labels_str + << ",le=" << le_ss.str() << "} " << cumulative_counts[i]; + ASSERT_EQ(NumMetricMatches(server, bucket_ss.str()), 1); } } @@ -431,7 +463,9 @@ TEST_F(MetricsApiTest, TestHistogramEndToEnd) FAIL_TEST_IF_ERR(TRITONSERVER_MetricArgsDelete(args), "delete metric args"); // Run through metric APIs and assert correctness - HistogramAPIHelper(metric); + std::string labels_str = + "example1=\"histogram_label1\",example2=\"histogram_label2\""; + HistogramAPIHelper(server_, metric, buckets, name, labels_str); // Assert custom metric is reported and found in output ASSERT_EQ(NumMetricMatches(server_, description), 1); diff --git a/src/tritonserver.cc b/src/tritonserver.cc index 06714d522..2faf96e9a 100644 --- a/src/tritonserver.cc +++ b/src/tritonserver.cc @@ -3525,18 +3525,6 @@ TRITONSERVER_MetricSet(TRITONSERVER_Metric* metric, double value) #endif // TRITON_ENABLE_METRICS } -TRITONSERVER_Error* -TRITONSERVER_MetricCollect(TRITONSERVER_Metric* metric, void* value) -{ -#ifdef TRITON_ENABLE_METRICS - return reinterpret_cast(metric)->Collect( - reinterpret_cast(value)); -#else - return TRITONSERVER_ErrorNew( - TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); -#endif // TRITON_ENABLE_METRICS -} - TRITONSERVER_Error* TRITONSERVER_GetMetricKind( TRITONSERVER_Metric* metric, TRITONSERVER_MetricKind* kind) From 60f1e63bb145a64a89212a7731ab0abb2d83a675 Mon Sep 17 00:00:00 2001 From: Yingge He Date: Tue, 13 Aug 2024 12:41:58 -0700 Subject: [PATCH 06/10] Test MetricArgs --- include/triton/core/tritonserver.h | 1 - src/metric_family.cc | 3 +- src/test/metrics_api_test.cc | 61 ++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/include/triton/core/tritonserver.h b/include/triton/core/tritonserver.h index 5831f8827..84f47912b 100644 --- a/include/triton/core/tritonserver.h +++ b/include/triton/core/tritonserver.h @@ -2693,7 +2693,6 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricArgsDelete( /// \param family The metric family to add this new metric to. /// \param labels The array of labels to associate with this new metric. /// \param label_count The number of labels. -/// bucket boundaries. For histogram only. /// \return a TRITONSERVER_Error indicating success or failure. TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricNew( struct TRITONSERVER_Metric** metric, diff --git a/src/metric_family.cc b/src/metric_family.cc index edd00064e..910f7ba71 100644 --- a/src/metric_family.cc +++ b/src/metric_family.cc @@ -104,8 +104,7 @@ MetricFamily::Add( "Bucket boundaries not found in Metric args."); } if (args->kind() != TRITONSERVER_METRIC_KIND_HISTOGRAM) { - throw std::invalid_argument( - "Incorrect Metric args kind in histogram Metric constructor."); + throw std::invalid_argument("Metric args not set to histogram kind."); } auto histogram_family_ptr = reinterpret_cast*>(family_); diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index 7a15f17d3..262650f9a 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -479,6 +479,67 @@ TEST_F(MetricsApiTest, TestHistogramEndToEnd) ASSERT_EQ(NumMetricMatches(server_, description), 0); } +// Test create a histogram metric without creating metric arguments +TEST_F(MetricsApiTest, TestHistogramNoMetricArgs) +{ + // Create metric family + TRITONSERVER_MetricFamily* family; + TRITONSERVER_MetricKind kind = TRITONSERVER_METRIC_KIND_HISTOGRAM; + const char* name = "no_metric_args"; + const char* description = "no metric args description"; + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricFamilyNew(&family, kind, name, description), + "Creating new metric family"); + + // MetricArgs not created + TRITONSERVER_MetricArgs* args = nullptr; + // Create metric + std::vector labels; + TRITONSERVER_Metric* metric = nullptr; + auto err = TRITONSERVER_MetricNewWithArgs( + &metric, family, labels.data(), labels.size(), args); + EXPECT_THAT( + TRITONSERVER_ErrorMessage(err), + HasSubstr("Bucket boundaries not found in Metric args")); + + // Cleanup + FAIL_TEST_IF_ERR(TRITONSERVER_MetricArgsDelete(args), "delete metric args"); + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricFamilyDelete(family), "delete metric family"); +} + +// Test create a histogram metric without setting metric arguments +TEST_F(MetricsApiTest, TestHistogramMetricArgsNotset) +{ + // Create metric family + TRITONSERVER_MetricFamily* family; + TRITONSERVER_MetricKind kind = TRITONSERVER_METRIC_KIND_HISTOGRAM; + const char* name = "metric_args_not_set"; + const char* description = "metric args not set description"; + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricFamilyNew(&family, kind, name, description), + "Creating new metric family"); + + // Create metric args object without setting it + TRITONSERVER_MetricArgs* args; + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricArgsNew(&args), "Creating new metric args"); + + // Create metric + std::vector labels; + TRITONSERVER_Metric* metric = nullptr; + auto err = TRITONSERVER_MetricNewWithArgs( + &metric, family, labels.data(), labels.size(), args); + EXPECT_THAT( + TRITONSERVER_ErrorMessage(err), + HasSubstr("Metric args not set to histogram kind")); + + // Cleanup + FAIL_TEST_IF_ERR(TRITONSERVER_MetricArgsDelete(args), "delete metric args"); + FAIL_TEST_IF_ERR( + TRITONSERVER_MetricFamilyDelete(family), "delete metric family"); +} + // Test that a duplicate metric family can't be added // with a conflicting type/kind TEST_F(MetricsApiTest, TestDupeMetricFamilyDiffKind) From 349daa691d7b1d6a0d18e20da913c106d0f3c4a7 Mon Sep 17 00:00:00 2001 From: Yingge He Date: Fri, 16 Aug 2024 05:36:30 -0700 Subject: [PATCH 07/10] Restore TRITONSERVER_MetricObserve --- include/triton/core/tritonserver.h | 30 +++++++++++++++++--------- src/metric_family.cc | 34 ++++++++++++++++++++++++++++++ src/test/metrics_api_test.cc | 2 +- src/tritonserver.cc | 11 ++++++++++ src/tritonserver_stub.cc | 5 +++++ 5 files changed, 71 insertions(+), 11 deletions(-) diff --git a/include/triton/core/tritonserver.h b/include/triton/core/tritonserver.h index 84f47912b..d9701e890 100644 --- a/include/triton/core/tritonserver.h +++ b/include/triton/core/tritonserver.h @@ -2648,7 +2648,7 @@ TRITONSERVER_MetricFamilyDelete(struct TRITONSERVER_MetricFamily* family); /// Get the TRITONSERVER_MetricKind of the metric family. /// -/// \param metric The metric family object to query. +/// \param family The metric family object to query. /// \param kind Returns the TRITONSERVER_MetricKind of metric. /// \return a TRITONSERVER_Error indicating success or failure. TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* @@ -2664,10 +2664,12 @@ TRITONSERVER_GetMetricFamilyKind( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricArgsNew( struct TRITONSERVER_MetricArgs** args); -/// Set metric args with prometheus histogram metric parameter. +/// Set metric args with histogram metric parameter. /// /// \param args The metric args object to set. -/// \param buckets The array of bucket boundaries. +/// \param buckets The array of bucket boundaries for the expected range of +/// observed values. +/// /// \param buckets_count The number of bucket boundaries. /// \return a TRITONSERVER_Error indicating success or failure. TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* @@ -2732,10 +2734,9 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricDelete( struct TRITONSERVER_Metric* metric); /// Get the current value of a metric object. -/// Supports metrics of kind TRITONSERVER_METRIC_KIND_COUNTER, -/// TRITONSERVER_METRIC_KIND_GAUGE, TRITONSERVER_METRIC_KIND_HISTOGRAM, and -/// returns TRITONSERVER_ERROR_UNSUPPORTED for unsupported -/// TRITONSERVER_MetricKind. +/// Supports metrics of kind TRITONSERVER_METRIC_KIND_COUNTER +/// and TRITONSERVER_METRIC_KIND_GAUGE, and returns +/// TRITONSERVER_ERROR_UNSUPPORTED for unsupported TRITONSERVER_MetricKind. /// /// \param metric The metric object to query. /// \param value Returns the current value of the metric object. @@ -2756,9 +2757,8 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricValue( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricIncrement( struct TRITONSERVER_Metric* metric, double value); -/// Set the current value of metric to value or observe the value to metric. -/// Supports metrics of kind TRITONSERVER_METRIC_KIND_GAUGE and -/// TRITONSERVER_METRIC_KIND_HISTOGRAM. Returns +/// Set the current value of metric to value. +/// Supports metrics of kind TRITONSERVER_METRIC_KIND_GAUGE and returns /// TRITONSERVER_ERROR_UNSUPPORTED for unsupported TRITONSERVER_MetricKind. /// /// \param metric The metric object to update. @@ -2767,6 +2767,16 @@ TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricIncrement( TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricSet( struct TRITONSERVER_Metric* metric, double value); +/// Sample an observation and count it to the appropriate bucket of a metric. +/// Supports metrics of kind TRITONSERVER_METRIC_KIND_HISTOGRAM and returns +/// TRITONSERVER_ERROR_UNSUPPORTED for unsupported TRITONSERVER_MetricKind. +/// +/// \param metric The metric object to update. +/// \param value The amount for metric to sample observation. +/// \return a TRITONSERVER_Error indicating success or failure. +TRITONSERVER_DECLSPEC struct TRITONSERVER_Error* TRITONSERVER_MetricObserve( + struct TRITONSERVER_Metric* metric, double value); + /// Get the TRITONSERVER_MetricKind of metric of its corresponding family. /// /// \param metric The metric object to query. diff --git a/src/metric_family.cc b/src/metric_family.cc index 910f7ba71..13fa23ca3 100644 --- a/src/metric_family.cc +++ b/src/metric_family.cc @@ -359,6 +359,40 @@ Metric::Set(double value) gauge_ptr->Set(value); break; } + case TRITONSERVER_METRIC_KIND_HISTOGRAM: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_HISTOGRAM does not support Set"); + } + default: + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "Unsupported TRITONSERVER_MetricKind"); + } + + return nullptr; // Success +} + +TRITONSERVER_Error* +Metric::Observe(double value) +{ + if (metric_ == nullptr) { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_INTERNAL, + "Could not set metric value. Metric has been invalidated."); + } + + switch (kind_) { + case TRITONSERVER_METRIC_KIND_COUNTER: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_COUNTER does not support Observe"); + } + case TRITONSERVER_METRIC_KIND_GAUGE: { + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, + "TRITONSERVER_METRIC_KIND_GAUGE does not support Observe"); + } case TRITONSERVER_METRIC_KIND_HISTOGRAM: { auto histogram_ptr = reinterpret_cast(metric_); histogram_ptr->Observe(value); diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index 262650f9a..41e4ac274 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -244,7 +244,7 @@ HistogramAPIHelper( double sum = 0.0; for (auto datum : data) { FAIL_TEST_IF_ERR( - TRITONSERVER_MetricSet(metric, datum), "observe metric value"); + TRITONSERVER_MetricObserve(metric, datum), "observe metric value"); sum += datum; } std::vector cumulative_counts = {1, 1, 2, 2, 3, 3}; diff --git a/src/tritonserver.cc b/src/tritonserver.cc index 2faf96e9a..1a8e3be38 100644 --- a/src/tritonserver.cc +++ b/src/tritonserver.cc @@ -3525,6 +3525,17 @@ TRITONSERVER_MetricSet(TRITONSERVER_Metric* metric, double value) #endif // TRITON_ENABLE_METRICS } +TRITONSERVER_Error* +TRITONSERVER_MetricObserve(TRITONSERVER_Metric* metric, double value) +{ +#ifdef TRITON_ENABLE_METRICS + return reinterpret_cast(metric)->Observe(value); +#else + return TRITONSERVER_ErrorNew( + TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); +#endif // TRITON_ENABLE_METRICS +} + TRITONSERVER_Error* TRITONSERVER_GetMetricKind( TRITONSERVER_Metric* metric, TRITONSERVER_MetricKind* kind) diff --git a/src/tritonserver_stub.cc b/src/tritonserver_stub.cc index f6fbe6933..5cdb65c3c 100644 --- a/src/tritonserver_stub.cc +++ b/src/tritonserver_stub.cc @@ -1105,6 +1105,11 @@ TRITONSERVER_MetricSet() { } +TRITONAPI_DECLSPEC void +TRITONSERVER_MetricObserve() +{ +} + TRITONAPI_DECLSPEC void TRITONSERVER_GetMetricKind() { From c3d478ae3db489bb8124bd4e0aa5c92fb11c4757 Mon Sep 17 00:00:00 2001 From: Yingge He Date: Fri, 16 Aug 2024 11:21:46 -0700 Subject: [PATCH 08/10] Fix build error with -DTRITON_ENABLE_METRICS=OFF --- src/tritonserver.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tritonserver.cc b/src/tritonserver.cc index 1a8e3be38..67343a730 100644 --- a/src/tritonserver.cc +++ b/src/tritonserver.cc @@ -3386,7 +3386,7 @@ TRITONSERVER_MetricArgsNew(TRITONSERVER_MetricArgs** args) *args = reinterpret_cast(largs); return nullptr; // Success #else - *metrics = nullptr; + *args = nullptr; return TRITONSERVER_ErrorNew( TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); #endif // TRITON_ENABLE_METRICS @@ -3402,7 +3402,6 @@ TRITONSERVER_MetricArgsSetHistogram( largs->SetHistogramArgs(buckets, buckets_count); return nullptr; // Success #else - *metrics = nullptr; return TRITONSERVER_ErrorNew( TRITONSERVER_ERROR_UNSUPPORTED, "metrics not supported"); #endif // TRITON_ENABLE_METRICS From 484c2be1ce6c4be01eeb25b8040b69a38c28b58c Mon Sep 17 00:00:00 2001 From: Yingge He Date: Fri, 16 Aug 2024 12:31:35 -0700 Subject: [PATCH 09/10] Minor update --- src/metric_family.h | 3 +-- src/test/metrics_api_test.cc | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/metric_family.h b/src/metric_family.h index 3953d6f53..2ea13eeda 100644 --- a/src/metric_family.h +++ b/src/metric_family.h @@ -51,8 +51,7 @@ class TritonServerMetricArgs { void* SetHistogramArgs(const double* buckets, uint64_t bucket_count) { kind_ = TRITONSERVER_METRIC_KIND_HISTOGRAM; - buckets_.resize(bucket_count); - std::memcpy(buckets_.data(), buckets, sizeof(double) * bucket_count); + buckets_ = std::vector(buckets, buckets + bucket_count); return nullptr; } TRITONSERVER_MetricKind kind() const { return kind_; } diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index 41e4ac274..e1261616f 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -233,6 +233,34 @@ MetricAPIHelper(TRITONSERVER_Metric* metric, TRITONSERVER_MetricKind kind) TRITONSERVER_ErrorDelete(err); } +// Calculate the cumulative_counts vector based on data and buckets. +std::vector +GetCumulativeCounts( + const std::vector& data, const std::vector& buckets) +{ + std::vector cumulative_counts(buckets.size() + 1, 0); + for (double datum : data) { + int i = 0; + for (; i < buckets.size(); ++i) { + if (datum <= buckets[i]) { + cumulative_counts[i]++; + break; + } + } + if (i == buckets.size()) { + cumulative_counts[i]++; + } + } + + double cumulative_sum = 0.0; + for (int i = 0; i < cumulative_counts.size(); ++i) { + std::cout << cumulative_counts[i] << std::endl; + cumulative_sum += cumulative_counts[i]; + cumulative_counts[i] = cumulative_sum; + } + return cumulative_counts; +} + void HistogramAPIHelper( TRITONSERVER_Server* server, TRITONSERVER_Metric* metric, @@ -247,8 +275,8 @@ HistogramAPIHelper( TRITONSERVER_MetricObserve(metric, datum), "observe metric value"); sum += datum; } - std::vector cumulative_counts = {1, 1, 2, 2, 3, 3}; - ASSERT_EQ(buckets.size() + 1, cumulative_counts.size()); + std::vector cumulative_counts = + GetCumulativeCounts(data, buckets); // Collect formatted output std::string metrics_str; From d4372af22ae7254e2c1858e9404de5082f4161a5 Mon Sep 17 00:00:00 2001 From: Yingge He Date: Fri, 16 Aug 2024 12:46:51 -0700 Subject: [PATCH 10/10] Simply GetCumulativeCounts --- src/test/metrics_api_test.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/test/metrics_api_test.cc b/src/test/metrics_api_test.cc index e1261616f..ea9d63a9e 100644 --- a/src/test/metrics_api_test.cc +++ b/src/test/metrics_api_test.cc @@ -252,11 +252,8 @@ GetCumulativeCounts( } } - double cumulative_sum = 0.0; - for (int i = 0; i < cumulative_counts.size(); ++i) { - std::cout << cumulative_counts[i] << std::endl; - cumulative_sum += cumulative_counts[i]; - cumulative_counts[i] = cumulative_sum; + for (int i = 1; i < cumulative_counts.size(); ++i) { + cumulative_counts[i] += cumulative_counts[i - 1]; } return cumulative_counts; }