From edb05338a26b4ec347639d77060aefdeed6ba23b Mon Sep 17 00:00:00 2001 From: Yingge He Date: Mon, 12 Aug 2024 19:30:47 -0700 Subject: [PATCH] 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() { }