Skip to content

Conversation

mattklein123
Copy link
Member

This reverts:

109d23a
bad70bf
77748b2

@mergeconflict as I mentioned in Slack (https://envoyproxy.slack.com/archives/C78HA81DH/p1555624409062200), for statsd backends this writes raw histograms out to statsd at an extremely high rate which is not optimal. We should probably make this feature opt-in for perf reasons, but we need to either have a giant warning about statsd or we need to use an in-memory histogram for this data and maybe just write out summary data to the stats sinks? WDYT?

Either way I would prefer to revert for now if that is OK with you while we figure out the right solution.

This reverts commit 109d23a.

Signed-off-by: Matt Klein <mklein@lyft.com>
This reverts commit bad70bf.

Signed-off-by: Matt Klein <mklein@lyft.com>
This reverts commit 77748b2.

Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123
Copy link
Member Author

cc @tonya11en

Signed-off-by: Matt Klein <mklein@lyft.com>
Copy link
Member

@mergeconflict mergeconflict left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, makes sense. If I can figure out how to do a per-thread in-memory histogram, that's probably optimal. And either way, a config setting makes sense.

@mattklein123 mattklein123 merged commit 43e06d2 into master Apr 19, 2019
@mattklein123 mattklein123 deleted the revert_dispatcher_stats branch April 19, 2019 00:07
mpuncel added a commit to mpuncel/envoy that referenced this pull request Apr 19, 2019
* master: (26 commits)
  docs: update docs to recommend /retest repokitteh command (envoyproxy#6655)
  http timeout integration test: wait for 15s for upstream reset (envoyproxy#6646)
  access log: add response code details to the access log formatter (envoyproxy#6626)
  build: add ppc build badge to README (envoyproxy#6629)
  Revert dispatcher stats (envoyproxy#6649)
  Batch implementation with timer (envoyproxy#6452)
  fault filter: reset token bucket on data start (envoyproxy#6627)
  event: update libevent dependency to fix race condition (envoyproxy#6637)
  examples: standardize docker-compose version and yaml extension (envoyproxy#6613)
  quiche: Implement SpdyUnsafeArena using SpdySimpleArena (envoyproxy#6612)
  router: support customizable retry back-off intervals (envoyproxy#6568)
  api: create OpenRCA service proto file (envoyproxy#6497)
  ext_authz: option for clearing route cache of authorized requests (envoyproxy#6503)
  build: update jinja to 2.10.1. (envoyproxy#6623)
  tools: check spelling in pre-push hook (envoyproxy#6631)
  security: blameless postmortem template. (envoyproxy#6553)
  Implementing Endpoint lease for ClusterLoadAssigment (envoyproxy#6477)
  add HTTP integration tests exercising timeouts (envoyproxy#6621)
  event: fix DispatcherImplTest::InitializeStats flake (envoyproxy#6619)
  Add tag extractor for RDS route config name (envoyproxy#6618)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
htuch pushed a commit that referenced this pull request Apr 23, 2019
Reintroduce dispatcher stats previously reverted in #6649. Dispatcher stats are now gated on a new bootstrap config parameter, enable_dispatcher_stats, and disabled by default.

Risk Level: Low
Testing: Manually verified that config is respected (see #6582, this still requires integration tests).
Docs Changes: Included a note about how statsd doesn't play well with these high-volume histograms.

Signed-off-by: Dan Rosen <mergeconflict@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants