stat: Add counterFromStatName(), gaugeFromStatName(), and histogramFromStatName()#6475
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>
ambuc
left a comment
ambuc
left a comment
There was a problem hiding this comment.
LGTM. Could you modify the stats-specific README to show where in the process this PR leaves us?
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
I'm a little reluctant to hack the doc as this is not a meaningful change to the operation of stats in this PR, just a new API this is defined and tested but not used. #6161 changes operations and explains how and why in stats.md. |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Looks good with 1 question/comment.
/wait-any
|
|
||
| // Variant of ScopePrefixer that references the scope being prefixed, but does | ||
| // not own it. | ||
| ScopePrefixer::ScopePrefixer(absl::string_view prefix, Scope& scope) |
There was a problem hiding this comment.
I don't see this variant used, is it used in a follow up?
Also, somewhat nitty, but for clarity I would prefer a single constructor that looks like:
ScopePrefixer::ScopePrefixer(absl::string_view prefix, Scope& scope, bool take_ownership)
And then personally I would have the following members:
std::unique_ptr<Scope> owned_scope_;
Scope* scope_to_use_;
And avoid the manual deletion. WDYT?
There was a problem hiding this comment.
Yeah I like your pattern better. However I also can't justify the speculative generality here; I needed it in some past iteration and don't see a use-case now. If I have to add it back in the future I'll use your pattern.
update coming.
Signed-off-by: Joshua Marantz <jmarantz@google.com>
… scope. Signed-off-by: Joshua Marantz <jmarantz@google.com>
* master: (137 commits) test: router upstream log to v2 config stubs (envoyproxy#6499) remove idle timeout validation (envoyproxy#6500) build: Change namespace of chromium_url. (envoyproxy#6506) coverage: exclude chromium_url (envoyproxy#6498) fix(tracing): allow 256 chars in path tag (envoyproxy#6492) Common: Introduce StopAllIteration filter status for decoding and encoding filters (envoyproxy#5954) build: update PGV url (envoyproxy#6495) subset lb: avoid partitioning host lists on worker threads (envoyproxy#6302) ci: Make envoy_select_quiche no-op. (envoyproxy#6393) watcher: notify when watched files are modified (envoyproxy#6215) stat: Add counterFromStatName(), gaugeFromStatName(), and histogramFromStatName() (envoyproxy#6475) bump to 1.11.0-dev (envoyproxy#6490) release: bump to 1.10.0 (envoyproxy#6489) hcm: path normalization. (#1) build: import manually minified Chrome URL lib. (envoyproxy#3) codec: reject embedded NUL in headers. (envoyproxy#2) Added veryfication if path contains query params and add them to path header (envoyproxy#6466) redis: basic integration test for redis_proxy (envoyproxy#6450) stats: report sample count as an integer to prevent loss of precision (envoyproxy#6274) Added VHDS protobuf message and updated RouteConfig to include it. (envoyproxy#6418) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
3 participants
Description: #6161 is a 45-file PR that converts the stats infrastructure to use a SymbolTable API for representing names, which is challenging to review due to several concepts occurring at once. This PR factors out one of the concepts: the new APIs for Stats::Store and Stats::Scope.
Risk Level: low -- this introduces new APIs that are not actually used yet, though they are tested.
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a