stats: Histogram recordValue() can cause infinite stack loop

Histogram recordValue() can cause infinite stack loop

The current implementation of a Histogram differs from the Gauge/Counter in that it is immediately delivered to the Statsd sink instead of being buffered and periodically flushed. This poses a problem in a scenario where Histogram::recordValue is called in the code path of the TCP stats flusher (such as the load balancer).

This issue was discovered during the hotrestart_test for #5234, where a value is recorded into a histogram inside of LoadBalancerBase::choosePriority(). Recording the histogram value resulted in an infinite recursion that looked something like this (top to bottom):

Envoy::Upstream::LoadBalancerBase::choosePriority()
Envoy::Stats::ParentHistogramImpl::recordValue()
Envoy::Stats::ThreadLocalStoreImpl::deliverHistogramToSinks()
Envoy::Stats::ThreadLocalStoreImpl::ScopeImpl::deliverHistogramToSinks()
Envoy::Extensions::StatSinks::Common::Statsd::TcpStatsdSink::onHistogramComplete()
Envoy::Extensions::StatSinks::Common::Statsd::TcpStatsdSink::TlsSink::onTimespanComplete()
Envoy::Extensions::StatSinks::Common::Statsd::TcpStatsdSink::TlsSink::write()
Envoy::Upstream::ClusterManagerImpl::tcpConnForCluster()
Envoy::Upstream::LoadBalancerBase::chooseHost()
Envoy::Upstream::EdfLoadBalancerBase::chooseHostOnce()
Envoy::Upstream::ZoneAwareLoadBalancerBase::hostSourceToUse()
Envoy::Upstream::LoadBalancerBase::chooseHostSet()
Envoy::Upstream::LoadBalancerBase::choosePriority() <--- back to start of loop
Envoy::Stats::ParentHistogramImpl::recordValue()
...

Possible Solutions:

1. Buffer histogram stats for periodic flushing

We can have a per-TlsSink vector of a struct containing the information passed to TcpStatsdSink::TlsSink::onTimespanComplete. So, a string and a chrono::milliseconds value. Instead of instantly calling TlsSink::write, we can push into the vector and schedule a timer for immediate callback. The timer can flush all pending timespans. @mattklein123 suggests that the vector will stabilize in size and optimizations can be made later.

2. Mimic Gauge and Counter behavior

Similar to what we see in TcpStatsdSink::flush.

3. Passing in a flag to prevent recursive histogram calls

Passing a flag around to prevent recursive calls to Histogram::recordValue would work, but it's inelegant and requires developers to be aware of this scenario when adding future instrumentation. I'm just adding this as an option that was considered, but dismissed.