Skip to content

Commit b1b1d5f

Browse files
committed
Add direct access API for metrics
This adds COUNTER_VALUE, GAUGE_VALUE, and HISTOGRAM_VALUE macros that allow reading metric values directly as native types (int64_t for counters/gauges, HistogramStatistics struct for histograms) without going through JSON serialization. Also made gather_result() public to allow custom metric collection without JSON if needed. Tests added to farm_test.cpp verify the new direct access methods work correctly across all metric types. Signed-off-by: Xiaoxi Chen <xiaoxchen@ebay.com>
1 parent 1a863d3 commit b1b1d5f

File tree

11 files changed

+240
-21
lines changed

11 files changed

+240
-21
lines changed

conanfile.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
class SISLConan(ConanFile):
1111
name = "sisl"
12-
version = "13.1.1"
12+
version = "13.1.2"
1313

1414
homepage = "https://github.com/eBay/sisl"
1515
description = "Library for fast data structures, utilities"

include/sisl/metrics/metrics.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,12 @@ struct NamedHistogram< tstring< elements... > > {
292292
#define COUNTER_DECREMENT_IF_ELSE(group, cond, namea, nameb, ...) \
293293
__VALIDATE_AND_EXECUTE_IF_ELSE(group, NamedCounter, counter_decrement, cond, namea, nameb, __VA_ARGS__)
294294

295+
#define COUNTER_VALUE(group, name) \
296+
((group).m_impl_ptr->counter_get(COUNTER_INDEX(name)))
297+
295298
#define GAUGE_UPDATE(group, name, ...) __VALIDATE_AND_EXECUTE(group, NamedGauge, gauge_update, name, __VA_ARGS__)
299+
#define GAUGE_VALUE(group, name) \
300+
((group).m_impl_ptr->gauge_get(GAUGE_INDEX(name)))
296301
#define GAUGE_UPDATE_IF_ELSE(group, cond, namea, nameb, ...) \
297302
__VALIDATE_AND_EXECUTE_IF_ELSE(group, NamedGauge, gauge_update, cond, namea, nameb, __VA_ARGS__)
298303

@@ -301,6 +306,9 @@ struct NamedHistogram< tstring< elements... > > {
301306
#define HISTOGRAM_OBSERVE_IF_ELSE(group, cond, namea, nameb, ...) \
302307
__VALIDATE_AND_EXECUTE_IF_ELSE(group, NamedHistogram, histogram_observe, cond, namea, nameb, __VA_ARGS__)
303308

309+
#define HISTOGRAM_VALUE(group, name) \
310+
((group).m_impl_ptr->histogram_get(HISTOGRAM_INDEX(name)))
311+
304312
#if 0
305313
#define COUNTER_INCREMENT(group, name, ...) \
306314
{ \

include/sisl/metrics/metrics_atomic.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@ class AtomicMetricsGroup : public MetricsGroupImpl {
9797

9898
void counter_increment(const uint64_t index, const int64_t val = 1) override;
9999
void counter_decrement(const uint64_t index, const int64_t val = 1) override;
100+
int64_t counter_get(const uint64_t index) override;
100101
void histogram_observe(const uint64_t index, const int64_t val) override;
101102
void histogram_observe(const uint64_t index, const int64_t val, const uint64_t count) override;
103+
HistogramStatistics histogram_get(const uint64_t index) override;
102104

103105
[[nodiscard]] group_impl_type_t impl_type() const { return group_impl_type_t::atomic; }
104106

include/sisl/metrics/metrics_group_impl.hpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,15 @@ class HistogramStaticInfo {
261261
const hist_bucket_boundaries_t& m_bkt_boundaries;
262262
};
263263

264+
// Structure to hold histogram statistics
265+
struct HistogramStatistics {
266+
int64_t count{0};
267+
double average{0.0};
268+
double p50{0.0}; // 50th percentile (median)
269+
double p95{0.0}; // 95th percentile
270+
double p99{0.0}; // 99th percentile
271+
};
272+
264273
class HistogramDynamicInfo {
265274
friend class MetricsGroupImpl;
266275

@@ -385,11 +394,14 @@ class MetricsGroupImpl {
385394

386395
virtual void counter_increment(const uint64_t index, const int64_t val = 1) = 0;
387396
virtual void counter_decrement(const uint64_t index, const int64_t val = 1) = 0;
397+
virtual int64_t counter_get(const uint64_t index) = 0;
388398

389399
void gauge_update(const uint64_t index, const int64_t val);
400+
int64_t gauge_get(const uint64_t index) { return m_gauge_values[index].get(); }
390401

391402
virtual void histogram_observe(const uint64_t index, const int64_t val, const uint64_t count) = 0;
392403
virtual void histogram_observe(const uint64_t index, const int64_t val) = 0;
404+
virtual HistogramStatistics histogram_get(const uint64_t index) = 0;
393405

394406
nlohmann::json get_result_in_json(const bool need_latest);
395407
[[nodiscard]] const std::string& get_group_name() const;
@@ -448,7 +460,7 @@ class MetricsGroupImpl {
448460

449461
virtual void on_register() = 0;
450462

451-
protected:
463+
// Public API to gather metrics with callbacks for direct access to counter/gauge/histogram values
452464
virtual void gather_result(const bool need_latest, const counter_gather_cb_t& counter_cb,
453465
const gauge_gather_cb_t& gauge_cb, const histogram_gather_cb_t& histogram_cb) = 0;
454466

include/sisl/metrics/metrics_rcu.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ class WisrBufferMetricsGroup : public MetricsGroupImpl {
3838

3939
void counter_increment(uint64_t index, int64_t val = 1) override;
4040
void counter_decrement(uint64_t index, int64_t val = 1) override;
41+
int64_t counter_get(uint64_t index) override;
4142
void histogram_observe(uint64_t index, int64_t val) override;
4243
void histogram_observe(uint64_t index, int64_t val, uint64_t count) override;
44+
HistogramStatistics histogram_get(uint64_t index) override;
4345

4446
group_impl_type_t impl_type() const { return group_impl_type_t::rcu; }
4547

include/sisl/metrics/metrics_tlocal.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,10 @@ class ThreadBufferMetricsGroup : public MetricsGroupImpl {
8686

8787
void counter_increment(const uint64_t index, const int64_t val = 1) override;
8888
void counter_decrement(const uint64_t index, const int64_t val = 1) override;
89+
int64_t counter_get(const uint64_t index) override;
8990
void histogram_observe(const uint64_t index, const int64_t val) override;
9091
void histogram_observe(const uint64_t index, const int64_t val, const uint64_t count) override;
92+
HistogramStatistics histogram_get(const uint64_t index) override;
9193

9294
static void flush_core_cache();
9395

src/metrics/metrics_atomic.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ void AtomicMetricsGroup::counter_increment(uint64_t index, int64_t val) { m_coun
4949

5050
void AtomicMetricsGroup::counter_decrement(uint64_t index, int64_t val) { m_counter_values[index].decrement(val); }
5151

52+
int64_t AtomicMetricsGroup::counter_get(uint64_t index) { return m_counter_values[index].to_counter_value().get(); }
53+
5254
// If we were to call the method with count parameter and compiler inlines them, binaries linked with libsisl gets
5355
// linker errors. At the same time we also don't want to non-inline this method, since its the most obvious call
5456
// everyone makes and wanted to avoid additional function call in the stack. Hence we are duplicating the function
@@ -60,4 +62,18 @@ void AtomicMetricsGroup::histogram_observe(uint64_t index, int64_t val) {
6062
void AtomicMetricsGroup::histogram_observe(uint64_t index, int64_t val, uint64_t count) {
6163
m_histogram_values[index].observe(val, hist_static_info(index).get_boundaries(), count);
6264
}
65+
66+
HistogramStatistics AtomicMetricsGroup::histogram_get(uint64_t index) {
67+
HistogramStatistics stats;
68+
HistogramValue hvalue = m_histogram_values[index].to_histogram_value();
69+
const auto& boundaries = hist_static_info(index).get_boundaries();
70+
71+
stats.count = hist_dynamic_info(index).count(hvalue);
72+
stats.average = hist_dynamic_info(index).average(hvalue);
73+
stats.p50 = hist_dynamic_info(index).percentile(hvalue, boundaries, 50.0f);
74+
stats.p95 = hist_dynamic_info(index).percentile(hvalue, boundaries, 95.0f);
75+
stats.p99 = hist_dynamic_info(index).percentile(hvalue, boundaries, 99.0f);
76+
77+
return stats;
78+
}
6379
} // namespace sisl

src/metrics/metrics_rcu.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ void WisrBufferMetricsGroup::counter_decrement(uint64_t index, int64_t val) {
3333
m->get_counter(index).decrement(val);
3434
}
3535

36+
int64_t WisrBufferMetricsGroup::counter_get(uint64_t index) {
37+
// Get the latest snapshot and read the specific counter
38+
PerThreadMetrics* tmetrics = m_metrics->now();
39+
return tmetrics->get_counter(index).get();
40+
}
41+
3642
// If we were to call the method with count parameter and compiler inlines them, binaries linked with libsisl gets
3743
// linker errors. At the same time we also don't want to non-inline this method, since its the most obvious call
3844
// everyone makes and wanted to avoid additional function call in the stack. Hence we are duplicating the function
@@ -47,6 +53,22 @@ void WisrBufferMetricsGroup::histogram_observe(uint64_t index, int64_t val, uint
4753
m->get_histogram(index).observe(val, hist_static_info(index).get_boundaries(), count);
4854
}
4955

56+
HistogramStatistics WisrBufferMetricsGroup::histogram_get(uint64_t index) {
57+
// Get the latest snapshot and read the specific histogram
58+
PerThreadMetrics* tmetrics = m_metrics->now();
59+
const HistogramValue& hvalue = tmetrics->get_histogram(index);
60+
const auto& boundaries = hist_static_info(index).get_boundaries();
61+
62+
HistogramStatistics stats;
63+
stats.count = hist_dynamic_info(index).count(hvalue);
64+
stats.average = hist_dynamic_info(index).average(hvalue);
65+
stats.p50 = hist_dynamic_info(index).percentile(hvalue, boundaries, 50.0f);
66+
stats.p95 = hist_dynamic_info(index).percentile(hvalue, boundaries, 95.0f);
67+
stats.p99 = hist_dynamic_info(index).percentile(hvalue, boundaries, 99.0f);
68+
69+
return stats;
70+
}
71+
5072
void WisrBufferMetricsGroup::gather_result(bool need_latest, const counter_gather_cb_t& counter_cb,
5173
const gauge_gather_cb_t& gauge_cb,
5274
const histogram_gather_cb_t& histogram_cb) {

src/metrics/metrics_tlocal.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,18 @@ void ThreadBufferMetricsGroup::counter_decrement(const uint64_t index, const int
150150
m_metrics_buf->get()->get_counter(index).decrement(val);
151151
}
152152

153+
int64_t ThreadBufferMetricsGroup::counter_get(const uint64_t index) {
154+
// Gather the specific counter value from all threads
155+
flush_core_cache();
156+
CounterValue result_counter;
157+
m_metrics_buf->access_all_threads([&](PerThreadMetrics* tmetrics, [[maybe_unused]] bool is_thread_running,
158+
[[maybe_unused]] bool is_last_thread) {
159+
result_counter.merge(tmetrics->get_counter(index));
160+
return true;
161+
});
162+
return result_counter.get();
163+
}
164+
153165
// If we were to call the method with count parameter and compiler inlines them, binaries linked with libsisl gets
154166
// linker errors. At the same time we also don't want to non-inline this method, since its the most obvious call
155167
// everyone makes and wanted to avoid additional function call in the stack. Hence we are duplicating the function
@@ -161,4 +173,26 @@ void ThreadBufferMetricsGroup::histogram_observe(const uint64_t index, const int
161173
void ThreadBufferMetricsGroup::histogram_observe(const uint64_t index, const int64_t val, const uint64_t count) {
162174
m_metrics_buf->get()->get_histogram(index).observe(val, m_static_info->m_histograms[index].get_boundaries(), count);
163175
}
176+
177+
HistogramStatistics ThreadBufferMetricsGroup::histogram_get(const uint64_t index) {
178+
// Gather the specific histogram value from all threads
179+
flush_core_cache();
180+
HistogramValue result_histogram;
181+
const auto& boundaries = hist_static_info(index).get_boundaries();
182+
183+
m_metrics_buf->access_all_threads([&](PerThreadMetrics* tmetrics, [[maybe_unused]] bool is_thread_running,
184+
[[maybe_unused]] bool is_last_thread) {
185+
result_histogram.merge(tmetrics->get_histogram(index), boundaries);
186+
return true;
187+
});
188+
189+
HistogramStatistics stats;
190+
stats.count = hist_dynamic_info(index).count(result_histogram);
191+
stats.average = hist_dynamic_info(index).average(result_histogram);
192+
stats.p50 = hist_dynamic_info(index).percentile(result_histogram, boundaries, 50.0f);
193+
stats.p95 = hist_dynamic_info(index).percentile(result_histogram, boundaries, 95.0f);
194+
stats.p99 = hist_dynamic_info(index).percentile(result_histogram, boundaries, 99.0f);
195+
196+
return stats;
197+
}
164198
} // namespace sisl

src/metrics/tests/farm_test.cpp

Lines changed: 94 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,33 +34,47 @@ SISL_LOGGING_INIT(vmod_metrics_framework)
3434

3535
using namespace sisl;
3636

37+
class Group1Metrics : public MetricsGroup {
38+
public:
39+
explicit Group1Metrics(const char* inst_name)
40+
: MetricsGroup("Group1", inst_name, group_impl_type_t::thread_buf_signal) {
41+
REGISTER_COUNTER(counter1, "Counter1");
42+
REGISTER_COUNTER(counter2, "Counter2");
43+
REGISTER_COUNTER(counter3, "Counter3");
44+
register_me_to_farm();
45+
}
46+
~Group1Metrics() { deregister_me_from_farm(); }
47+
};
48+
49+
class Group2Metrics : public MetricsGroup {
50+
public:
51+
explicit Group2Metrics(const char* inst_name)
52+
: MetricsGroup("Group2", inst_name, group_impl_type_t::thread_buf_signal) {
53+
REGISTER_GAUGE(gauge1, "Gauge1");
54+
REGISTER_GAUGE(gauge2, "Gauge2");
55+
register_me_to_farm();
56+
}
57+
~Group2Metrics() { deregister_me_from_farm(); }
58+
};
59+
3760
void userA() {
38-
auto mgroup = std::make_shared< ThreadBufferMetricsGroup >("Group1", "Instance1");
39-
mgroup->register_counter("counter1", "Counter1");
40-
mgroup->register_counter("counter2", "Counter2");
41-
mgroup->register_counter("counter3", "Counter3");
42-
MetricsFarm::getInstance().register_metrics_group(mgroup);
43-
44-
mgroup->counter_increment(0);
45-
mgroup->counter_increment(2, 4);
61+
Group1Metrics metrics("Instance1");
62+
63+
COUNTER_INCREMENT(metrics, counter1, 1);
64+
COUNTER_INCREMENT(metrics, counter3, 4);
4665
std::this_thread::sleep_for(std::chrono::seconds(3));
47-
mgroup->counter_increment(1);
66+
COUNTER_INCREMENT(metrics, counter2, 1);
4867
std::this_thread::sleep_for(std::chrono::seconds(4));
49-
MetricsFarm::getInstance().deregister_metrics_group(mgroup);
5068
}
5169

5270
void userB() {
53-
auto mgroup = std::make_shared< ThreadBufferMetricsGroup >("Group2", "Instance1");
54-
mgroup->register_gauge("gauge1", "Gauge1");
55-
mgroup->register_gauge("gauge2", "Gauge2");
56-
MetricsFarm::getInstance().register_metrics_group(mgroup);
71+
Group2Metrics metrics("Instance1");
5772

58-
mgroup->gauge_update(0, 5);
73+
GAUGE_UPDATE(metrics, gauge1, 5);
5974
std::this_thread::sleep_for(std::chrono::seconds(3));
60-
mgroup->gauge_update(1, 2);
61-
mgroup->gauge_update(0, 3);
75+
GAUGE_UPDATE(metrics, gauge2, 2);
76+
GAUGE_UPDATE(metrics, gauge1, 3);
6277
std::this_thread::sleep_for(std::chrono::seconds(4));
63-
MetricsFarm::getInstance().deregister_metrics_group(mgroup);
6478
}
6579

6680
// clang-format off
@@ -134,7 +148,7 @@ void gather() {
134148
}
135149
}
136150

137-
TEST(farmTest, gather) {
151+
TEST(FarmTest, gather) {
138152
std::thread th1(userA);
139153
std::thread th2(userB);
140154
std::thread th3(gather);
@@ -144,6 +158,67 @@ TEST(farmTest, gather) {
144158
th3.join();
145159
}
146160

161+
// Helper class for DirectAccess tests
162+
class TestMetrics : public MetricsGroup {
163+
public:
164+
explicit TestMetrics(const char* inst_name, group_impl_type_t type = group_impl_type_t::rcu)
165+
: MetricsGroup("TestGroup", inst_name, type) {
166+
REGISTER_COUNTER(test_counter, "Test counter");
167+
REGISTER_GAUGE(test_gauge, "Test gauge");
168+
REGISTER_HISTOGRAM(test_histogram, "Test histogram");
169+
register_me_to_farm();
170+
}
171+
~TestMetrics() { deregister_me_from_farm(); }
172+
};
173+
174+
// Parameterized test fixture for DirectAccess tests
175+
class DirectAccessTest : public ::testing::TestWithParam< group_impl_type_t > {};
176+
177+
// Test direct access to counter, gauge, and histogram values
178+
TEST_P(DirectAccessTest, allMetricTypes) {
179+
group_impl_type_t impl_type = GetParam();
180+
TestMetrics metrics("direct_access_test", impl_type);
181+
182+
// Test counter
183+
COUNTER_INCREMENT(metrics, test_counter, 5);
184+
COUNTER_INCREMENT(metrics, test_counter, 10);
185+
COUNTER_INCREMENT(metrics, test_counter, 15);
186+
int64_t counter_value = COUNTER_VALUE(metrics, test_counter);
187+
EXPECT_EQ(counter_value, 30);
188+
189+
// Test gauge
190+
GAUGE_UPDATE(metrics, test_gauge, 100);
191+
EXPECT_EQ(GAUGE_VALUE(metrics, test_gauge), 100);
192+
GAUGE_UPDATE(metrics, test_gauge, 250);
193+
EXPECT_EQ(GAUGE_VALUE(metrics, test_gauge), 250);
194+
195+
// Test histogram
196+
HISTOGRAM_OBSERVE(metrics, test_histogram, 10);
197+
HISTOGRAM_OBSERVE(metrics, test_histogram, 20);
198+
HISTOGRAM_OBSERVE(metrics, test_histogram, 30);
199+
HISTOGRAM_OBSERVE(metrics, test_histogram, 40);
200+
HISTOGRAM_OBSERVE(metrics, test_histogram, 50);
201+
202+
sisl::HistogramStatistics stats = HISTOGRAM_VALUE(metrics, test_histogram);
203+
EXPECT_EQ(stats.count, 5);
204+
EXPECT_DOUBLE_EQ(stats.average, 30.0);
205+
EXPECT_GT(stats.p50, 0.0);
206+
EXPECT_GT(stats.p95, 0.0);
207+
EXPECT_GT(stats.p99, 0.0);
208+
}
209+
210+
INSTANTIATE_TEST_SUITE_P(AllImplementations, DirectAccessTest,
211+
::testing::Values(group_impl_type_t::rcu, group_impl_type_t::atomic,
212+
group_impl_type_t::thread_buf_signal),
213+
[](const ::testing::TestParamInfo< group_impl_type_t >& info) {
214+
switch (info.param) {
215+
case group_impl_type_t::rcu: return "RCU";
216+
case group_impl_type_t::atomic: return "Atomic";
217+
case group_impl_type_t::thread_buf_signal: return "ThreadLocal";
218+
default: return "Unknown";
219+
}
220+
});
221+
147222
int main(int argc, char* argv[]) {
148223
::testing::InitGoogleTest(&argc, argv);
149224
return RUN_ALL_TESTS();

0 commit comments

Comments
 (0)