Skip to content

Commit c7062dd

Browse files
wbpcodewbpcode
committed
dym: make the metric id from 1 rather than 0 (#43675)
Commit Message: dym: make the metric id from 1 rather than 0 Additional Description: 0 is common default value for variable. When the `define_*` metrics ABI failed to create a new metric, will keep the id as unset (which basically will be 0). But in previous implementation, the 0 may also be an valid id for the first metric. To avoid confusion, the PR will preserve the 0 as unknown metric. Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: wbpcode <wbphub@gmail.com>
1 parent 88d9981 commit c7062dd

9 files changed

Lines changed: 96 additions & 86 deletions

File tree

‎source/extensions/access_loggers/dynamic_modules/access_log_config.h‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,45 +95,46 @@ class DynamicModuleAccessLogConfig {
9595
Stats::Histogram& histogram_;
9696
};
9797

98+
// We use 1-based IDs for the metrics in the ABI, so we need to convert them to 0-based indices
99+
// for our internal storage. These helper functions do that conversion.
100+
#define ID_TO_INDEX(id) ((id) - 1)
101+
98102
// Methods for adding metrics during configuration.
99103
size_t addCounter(ModuleCounterHandle&& counter) {
100-
size_t id = counters_.size();
101104
counters_.push_back(std::move(counter));
102-
return id;
105+
return counters_.size();
103106
}
104107

105108
size_t addGauge(ModuleGaugeHandle&& gauge) {
106-
size_t id = gauges_.size();
107109
gauges_.push_back(std::move(gauge));
108-
return id;
110+
return gauges_.size();
109111
}
110112

111113
size_t addHistogram(ModuleHistogramHandle&& histogram) {
112-
size_t id = histograms_.size();
113114
histograms_.push_back(std::move(histogram));
114-
return id;
115+
return histograms_.size();
115116
}
116117

117118
// Methods for getting metrics by ID.
118119
OptRef<const ModuleCounterHandle> getCounterById(size_t id) const {
119-
if (id >= counters_.size()) {
120+
if (id == 0 || id > counters_.size()) {
120121
return {};
121122
}
122-
return counters_[id];
123+
return counters_[ID_TO_INDEX(id)];
123124
}
124125

125126
OptRef<const ModuleGaugeHandle> getGaugeById(size_t id) const {
126-
if (id >= gauges_.size()) {
127+
if (id == 0 || id > gauges_.size()) {
127128
return {};
128129
}
129-
return gauges_[id];
130+
return gauges_[ID_TO_INDEX(id)];
130131
}
131132

132133
OptRef<const ModuleHistogramHandle> getHistogramById(size_t id) const {
133-
if (id >= histograms_.size()) {
134+
if (id == 0 || id > histograms_.size()) {
134135
return {};
135136
}
136-
return histograms_[id];
137+
return histograms_[ID_TO_INDEX(id)];
137138
}
138139

139140
// Stats scope for metric creation.

‎source/extensions/filters/http/dynamic_modules/filter_config.h‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -211,84 +211,84 @@ class DynamicModuleHttpFilterConfig
211211
Stats::Histogram::Unit unit_;
212212
};
213213

214+
// We use 1-based IDs for the metrics in the ABI, so we need to convert them to 0-based indices
215+
// for our internal storage. These helper functions do that conversion.
216+
#define ID_TO_INDEX(id) ((id) - 1)
217+
214218
size_t addCounter(ModuleCounterHandle&& counter) {
215-
size_t id = counters_.size();
216219
counters_.push_back(std::move(counter));
217-
return id;
220+
return counters_.size();
218221
}
219222

220223
size_t addCounterVec(ModuleCounterVecHandle&& counter_vec) {
221-
size_t id = counter_vecs_.size();
222224
counter_vecs_.push_back(std::move(counter_vec));
223-
return id;
225+
return counter_vecs_.size();
224226
}
225227

226228
OptRef<const ModuleCounterHandle> getCounterById(size_t id) const {
227-
if (id >= counters_.size()) {
229+
if (id == 0 || id > counters_.size()) {
228230
return {};
229231
}
230-
return counters_[id];
232+
return counters_[ID_TO_INDEX(id)];
231233
}
232234

233235
OptRef<const ModuleCounterVecHandle> getCounterVecById(size_t id) const {
234-
if (id >= counter_vecs_.size()) {
236+
if (id == 0 || id > counter_vecs_.size()) {
235237
return {};
236238
}
237-
return counter_vecs_[id];
239+
return counter_vecs_[ID_TO_INDEX(id)];
238240
}
239241

240242
size_t addGauge(ModuleGaugeHandle&& gauge) {
241-
size_t id = gauges_.size();
242243
gauges_.push_back(std::move(gauge));
243-
return id;
244+
return gauges_.size();
244245
}
245246

246247
size_t addGaugeVec(ModuleGaugeVecHandle&& gauge_vec) {
247-
size_t id = gauge_vecs_.size();
248248
gauge_vecs_.push_back(std::move(gauge_vec));
249-
return id;
249+
return gauge_vecs_.size();
250250
}
251251

252252
OptRef<const ModuleGaugeHandle> getGaugeById(size_t id) const {
253-
if (id >= gauges_.size()) {
253+
if (id == 0 || id > gauges_.size()) {
254254
return {};
255255
}
256-
return gauges_[id];
256+
return gauges_[ID_TO_INDEX(id)];
257257
}
258258

259259
OptRef<const ModuleGaugeVecHandle> getGaugeVecById(size_t id) const {
260-
if (id >= gauge_vecs_.size()) {
260+
if (id == 0 || id > gauge_vecs_.size()) {
261261
return {};
262262
}
263-
return gauge_vecs_[id];
263+
return gauge_vecs_[ID_TO_INDEX(id)];
264264
}
265265

266266
size_t addHistogram(ModuleHistogramHandle&& hist) {
267-
size_t id = hists_.size();
268267
hists_.push_back(std::move(hist));
269-
return id;
268+
return hists_.size();
269+
}
270+
271+
size_t addHistogramVec(ModuleHistogramVecHandle&& hist_vec) {
272+
hist_vecs_.push_back(std::move(hist_vec));
273+
return hist_vecs_.size();
270274
}
271275

272276
OptRef<const ModuleHistogramHandle> getHistogramById(size_t id) const {
273-
if (id >= hists_.size()) {
277+
if (id == 0 || id > hists_.size()) {
274278
return {};
275279
}
276-
return hists_[id];
277-
}
278-
279-
size_t addHistogramVec(ModuleHistogramVecHandle&& hist_vec) {
280-
size_t id = hist_vecs_.size();
281-
hist_vecs_.push_back(std::move(hist_vec));
282-
return id;
280+
return hists_[ID_TO_INDEX(id)];
283281
}
284282

285283
OptRef<const ModuleHistogramVecHandle> getHistogramVecById(size_t id) const {
286-
if (id >= hist_vecs_.size()) {
284+
if (id == 0 || id > hist_vecs_.size()) {
287285
return {};
288286
}
289-
return hist_vecs_[id];
287+
return hist_vecs_[ID_TO_INDEX(id)];
290288
}
291289

290+
#undef ID_TO_INDEX
291+
292292
private:
293293
// The name of the filter passed in the constructor.
294294
const std::string filter_name_;

‎source/extensions/filters/listener/dynamic_modules/filter_config.h‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,47 +120,50 @@ class DynamicModuleListenerFilterConfig
120120
Stats::Histogram& histogram_;
121121
};
122122

123+
// We use 1-based IDs for the metrics in the ABI, so we need to convert them to 0-based indices
124+
// for our internal storage. These helper functions do that conversion.
125+
#define ID_TO_INDEX(id) ((id) - 1)
126+
123127
// Methods for adding metrics during configuration.
124128
size_t addCounter(ModuleCounterHandle&& counter) {
125-
size_t id = counters_.size();
126129
counters_.push_back(std::move(counter));
127-
return id;
130+
return counters_.size();
128131
}
129132

130133
size_t addGauge(ModuleGaugeHandle&& gauge) {
131-
size_t id = gauges_.size();
132134
gauges_.push_back(std::move(gauge));
133-
return id;
135+
return gauges_.size();
134136
}
135137

136138
size_t addHistogram(ModuleHistogramHandle&& histogram) {
137-
size_t id = histograms_.size();
138139
histograms_.push_back(std::move(histogram));
139-
return id;
140+
return histograms_.size();
140141
}
141142

142143
// Methods for getting metrics by ID.
143144
OptRef<const ModuleCounterHandle> getCounterById(size_t id) const {
144-
if (id >= counters_.size()) {
145+
if (id == 0 || id > counters_.size()) {
145146
return {};
146147
}
147-
return counters_[id];
148+
return counters_[ID_TO_INDEX(id)];
148149
}
149150

150151
OptRef<const ModuleGaugeHandle> getGaugeById(size_t id) const {
151-
if (id >= gauges_.size()) {
152+
if (id == 0 || id > gauges_.size()) {
152153
return {};
153154
}
154-
return gauges_[id];
155+
return gauges_[ID_TO_INDEX(id)];
155156
}
156157

157158
OptRef<const ModuleHistogramHandle> getHistogramById(size_t id) const {
158-
if (id >= histograms_.size()) {
159+
if (id == 0 || id > histograms_.size()) {
159160
return {};
160161
}
161-
return histograms_[id];
162+
return histograms_[ID_TO_INDEX(id)];
162163
}
163164

165+
#undef ID_TO_INDEX
166+
164167
// Stats scope for metric creation.
165168
const Stats::ScopeSharedPtr stats_scope_;
166169
Stats::StatNamePool stat_name_pool_;

‎source/extensions/filters/network/dynamic_modules/filter_config.h‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,47 +126,50 @@ class DynamicModuleNetworkFilterConfig
126126
Stats::Histogram& histogram_;
127127
};
128128

129+
// We use 1-based IDs for the metrics in the ABI, so we need to convert them to 0-based indices
130+
// for our internal storage. These helper functions do that conversion.
131+
#define ID_TO_INDEX(id) ((id) - 1)
132+
129133
// Methods for adding metrics during configuration.
130134
size_t addCounter(ModuleCounterHandle&& counter) {
131-
size_t id = counters_.size();
132135
counters_.push_back(std::move(counter));
133-
return id;
136+
return counters_.size();
134137
}
135138

136139
size_t addGauge(ModuleGaugeHandle&& gauge) {
137-
size_t id = gauges_.size();
138140
gauges_.push_back(std::move(gauge));
139-
return id;
141+
return gauges_.size();
140142
}
141143

142144
size_t addHistogram(ModuleHistogramHandle&& histogram) {
143-
size_t id = histograms_.size();
144145
histograms_.push_back(std::move(histogram));
145-
return id;
146+
return histograms_.size();
146147
}
147148

148149
// Methods for getting metrics by ID.
149150
OptRef<const ModuleCounterHandle> getCounterById(size_t id) const {
150-
if (id >= counters_.size()) {
151+
if (id == 0 || id > counters_.size()) {
151152
return {};
152153
}
153-
return counters_[id];
154+
return counters_[ID_TO_INDEX(id)];
154155
}
155156

156157
OptRef<const ModuleGaugeHandle> getGaugeById(size_t id) const {
157-
if (id >= gauges_.size()) {
158+
if (id == 0 || id > gauges_.size()) {
158159
return {};
159160
}
160-
return gauges_[id];
161+
return gauges_[ID_TO_INDEX(id)];
161162
}
162163

163164
OptRef<const ModuleHistogramHandle> getHistogramById(size_t id) const {
164-
if (id >= histograms_.size()) {
165+
if (id == 0 || id > histograms_.size()) {
165166
return {};
166167
}
167-
return histograms_[id];
168+
return histograms_[ID_TO_INDEX(id)];
168169
}
169170

171+
#undef ID_TO_INDEX
172+
170173
// Stats scope for metric creation.
171174
const Stats::ScopeSharedPtr stats_scope_;
172175
Stats::StatNamePool stat_name_pool_;

‎source/extensions/filters/udp/dynamic_modules/filter_config.h‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,47 +71,50 @@ class DynamicModuleUdpListenerFilterConfig {
7171
Stats::Histogram& histogram_;
7272
};
7373

74+
// We use 1-based IDs for the metrics in the ABI, so we need to convert them to 0-based indices
75+
// for our internal storage. These helper functions do that conversion.
76+
#define ID_TO_INDEX(id) ((id) - 1)
77+
7478
// Methods for adding metrics during configuration.
7579
size_t addCounter(ModuleCounterHandle&& counter) {
76-
size_t id = counters_.size();
7780
counters_.push_back(std::move(counter));
78-
return id;
81+
return counters_.size();
7982
}
8083

8184
size_t addGauge(ModuleGaugeHandle&& gauge) {
82-
size_t id = gauges_.size();
8385
gauges_.push_back(std::move(gauge));
84-
return id;
86+
return gauges_.size();
8587
}
8688

8789
size_t addHistogram(ModuleHistogramHandle&& histogram) {
88-
size_t id = histograms_.size();
8990
histograms_.push_back(std::move(histogram));
90-
return id;
91+
return histograms_.size();
9192
}
9293

9394
// Methods for getting metrics by ID.
9495
OptRef<const ModuleCounterHandle> getCounterById(size_t id) const {
95-
if (id >= counters_.size()) {
96+
if (id == 0 || id > counters_.size()) {
9697
return {};
9798
}
98-
return counters_[id];
99+
return counters_[ID_TO_INDEX(id)];
99100
}
100101

101102
OptRef<const ModuleGaugeHandle> getGaugeById(size_t id) const {
102-
if (id >= gauges_.size()) {
103+
if (id == 0 || id > gauges_.size()) {
103104
return {};
104105
}
105-
return gauges_[id];
106+
return gauges_[ID_TO_INDEX(id)];
106107
}
107108

108109
OptRef<const ModuleHistogramHandle> getHistogramById(size_t id) const {
109-
if (id >= histograms_.size()) {
110+
if (id == 0 || id > histograms_.size()) {
110111
return {};
111112
}
112-
return histograms_[id];
113+
return histograms_[ID_TO_INDEX(id)];
113114
}
114115

116+
#undef ID_TO_INDEX
117+
115118
// Stats scope for metric creation.
116119
const Stats::ScopeSharedPtr stats_scope_;
117120
Stats::StatNamePool stat_name_pool_;

‎test/extensions/access_loggers/dynamic_modules/access_log_test.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ TEST_F(DynamicModuleAccessLogTest, MetricsCounterDefineAndIncrement) {
257257
EXPECT_EQ(envoy_dynamic_module_type_metrics_result_Success,
258258
envoy_dynamic_module_callback_access_logger_config_define_counter(
259259
static_cast<void*>(config_.get()), name, &counter_id));
260-
EXPECT_EQ(0, counter_id);
260+
EXPECT_EQ(1, counter_id);
261261

262262
EXPECT_EQ(envoy_dynamic_module_type_metrics_result_Success,
263263
envoy_dynamic_module_callback_access_logger_increment_counter(
@@ -277,7 +277,7 @@ TEST_F(DynamicModuleAccessLogTest, MetricsGaugeDefineAndManipulate) {
277277
EXPECT_EQ(envoy_dynamic_module_type_metrics_result_Success,
278278
envoy_dynamic_module_callback_access_logger_config_define_gauge(
279279
static_cast<void*>(config_.get()), name, &gauge_id));
280-
EXPECT_EQ(0, gauge_id);
280+
EXPECT_EQ(1, gauge_id);
281281

282282
EXPECT_EQ(envoy_dynamic_module_type_metrics_result_Success,
283283
envoy_dynamic_module_callback_access_logger_set_gauge(static_cast<void*>(config_.get()),
@@ -305,7 +305,7 @@ TEST_F(DynamicModuleAccessLogTest, MetricsHistogramDefineAndRecord) {
305305
EXPECT_EQ(envoy_dynamic_module_type_metrics_result_Success,
306306
envoy_dynamic_module_callback_access_logger_config_define_histogram(
307307
static_cast<void*>(config_.get()), name, &histogram_id));
308-
EXPECT_EQ(0, histogram_id);
308+
EXPECT_EQ(1, histogram_id);
309309

310310
EXPECT_EQ(envoy_dynamic_module_type_metrics_result_Success,
311311
envoy_dynamic_module_callback_access_logger_record_histogram_value(

0 commit comments

Comments
 (0)