Schneems/rack compat #18
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In most cases the metrics being collected are not actually timings, but instead are counters. There are, however, some things that *are* actually "timers" which we need to address in a future commit. For now, we still submit the counters, as timers, to statsd to maintain the statsd aggregation facilities that are useful for per request stat tracking. (Tests are probably failing after this commit)
RackReporter reports only counters (as timers), which are useful in a per request context (e.g. how many minor collections are happening per request), using sampling strategies. However, data for the entire process, which previously was reported as Rack::Server should not be sampled. Instead, the PeriodicReporter reports to statsd as counts and gauges, providing a more accurate look at the server's characteristics over time. The intention of PeriodicReporter is to be used by a clock that measures, and then reports on an interval corresponding to the statsd flushInterval (by default something like 10 seconds). A clock is not included in this commit. Stay tuned.
Periodic collects metrics and reports them to reporter on an interval, rather than on a per request basis (such as the Rack class). To avoid large rearchitecting, it mimics that of the Rack Middleware by conforming to the environment passing model of the reporter and metrics storage, using *it's* thread local storage for that of persisted last values of counters to track changes. In addition, we add a RailTie, specifically for the enabling and starting the periodic reporter.
- ReporterTest ensures that report implementations do nothing - Move ReporterTest -> RackReporterTest - Add PeriodicReporterTest which tests sample rates implementation.
Provide a test helper for Statsd which yields after a call to `:block`. This provides a basis for mocking out the `:count` and `gauge` methods.
For reasons of Linux, and, related to Puma things, this SYN_RECV counter isn't an effective tool for measuring anything meaningful. The Linux kernel actually has two queues, and the queue that is important to measure ... can't be. So, this SYN_RECV counter is really measuring the number of client initiated connections that haven't completed the 3-way TCP handshake (e.g. haven't ACK'd), and that's a typical thing of a malicious actor, or very slow internet connection (but something we'd unlikely catch at a 20 second polling). There's an item in Puma (and possibly other ruby web servers, called the "backlog", which represents the number of connections waiting for a thread in the thread pool. This is likely a more interesting measurement, but even more interesting is probably the amount of time those connections sit idle waiting to be picked up, which we can get from the Heroku router. Since we're targetting Heroku with this, we'll simply dismiss this as being useful and move on.
Round of Changes
Rename trashed to barnes.
Remove any and all assumptions that things are request driven. It's all periodic, all the time. In addition, simplify the concept of a meter, into a panel which contains instruments that can be `instrument!`'d. These `!`methods modify passed in state and are essentially folded over to collect the current values of the gauges and counters. The instruments themselves emit the proper key, and in the case of counters that we'd like to treat as special counters, they apply sampling. See an explanation of that in doc.rb. Fix up the Railtie such that we can actually configure this thing a bit easier, and remove things we'll just plain not utilize, like the ChangeInstrument and GaugeInstrument which weren't utilized and confused things.
[WIP] Giant refactor!!!!!!!!!
- prefer named args - don't use `send` when possible to avoid - Barnes.start now can be called by Rack for easy starting - Railties uses Barnes.start - Prefer aborting when there's an exception in the background thread
|
Ugh, sorry. |
|
🤣 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.