From 17a2b7ea3011d0b6801cfd38b1fe4c11e3e7aaf3 Mon Sep 17 00:00:00 2001 From: Buqian Zheng Date: Mon, 15 Jun 2026 10:49:01 +0000 Subject: [PATCH 1/4] feat: add feature report metrics Signed-off-by: Buqian Zheng --- include/common/FeatureReport.h | 79 +++++++++++++++++++++ src/common/FeatureReport.cpp | 126 +++++++++++++++++++++++++++++++++ test/CMakeLists.txt | 1 + test/FeatureReportTest.cpp | 73 +++++++++++++++++++ 4 files changed, 279 insertions(+) create mode 100644 include/common/FeatureReport.h create mode 100644 src/common/FeatureReport.cpp create mode 100644 test/FeatureReportTest.cpp diff --git a/include/common/FeatureReport.h b/include/common/FeatureReport.h new file mode 100644 index 0000000..0edbd4a --- /dev/null +++ b/include/common/FeatureReport.h @@ -0,0 +1,79 @@ +// Copyright (C) 2019-2020 Zilliz. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under the License +// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions and limitations under the License. + +#pragma once + +#include + +#include +#include +#include +#include + +namespace milvus::monitor::feature_report { + +constexpr std::string_view kHybridSearch = "hybrid_search"; +constexpr std::string_view kPartitionKey = "partition_key"; +constexpr std::string_view kDynamicField = "dynamic_field"; +constexpr std::string_view kBM25Function = "bm25_function"; +constexpr std::string_view kResourceGroup = "resource_group"; +constexpr std::string_view kBulkImport = "bulk_import"; + +class alignas(64) FeatureReporter { + public: + // Prefer the predeclared static reporters below on hot paths. + explicit FeatureReporter(std::string_view name); + + FeatureReporter(const FeatureReporter&) = delete; + FeatureReporter& + operator=(const FeatureReporter&) = delete; + + std::string_view + Name() const noexcept; + + bool + Record(); + + bool + RecordAtForTest(std::chrono::steady_clock::time_point now); + + void + ResetForTest(); + + private: + bool + recordAt(std::chrono::steady_clock::time_point now); + + private: + std::atomic next_allowed_nanos_{0}; + std::string name_; + prometheus::Counter& counter_; +}; + +FeatureReporter& +HybridSearch(); + +FeatureReporter& +PartitionKey(); + +FeatureReporter& +DynamicField(); + +FeatureReporter& +BM25Function(); + +FeatureReporter& +ResourceGroup(); + +FeatureReporter& +BulkImport(); + +} // namespace milvus::monitor::feature_report diff --git a/src/common/FeatureReport.cpp b/src/common/FeatureReport.cpp new file mode 100644 index 0000000..4a13793 --- /dev/null +++ b/src/common/FeatureReport.cpp @@ -0,0 +1,126 @@ +// Copyright (C) 2019-2020 Zilliz. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under the License +// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions and limitations under the License. + +#include "common/FeatureReport.h" + +#include + +#include + +#include "common/PrometheusClient.h" + +namespace milvus::monitor::feature_report { +namespace { + +constexpr std::string_view kSourceCpp = "cpp"; +constexpr auto kReportInterval = std::chrono::hours(1); + +prometheus::Family& +FeatureReportFamily() { + static auto& family = prometheus::BuildCounter() + .Name("milvus_feature_report_total") + .Help("Count of throttled feature reports.") + .Register(milvus::monitor::getPrometheusClient().GetRegistry()); + return family; +} + +prometheus::Counter& +CounterForFeature(std::string_view feature) { + return FeatureReportFamily().Add({{"feature", std::string(feature)}, {"source", std::string(kSourceCpp)}}); +} + +int64_t +ToNanos(std::chrono::steady_clock::time_point time_point) { + return std::chrono::duration_cast(time_point.time_since_epoch()).count(); +} + +} // namespace + +FeatureReporter::FeatureReporter(std::string_view name) : name_(name), counter_(CounterForFeature(name_)) { +} + +std::string_view +FeatureReporter::Name() const noexcept { + return std::string_view(name_); +} + +bool +FeatureReporter::Record() { + return recordAt(std::chrono::steady_clock::now()); +} + +bool +FeatureReporter::RecordAtForTest(std::chrono::steady_clock::time_point now) { + return recordAt(now); +} + +void +FeatureReporter::ResetForTest() { + next_allowed_nanos_.store(0, std::memory_order_release); +} + +bool +FeatureReporter::recordAt(std::chrono::steady_clock::time_point now) { + if (name_.empty()) { + return false; + } + + const auto now_nanos = ToNanos(now); + const auto next_nanos = ToNanos(now + kReportInterval); + + auto old = next_allowed_nanos_.load(std::memory_order_acquire); + while (now_nanos >= old) { + if (next_allowed_nanos_.compare_exchange_weak(old, next_nanos, std::memory_order_acq_rel, + std::memory_order_acquire)) { + counter_.Increment(); + return true; + } + } + return false; +} + +FeatureReporter& +HybridSearch() { + static FeatureReporter reporter{kHybridSearch}; + return reporter; +} + +FeatureReporter& +PartitionKey() { + static FeatureReporter reporter{kPartitionKey}; + return reporter; +} + +FeatureReporter& +DynamicField() { + static FeatureReporter reporter{kDynamicField}; + return reporter; +} + +FeatureReporter& +BM25Function() { + static FeatureReporter reporter{kBM25Function}; + return reporter; +} + +FeatureReporter& +ResourceGroup() { + static FeatureReporter reporter{kResourceGroup}; + return reporter; +} + +FeatureReporter& +BulkImport() { + static FeatureReporter reporter{kBulkImport}; + return reporter; +} + +} // namespace milvus::monitor::feature_report diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d23b993..bab66a8 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -16,6 +16,7 @@ find_package(Threads REQUIRED) set(ALL_TEST_FILES init_gtest.cpp + FeatureReportTest.cpp TracerTest.cpp StreamTest.cpp ) diff --git a/test/FeatureReportTest.cpp b/test/FeatureReportTest.cpp new file mode 100644 index 0000000..47fbc76 --- /dev/null +++ b/test/FeatureReportTest.cpp @@ -0,0 +1,73 @@ +// Copyright (C) 2019-2020 Zilliz. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under the License +// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions and limitations under the License. + +#include + +#include +#include +#include +#include +#include + +#include "common/FeatureReport.h" +#include "common/PrometheusClient.h" + +namespace milvus::monitor::feature_report { +namespace { + +bool +MetricsContain(const std::string& metrics, const std::string& feature, const std::string& value) { + const auto expected = "milvus_feature_report_total{feature=\"" + feature + "\",source=\"cpp\"} " + value; + return metrics.find(expected) != std::string::npos; +} + +TEST(FeatureReportTest, Throttle) { + FeatureReporter reporter{"feature_report_test_throttle"}; + const auto now = std::chrono::steady_clock::time_point(std::chrono::seconds(100)); + + ASSERT_TRUE(reporter.RecordAtForTest(now)); + ASSERT_FALSE(reporter.RecordAtForTest(now + std::chrono::minutes(1))); + ASSERT_TRUE(reporter.RecordAtForTest(now + std::chrono::hours(1))); + + ASSERT_TRUE(MetricsContain(getPrometheusClient().GetMetrics(), std::string(reporter.Name()), "2")); +} + +TEST(FeatureReportTest, ConcurrentCalls) { + FeatureReporter reporter{"feature_report_test_concurrent"}; + const auto now = std::chrono::steady_clock::time_point(std::chrono::seconds(200)); + constexpr int kThreads = 32; + + std::atomic reported{0}; + std::vector threads; + threads.reserve(kThreads); + for (int i = 0; i < kThreads; ++i) { + threads.emplace_back([&] { + if (reporter.RecordAtForTest(now)) { + reported.fetch_add(1, std::memory_order_relaxed); + } + }); + } + for (auto& thread : threads) { + thread.join(); + } + + ASSERT_EQ(reported.load(std::memory_order_relaxed), 1); + ASSERT_TRUE(MetricsContain(getPrometheusClient().GetMetrics(), std::string(reporter.Name()), "1")); +} + +TEST(FeatureReportTest, PredeclaredReporter) { + HybridSearch().ResetForTest(); + ASSERT_EQ(HybridSearch().Name(), kHybridSearch); + ASSERT_TRUE(HybridSearch().RecordAtForTest(std::chrono::steady_clock::time_point(std::chrono::seconds(300)))); +} + +} // namespace +} // namespace milvus::monitor::feature_report From babb8d14da070bc6ed25df8fd15be46070e5a014 Mon Sep 17 00:00:00 2001 From: Buqian Zheng Date: Mon, 15 Jun 2026 13:08:52 +0000 Subject: [PATCH 2/4] enhance: add feature reporter macro Signed-off-by: Buqian Zheng --- include/common/FeatureReport.h | 33 ++++++++++------------------ src/common/FeatureReport.cpp | 40 ++++++---------------------------- 2 files changed, 19 insertions(+), 54 deletions(-) diff --git a/include/common/FeatureReport.h b/include/common/FeatureReport.h index 0edbd4a..65dd4ec 100644 --- a/include/common/FeatureReport.h +++ b/include/common/FeatureReport.h @@ -20,12 +20,13 @@ namespace milvus::monitor::feature_report { -constexpr std::string_view kHybridSearch = "hybrid_search"; -constexpr std::string_view kPartitionKey = "partition_key"; -constexpr std::string_view kDynamicField = "dynamic_field"; -constexpr std::string_view kBM25Function = "bm25_function"; -constexpr std::string_view kResourceGroup = "resource_group"; -constexpr std::string_view kBulkImport = "bulk_import"; +#define MILVUS_FEATURE_REPORTERS(FEATURE) \ + FEATURE(HybridSearch, "hybrid_search") \ + FEATURE(PartitionKey, "partition_key") \ + FEATURE(DynamicField, "dynamic_field") \ + FEATURE(BM25Function, "bm25_function") \ + FEATURE(ResourceGroup, "resource_group") \ + FEATURE(BulkImport, "bulk_import") class alignas(64) FeatureReporter { public: @@ -58,22 +59,12 @@ class alignas(64) FeatureReporter { prometheus::Counter& counter_; }; -FeatureReporter& -HybridSearch(); +#define DECLARE_FEATURE_REPORTER(name, label) \ + constexpr std::string_view k##name = label; \ + FeatureReporter& name(); -FeatureReporter& -PartitionKey(); +MILVUS_FEATURE_REPORTERS(DECLARE_FEATURE_REPORTER) -FeatureReporter& -DynamicField(); - -FeatureReporter& -BM25Function(); - -FeatureReporter& -ResourceGroup(); - -FeatureReporter& -BulkImport(); +#undef DECLARE_FEATURE_REPORTER } // namespace milvus::monitor::feature_report diff --git a/src/common/FeatureReport.cpp b/src/common/FeatureReport.cpp index 4a13793..63cce4e 100644 --- a/src/common/FeatureReport.cpp +++ b/src/common/FeatureReport.cpp @@ -87,40 +87,14 @@ FeatureReporter::recordAt(std::chrono::steady_clock::time_point now) { return false; } -FeatureReporter& -HybridSearch() { - static FeatureReporter reporter{kHybridSearch}; - return reporter; -} - -FeatureReporter& -PartitionKey() { - static FeatureReporter reporter{kPartitionKey}; - return reporter; -} - -FeatureReporter& -DynamicField() { - static FeatureReporter reporter{kDynamicField}; - return reporter; -} +#define DEFINE_FEATURE_REPORTER(name, label) \ + FeatureReporter& name() { \ + static FeatureReporter reporter{k##name}; \ + return reporter; \ + } -FeatureReporter& -BM25Function() { - static FeatureReporter reporter{kBM25Function}; - return reporter; -} +MILVUS_FEATURE_REPORTERS(DEFINE_FEATURE_REPORTER) -FeatureReporter& -ResourceGroup() { - static FeatureReporter reporter{kResourceGroup}; - return reporter; -} - -FeatureReporter& -BulkImport() { - static FeatureReporter reporter{kBulkImport}; - return reporter; -} +#undef DEFINE_FEATURE_REPORTER } // namespace milvus::monitor::feature_report From 3fd6842f43501a4a7078379d0cd28fd603f17070 Mon Sep 17 00:00:00 2001 From: Buqian Zheng Date: Tue, 16 Jun 2026 09:50:40 +0000 Subject: [PATCH 3/4] enhance: tighten feature reporter construction Signed-off-by: Buqian Zheng --- include/common/FeatureReport.h | 11 ++++++++--- test/FeatureReportTest.cpp | 14 ++++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/include/common/FeatureReport.h b/include/common/FeatureReport.h index 65dd4ec..f2060c2 100644 --- a/include/common/FeatureReport.h +++ b/include/common/FeatureReport.h @@ -30,9 +30,6 @@ namespace milvus::monitor::feature_report { class alignas(64) FeatureReporter { public: - // Prefer the predeclared static reporters below on hot paths. - explicit FeatureReporter(std::string_view name); - FeatureReporter(const FeatureReporter&) = delete; FeatureReporter& operator=(const FeatureReporter&) = delete; @@ -50,9 +47,17 @@ class alignas(64) FeatureReporter { ResetForTest(); private: + explicit FeatureReporter(std::string_view name); + bool recordAt(std::chrono::steady_clock::time_point now); +#define FRIEND_FEATURE_REPORTER(name, label) friend FeatureReporter& name(); + + MILVUS_FEATURE_REPORTERS(FRIEND_FEATURE_REPORTER) + +#undef FRIEND_FEATURE_REPORTER + private: std::atomic next_allowed_nanos_{0}; std::string name_; diff --git a/test/FeatureReportTest.cpp b/test/FeatureReportTest.cpp index 47fbc76..e6337c7 100644 --- a/test/FeatureReportTest.cpp +++ b/test/FeatureReportTest.cpp @@ -25,12 +25,13 @@ namespace { bool MetricsContain(const std::string& metrics, const std::string& feature, const std::string& value) { - const auto expected = "milvus_feature_report_total{feature=\"" + feature + "\",source=\"cpp\"} " + value; + const auto expected = "milvus_feature_report_total{feature=\"" + feature + "\",source=\"cpp\"} " + value + "\n"; return metrics.find(expected) != std::string::npos; } TEST(FeatureReportTest, Throttle) { - FeatureReporter reporter{"feature_report_test_throttle"}; + auto& reporter = HybridSearch(); + reporter.ResetForTest(); const auto now = std::chrono::steady_clock::time_point(std::chrono::seconds(100)); ASSERT_TRUE(reporter.RecordAtForTest(now)); @@ -41,7 +42,8 @@ TEST(FeatureReportTest, Throttle) { } TEST(FeatureReportTest, ConcurrentCalls) { - FeatureReporter reporter{"feature_report_test_concurrent"}; + auto& reporter = PartitionKey(); + reporter.ResetForTest(); const auto now = std::chrono::steady_clock::time_point(std::chrono::seconds(200)); constexpr int kThreads = 32; @@ -64,9 +66,9 @@ TEST(FeatureReportTest, ConcurrentCalls) { } TEST(FeatureReportTest, PredeclaredReporter) { - HybridSearch().ResetForTest(); - ASSERT_EQ(HybridSearch().Name(), kHybridSearch); - ASSERT_TRUE(HybridSearch().RecordAtForTest(std::chrono::steady_clock::time_point(std::chrono::seconds(300)))); + DynamicField().ResetForTest(); + ASSERT_EQ(DynamicField().Name(), kDynamicField); + ASSERT_TRUE(DynamicField().RecordAtForTest(std::chrono::steady_clock::time_point(std::chrono::seconds(300)))); } } // namespace From d267a9eba37e1b02a6bcba3101601a3dea59b971 Mon Sep 17 00:00:00 2001 From: Buqian Zheng Date: Tue, 16 Jun 2026 11:51:38 +0000 Subject: [PATCH 4/4] enhance: hide feature reporter test hooks Signed-off-by: Buqian Zheng --- include/common/FeatureReport.h | 11 +++--- src/common/FeatureReport.cpp | 9 ++--- test/FeatureReportTest.cpp | 64 ++++++++++++++++++++++++++-------- 3 files changed, 57 insertions(+), 27 deletions(-) diff --git a/include/common/FeatureReport.h b/include/common/FeatureReport.h index f2060c2..59f35ec 100644 --- a/include/common/FeatureReport.h +++ b/include/common/FeatureReport.h @@ -40,18 +40,17 @@ class alignas(64) FeatureReporter { bool Record(); - bool - RecordAtForTest(std::chrono::steady_clock::time_point now); - - void - ResetForTest(); - private: explicit FeatureReporter(std::string_view name); bool recordAt(std::chrono::steady_clock::time_point now); + void + reset(); + + friend struct FeatureReporterTestPeer; + #define FRIEND_FEATURE_REPORTER(name, label) friend FeatureReporter& name(); MILVUS_FEATURE_REPORTERS(FRIEND_FEATURE_REPORTER) diff --git a/src/common/FeatureReport.cpp b/src/common/FeatureReport.cpp index 63cce4e..0cacd1c 100644 --- a/src/common/FeatureReport.cpp +++ b/src/common/FeatureReport.cpp @@ -27,7 +27,7 @@ prometheus::Family& FeatureReportFamily() { static auto& family = prometheus::BuildCounter() .Name("milvus_feature_report_total") - .Help("Count of throttled feature reports.") + .Help("Count of feature reports emitted.") .Register(milvus::monitor::getPrometheusClient().GetRegistry()); return family; } @@ -57,13 +57,8 @@ FeatureReporter::Record() { return recordAt(std::chrono::steady_clock::now()); } -bool -FeatureReporter::RecordAtForTest(std::chrono::steady_clock::time_point now) { - return recordAt(now); -} - void -FeatureReporter::ResetForTest() { +FeatureReporter::reset() { next_allowed_nanos_.store(0, std::memory_order_release); } diff --git a/test/FeatureReportTest.cpp b/test/FeatureReportTest.cpp index e6337c7..fff2072 100644 --- a/test/FeatureReportTest.cpp +++ b/test/FeatureReportTest.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -21,29 +22,63 @@ #include "common/PrometheusClient.h" namespace milvus::monitor::feature_report { + +struct FeatureReporterTestPeer { + static bool + RecordAt(FeatureReporter& reporter, std::chrono::steady_clock::time_point now) { + return reporter.recordAt(now); + } + + static void + Reset(FeatureReporter& reporter) { + reporter.reset(); + } +}; + namespace { -bool -MetricsContain(const std::string& metrics, const std::string& feature, const std::string& value) { - const auto expected = "milvus_feature_report_total{feature=\"" + feature + "\",source=\"cpp\"} " + value + "\n"; - return metrics.find(expected) != std::string::npos; +double +FeatureReportValue(std::string_view feature) { + for (const auto& family : getPrometheusClient().GetRegistry().Collect()) { + if (family.name != "milvus_feature_report_total" || family.type != prometheus::MetricType::Counter) { + continue; + } + + for (const auto& metric : family.metric) { + bool feature_match = false; + bool source_match = false; + for (const auto& label : metric.label) { + if (label.name == "feature" && std::string_view(label.value) == feature) { + feature_match = true; + } else if (label.name == "source" && label.value == "cpp") { + source_match = true; + } + } + if (feature_match && source_match) { + return metric.counter.value; + } + } + } + return 0.0; } TEST(FeatureReportTest, Throttle) { auto& reporter = HybridSearch(); - reporter.ResetForTest(); + FeatureReporterTestPeer::Reset(reporter); + const auto before = FeatureReportValue(reporter.Name()); const auto now = std::chrono::steady_clock::time_point(std::chrono::seconds(100)); - ASSERT_TRUE(reporter.RecordAtForTest(now)); - ASSERT_FALSE(reporter.RecordAtForTest(now + std::chrono::minutes(1))); - ASSERT_TRUE(reporter.RecordAtForTest(now + std::chrono::hours(1))); + ASSERT_TRUE(FeatureReporterTestPeer::RecordAt(reporter, now)); + ASSERT_FALSE(FeatureReporterTestPeer::RecordAt(reporter, now + std::chrono::minutes(1))); + ASSERT_TRUE(FeatureReporterTestPeer::RecordAt(reporter, now + std::chrono::hours(1))); - ASSERT_TRUE(MetricsContain(getPrometheusClient().GetMetrics(), std::string(reporter.Name()), "2")); + ASSERT_DOUBLE_EQ(FeatureReportValue(reporter.Name()) - before, 2.0); } TEST(FeatureReportTest, ConcurrentCalls) { auto& reporter = PartitionKey(); - reporter.ResetForTest(); + FeatureReporterTestPeer::Reset(reporter); + const auto before = FeatureReportValue(reporter.Name()); const auto now = std::chrono::steady_clock::time_point(std::chrono::seconds(200)); constexpr int kThreads = 32; @@ -52,7 +87,7 @@ TEST(FeatureReportTest, ConcurrentCalls) { threads.reserve(kThreads); for (int i = 0; i < kThreads; ++i) { threads.emplace_back([&] { - if (reporter.RecordAtForTest(now)) { + if (FeatureReporterTestPeer::RecordAt(reporter, now)) { reported.fetch_add(1, std::memory_order_relaxed); } }); @@ -62,13 +97,14 @@ TEST(FeatureReportTest, ConcurrentCalls) { } ASSERT_EQ(reported.load(std::memory_order_relaxed), 1); - ASSERT_TRUE(MetricsContain(getPrometheusClient().GetMetrics(), std::string(reporter.Name()), "1")); + ASSERT_DOUBLE_EQ(FeatureReportValue(reporter.Name()) - before, 1.0); } TEST(FeatureReportTest, PredeclaredReporter) { - DynamicField().ResetForTest(); + FeatureReporterTestPeer::Reset(DynamicField()); ASSERT_EQ(DynamicField().Name(), kDynamicField); - ASSERT_TRUE(DynamicField().RecordAtForTest(std::chrono::steady_clock::time_point(std::chrono::seconds(300)))); + ASSERT_TRUE(FeatureReporterTestPeer::RecordAt(DynamicField(), + std::chrono::steady_clock::time_point(std::chrono::seconds(300)))); } } // namespace