Skip to content

Commit ba811b9

Browse files
lingbinaditi-pandit
authored andcommitted
[native] Fix recording of SUM type in PrometheusStatsReporter
For SUM type worker metrics, the corresponding type in `PrometheusStatsReporter` is `prometheus::Gauge`. For these metrics, each time they are recorded (via `RECORD_METRIC_VALUE`), a "delta" is passed in, so `Gauge::Increment()` should be used in `PrometheusStatsReporter` instead of `Gauge::Set()` (which overwrites the old value).
1 parent fd2615e commit ba811b9

File tree

2 files changed

+24
-11
lines changed

2 files changed

+24
-11
lines changed

presto-native-execution/presto_cpp/main/runtime-metrics/PrometheusStatsReporter.cpp

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,17 +169,23 @@ void PrometheusStatsReporter::addMetricValue(const char* key, size_t value)
169169
auto statsInfo = metricIterator->second;
170170
switch (statsInfo.statType) {
171171
case velox::StatType::COUNT: {
172-
auto counter =
172+
auto* counter =
173173
reinterpret_cast<::prometheus::Counter*>(statsInfo.metricPtr);
174-
counter->Increment(value);
175-
} break;
176-
case velox::StatType::SUM:
174+
counter->Increment(static_cast<double>(value));
175+
break;
176+
}
177+
case velox::StatType::SUM: {
178+
auto* gauge = reinterpret_cast<::prometheus::Gauge*>(statsInfo.metricPtr);
179+
gauge->Increment(static_cast<double>(value));
180+
break;
181+
}
177182
case velox::StatType::AVG:
178183
case velox::StatType::RATE: {
179184
// Overrides the existing state.
180-
auto gauge = reinterpret_cast<::prometheus::Gauge*>(statsInfo.metricPtr);
181-
gauge->Set(value);
182-
} break;
185+
auto* gauge = reinterpret_cast<::prometheus::Gauge*>(statsInfo.metricPtr);
186+
gauge->Set(static_cast<double>(value));
187+
break;
188+
}
183189
default:
184190
VELOX_UNSUPPORTED(
185191
"Unsupported metric type {}",

presto-native-execution/presto_cpp/main/runtime-metrics/tests/PrometheusReporterTest.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class PrometheusReporterTest : public testing::Test {
2222
void SetUp() override {
2323
reporter = std::make_shared<PrometheusStatsReporter>(testLabels);
2424
}
25+
2526
void verifySerializedResult(
2627
const std::string& fullSerializedResult,
2728
std::vector<std::string>& expected) {
@@ -32,6 +33,7 @@ class PrometheusReporterTest : public testing::Test {
3233
EXPECT_EQ(line, expected[i++]);
3334
}
3435
}
36+
3537
const std::map<std::string, std::string> testLabels = {
3638
{"cluster", "test_cluster"},
3739
{"worker", "test_worker_pod"}};
@@ -62,24 +64,29 @@ TEST_F(PrometheusReporterTest, testCountAndGauge) {
6264
facebook::velox::StatType::RATE,
6365
reporter->registeredMetricsMap_.find("test.key4")->second.statType);
6466

65-
std::vector<size_t> testData = {10, 11, 15};
67+
std::vector<size_t> testData = {10, 12, 14};
6668
for (auto i : testData) {
6769
reporter->addMetricValue("test.key1", i);
6870
reporter->addMetricValue("test.key2", i + 1000);
71+
reporter->addMetricValue("test.key3", i + 2000);
72+
reporter->addMetricValue("test.key4", i + 3000);
6973
}
74+
7075
// Uses default value of 1 for second parameter.
7176
reporter->addMetricValue("test.key1");
77+
reporter->addMetricValue("test.key3");
78+
7279
auto fullSerializedResult = reporter->fetchMetrics();
7380

7481
std::vector<std::string> expected = {
7582
"# TYPE test_key1 counter",
7683
"test_key1{" + labelsSerialized + "} 37",
7784
"# TYPE test_key2 gauge",
78-
"test_key2{" + labelsSerialized + "} 1015",
85+
"test_key2{" + labelsSerialized + "} 1014",
7986
"# TYPE test_key3 gauge",
80-
"test_key3{" + labelsSerialized + "} 0",
87+
"test_key3{" + labelsSerialized + "} 6037",
8188
"# TYPE test_key4 gauge",
82-
"test_key4{" + labelsSerialized + "} 0"};
89+
"test_key4{" + labelsSerialized + "} 3014"};
8390

8491
verifySerializedResult(fullSerializedResult, expected);
8592
};

0 commit comments

Comments
 (0)