instrumentation: add timers and warnings to platform callbacks#2300
instrumentation: add timers and warnings to platform callbacks#2300
Conversation
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
|
|
||
| namespace { | ||
|
|
||
| constexpr auto SlowCallbackWarningThreshold = std::chrono::seconds(1); |
There was a problem hiding this comment.
+1 to starting at 1 second here. Ideally we roll this out at scale and see that it's rarely hit, if ever, and we can reduce it accordingly. Ideally pt2 is that we find a good way to make this configurable.
|
Since this is a user-facing change, can you please add an entry to the version history doc? |
| const envoy_http_filter* platform_filter() const { return platform_filter_; } | ||
|
|
||
| private: | ||
| static PlatformBridgeFilterStats generateStats(const std::string& stat_prefix, |
There was a problem hiding this comment.
Why have the stat prefix if we only set it to the empty string?
There was a problem hiding this comment.
Ah, good question, it's
a) because I Considered making it the filter name and think that still might be worth considering, but I held off because I'm not sure how consistently/well those are formatted for use in stats,
and
b) because this is the signature used everywhere in Envoy for this pattern.
jpsim
left a comment
There was a problem hiding this comment.
Approving now so we can observe the stats over the weekend, but I'll review in more detail next week.
Signed-off-by: Mike Schore <mike.schore@gmail.com>
* origin/main: (32 commits) Compress xcframework release asset (#2324) bazel: update rules_apple (#2326) Merge `android_dist` with `android_dist_ci` (#2323) kotlin: fix flaky receive error test (#2317) kotlin: fix flaky grpc test (#2316) build(deps): bump pyjwt from 2.1.0 to 2.4.0 in /.github/actions/pr_notifier (#2314) test: making C++ integration test more authentic (#2315) reverting override to override_request_timeout_by_gateway_timeout (#2296) Update Envoy (#2309) release: use `CREDENTIALS_GITHUB_RELEASE_DEPLOY_KEY` Use `CREDENTIALS_GITHUB_PUSH_TOKEN` Push to branch before tagging the release Cronvoy: preparation to unittest certificate verification JNI (#2251) Corrected typo in documentation for Android building requirements (#2313) instrumentation: add timers and warnings to platform callbacks (#2300) Bump Lyft Support Rotation (#2310) Revert "android: use local addresses as opposed to prefix (#2081)" (#2307) Fix release GitHub workflow (#2306) mobile: moving the c++ integration test to use default config (#2293) config: cleaning up deprecated configs (#2295) ... Signed-off-by: JP Simard <jp@jpsim.com>
Description: Adds Envoy histograms (timers) to most platform-provided callbacks in the Http::Client and in PlatformBridge Filters. Beyond a hardcoded threshold (1s), a warn-level LOG_EVENT will be emitted for slow-to-execute callbacks.
Risk Level: Low
Testing: Manual & Existing Coverage
Signed-off-by: Mike Schore mike.schore@gmail.com