go-metrics icon indicating copy to clipboard operation
go-metrics copied to clipboard

Consider adding mechanism to register alternate Sinks with NewMetricSinkFromURL

Open banks opened this issue 6 years ago • 2 comments

In my case, I'd like to use the provided sub-package Dogstatsd sink but without loosing the abstraction this library provides.

While I could write my own config/setup method that parses URL and does something different if protocol is dogstatsd:// (i.e. instantiates dogstatsd sink directly instead of using NewMetricSinkFromURL) it seems undesirable to reinvent that wheel.

Proposal

Allow external packages to register syncs early in execution (i.e. in func init()) that can hook into the central mechanism.

That might look something like this:


// Existing registry
var sinkRegistry = map[string]sinkURLFactoryFunc{
	"statsd":   NewStatsdSinkFromURL,
	"statsite": NewStatsiteSinkFromURL,
	"inmem":    NewInmemSinkFromURL,
}

// Add new mutex - might be unnecessary if we document that it's only safe to register in 
// `init` methods but I haven't found spec to say those are guaranteed not to be run in 
// parallel yet either.
var registryMu sync.RWMutex

func RegisterSyncForScheme(scheme string, factory sinkURLFactoryFunc) {
    registryMu.Lock()
    defer registryMu.Unlock()

    sinkRegistry[scheme] = factory
}

// Existing func
func NewMetricSinkFromURL(urlStr string) (MetricSink, error) {
    // ...
    registryMu.RLock()
    sinkURLFactoryFunc := sinkRegistry[u.Scheme]
    registryMu.RUnlock()
    /// ...
}

Then the included additional sinks or custom sinks defined in outer packages can use the same method to be instantiated at runtime.

I made an issue rather than just a straight PR with that code in just to check that there is no good reason this wasn't done already. If you agree it's worthwhile @armon I can make a PR.

banks avatar Aug 30 '17 12:08 banks

Golang spec says:

Package initialization—variable initialization and the invocation of init functions—happens in a single goroutine, sequentially, one package at a time.

https://golang.org/ref/spec#Package_initialization

So we could go without mutex and document that it's only valid to call Register... from func init.

banks avatar Aug 30 '17 15:08 banks

@banks I like it! Seems like a good way to make it extensible

armon avatar Sep 04 '17 22:09 armon