NopCachedStatsReporter

[anuptalwalkar]

I see that we have implemented CachedStatsReporter here, #22 but there is no Nop implementation for testing. This is making transition from StatsReporter to CachedStatsReporter rather difficult. Do we have any plans to provide NopCachedStatsReporter similar to existing tally.NullStatsReporter?

[Raynos]

We've not implemented Null, Statsd or Prometheus yet for the cached stats reporter.

For testing purposes I will need to use use Statsd and Null reporters soon, I can contribute these to tally.

I've currently worked around it on master by having all tests use an InternalMetricsReporter.

I don't personally use Prometheus so someone else will need to have look into implementing CachedRepoter for Prometheus.

[anuptalwalkar]

I have a NopCachedStatsReporter written for testing - https://github.com/uber-go/fx/blob/master/metrics/nop_reporter.go I am happy to share if needed.

[robskillington]

@anuptalwalkar is there any reason you can't use a tally.TestScope?

You can create one by calling tally.NewTestScope(...) as implemented here:

func NewTestScope(
	prefix string,
	tags map[string]string,
) TestScope {
	// ...
}

Then you get the following abilities:

type TestScope interface {
	Scope

	// Snapshot returns a copy of all values since the last report execution,
	// this is an expensive operation and should only be use for testing purposes
	Snapshot() Snapshot
}

// Snapshot is a snapshot of values since last report execution
type Snapshot interface {
	// Counters returns a snapshot of all counter summations since last report execution
	Counters() map[string]CounterSnapshot

	// Gauges returns a snapshot of gauge last values since last report execution
	Gauges() map[string]GaugeSnapshot

	// Timers returns a snapshot of timer values since last report execution
	Timers() map[string]TimerSnapshot
}

// CounterSnapshot is a snapshot of a counter
type CounterSnapshot interface {
	// Name returns the name
	Name() string

	// Tags returns the tags
	Tags() map[string]string

	// Value returns the value
	Value() int64
}

// GaugeSnapshot is a snapshot of a gauge
type GaugeSnapshot interface {
	// Name returns the name
	Name() string

	// Tags returns the tags
	Tags() map[string]string

	// Value returns the value
	Value() float64
}

// TimerSnapshot is a snapshot of a timer
type TimerSnapshot interface {
	// Name returns the name
	Name() string

	// Tags returns the tags
	Tags() map[string]string

	// Values returns the values
	Values() []time.Duration
}

[jeffbean]

The issue here is doing a setup function to return the scope, reporter and closer can not easily be replaced today. The NullStatsReporter does not implement CachedStatsReporter interface.

The use case is something I want to test with as well. If you make a public function to create the metrics with a CachedStatsReporter and need to test it out there is no supporting tally NullCachedStatsReporter.

Example from my use case as well. https://github.com/jeffbean/zkpacket/blob/master/metrics.go

[anuptalwalkar]

@robskillington, @jeffbean answered your question. The M3 backend also takes CachedStatsReporter instead of StatsReported. This prompted me to have NopCachedStatsReporter for nop use cases. Updated implementation is here with histogram support - uber-go/fx#334 This should be part of Tally.

[robskillington]

Hey, cool no problems. Sure if you don't want to collect the actual values then a nil reporter is useful. Let me add.

[madhuravi]

any update on this? @robskillington

[akshayjshah]

Not sure about @jeffbean, but FX doesn't need this any more. Unless Bean still needs this functionality, feel free to close this.