channel pool metric prototype - #16324
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces tracking for outstanding RPCs on selected channels within the Bigtable client's connection pool. It updates the BigtableStub interface and its decorators to accept an OperationContext parameter across all synchronous and asynchronous RPC methods. Additionally, it renames ResourceLabels and DataLabels to TableResourceLabels and TableDataLabels respectively, while introducing ClientResourceLabels and the OutstandingRpcs metric. Feedback on the changes suggests simplifying the label filtering logic in metrics.cc by replacing a complex std::set_difference call with a simpler loop, adhering to the repository's "Reject 'Code Poetry'" style guide principle.
| struct Compare { | ||
| bool operator()(std::pair<std::string const, std::string> const& a, | ||
| std::string const& b) { | ||
| return a.first < b; | ||
| } | ||
|
|
||
| bool operator()(std::string const& a, | ||
| std::pair<std::string const, std::string> const& b) { | ||
| return a < b.first; | ||
| } | ||
| }; | ||
|
|
||
| std::set_difference(data.begin(), data.end(), filtered_data_labels.begin(), | ||
| filtered_data_labels.end(), | ||
| std::inserter(labels, labels.begin()), Compare()); |
There was a problem hiding this comment.
Reject "Code Poetry": Simplify Label Filtering
The data map contains exactly 3 elements. Using std::set_difference with a custom Compare struct is overly complex for filtering such a small number of elements.
Per the repository style guide (Reject "Code Poetry"), we should prefer simplicity over cleverness. A simple loop checking if each key is in filtered_data_labels is much easier to read, maintain, and is likely more performant by avoiding the overhead of std::set_difference and custom comparators.
for (auto const& kv : data) {
if (filtered_data_labels.find(kv.first) == filtered_data_labels.end()) {
labels.insert(kv);
}
}References
- Reject "Code Poetry": Dismantle complex abstractions used for simple tasks. Prefer simplicity over cleverness. (link)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16324 +/- ##
==========================================
+ Coverage 92.24% 92.27% +0.03%
==========================================
Files 2227 2227
Lines 209208 209410 +202
==========================================
+ Hits 192976 193233 +257
+ Misses 16232 16177 -55 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
79de35f to
e0f403a
Compare
1 participant
No description provided.