tally NoopScope doesn't support tagging

[yutongp]

tally.NoopScope.Capabilities().Tagging() return false go-common/metrics.None.Capabilities().Tagging() return true

I think for testing, NoopScope should support tagging since some model like (rpcinit) will panic if user pass in a scope which doesn't support tagging

[prateek]

tl;dr - you should still be able to create tagged metrics with NoopScope/TestScope. E.g. test demonstrating this:

func TestExistingNoopAndTestScopeScope(t *testing.T) {
	scopes := []tally.Scope{
		tally.NoopScope,
		tally.NewTestScope("", nil),
	}
	for _, s := range scopes {
		require.False(t, s.Capabilities().Reporting())
		require.False(t, s.Capabilities().Tagging())
		s.Tagged(map[string]string{
			"a": "b",
		}).Counter("xyz").Inc(1) // <-- works fine
	}
}

Longer version: A scope's capabilities depend upon the underlying reporter's capabilities. The NoopScope uses the NullStatsReporter which does not have any capabilities, and returns the same.

If you need to use a reporter which has those capabilities, you can create a stub impl for it, e.g. -

func TestNoopReporter(t *testing.T) {
	noopScope, _ := tally.NewRootScope(tally.ScopeOptions{
		Reporter: NoopStatsReporter,
	}, 0)
	require.True(t, noopScope.Capabilities().Reporting())
	require.True(t, noopScope.Capabilities().Tagging())
} // test passes, i.e. the scope returns it has tagging and reporting capabilities 

var NoopStatsReporter tally.StatsReporter = noopStatsReporter{}

func (r noopStatsReporter) Capabilities() tally.Capabilities                     { return allCapabilities{} }
func (r noopStatsReporter) Flush()                                               {}
func (r noopStatsReporter) ReportCounter(string, map[string]string, int64)       {}
func (r noopStatsReporter) ReportGauge(string, map[string]string, float64)       {}
func (r noopStatsReporter) ReportTimer(string, map[string]string, time.Duration) {}
func (r noopStatsReporter) ReportHistogramValueSamples(string, map[string]string, tally.Buckets, float64, float64, int64) {
}
func (r noopStatsReporter) ReportHistogramDurationSamples(string, map[string]string, tally.Buckets, time.Duration, time.Duration, int64) {
}

type noopStatsReporter struct{}

func (c allCapabilities) Tagging() bool   { return true }
func (c allCapabilities) Reporting() bool { return true }

type allCapabilities struct{}

I'm curious why you need this, do your tests/source-code check the capabilities of the reporter prior to usage?

[yutongp]

Hi Prateek,

if you check the code of uber's internal rpcinit:

func NewDispatcherFromConfig(dCfg *DispatcherConfig) *Dispatcher {
	if dCfg.Metrics == nil {
		log.Panic("a metrics scope is required for rpcinit")
	}

	if !dCfg.Metrics.Capabilities().Tagging() {
		log.Panic("a metrics scope supporting tags is required for rpcinit (see M3Scope and MigrationScope")
	}

before using tally, I can pass a go-common/metrics.None to get a Dispatcher, but now with tally.NoopScope, this function painc.

[prateek]

@yutongp why do you want to migrate to tally instead of using the go-common/metrics wrapper?