stats: cleanup APIs and avoid extra conversions between StatName and string.#10165
stats: cleanup APIs and avoid extra conversions between StatName and string.#10165mattklein123 merged 16 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…kes. Signed-off-by: Joshua Marantz <jmarantz@google.com>
… to StatName. Signed-off-by: Joshua Marantz <jmarantz@google.com>
jmarantz
changed the title
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
Thanks for this, just a few questions
| void SymbolTableImpl::Encoding::appendToMemBlock(StatName stat_name, | ||
| MemBlockBuilder<uint8_t>& mem_block) { | ||
| const uint8_t* data = stat_name.dataIncludingSize(); | ||
| if (data == nullptr) { |
There was a problem hiding this comment.
what's the use case for appending a stat name with no data? is this just to make it easier in test?
There was a problem hiding this comment.
no, empty constructor for null-stats and for when there is no tag-extracted-name.
This wasn't necessary before because a StatName constructed from "" didn't require this special-casing, but I wanted to allow StatName() to also work; without this it crashes.
| uint64_t num_bytes = 0; | ||
| for (StatName stat_name : stat_names) { | ||
| num_bytes += stat_name.dataSize(); | ||
| if (!stat_name.empty()) { |
There was a problem hiding this comment.
There are places in the code where elements being join()ed are empty, and we have been skipping these.
Here though we just have another case of an empty-constructed StatName() will crash now if we don't special-case this.
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
| MOCK_METHOD(void, onData, (Buffer::Instance & data)); | ||
| }; | ||
|
|
||
| class TestStatStore : public Stats::SymbolTableProvider, public Stats::IsolatedStoreImpl { |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
LGTM, thanks. Do you want to merge master?
Signed-off-by: Joshua Marantz <jmarantz@google.com>
4 participants
Description: Now that we take tags as a vector of StatName pairs, change the other args to also pass via StatName. This will reduce redundant converstions between string and StatName and also reduce acquisition of the symbol table lock.
Risk Level: low
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a