Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] Allow users to create package global metrics #59

Open
prashantv opened this issue Sep 20, 2017 · 1 comment
Open

[proposal] Allow users to create package global metrics #59

prashantv opened this issue Sep 20, 2017 · 1 comment

Comments

@prashantv
Copy link
Contributor

A user currently can't create a package global metric like so:

package foo

var totalRequests = root.NewCounter("total-requests")

func myFunc() {}

I'm not sure whether this is a pattern we want to encourage, but I've seen many examples of users building wrapper packages to provide this functionality, and other libraries allow this (see below).

Prometheus:
https://godoc.org/github.com/prometheus/client_golang/prometheus

As well as the inbuilt expvar package:
https://golang.org/pkg/expvar/#NewInt

Tally requires a scope to create any metrics, and the scope isn't created till the backend is initialized which often requires configuration etc. Instead of forcing users to create a scope for the correct backend in init, I think we could support the idea of deferred scopes/metrics.

type DeferredScope struct {
  counters []*deferredCounter
  [...]
}

func (ds *DeferredScope) Counter(name string) Counter {
  c := &deferredCounter{Name: name}
  ds.counters = append(ds.counters, c)
  return c
}

type deferredCounter struct {
  name string
  counter Counter
}

func (dc *deferredCounter) init(scope Scope) {
  dc.counter = scope.Counter(dc.name)
}

func (dc *deferredCounter) Inc(delta int64) {
  if dc.counter == nil {
    return
  }
  dc.counter.Inc(delta)
}

By default, the deferred scope just throws away these counters, but it could then be initialized using a real scope:

func (ds *DeferredScope) Init(rootScope Scope) {
  for _, c := range ds.counters {
    c.init(rootScope)
  }
  [..]
}

That way the user can create all their metrics as package globals, and later on (typically in their main function), once they have a config, they can create a root scope from a real backend and call Init on the deferred scope.

cc @lopter @akshayjshah @martin-mao

@jeromefroe
Copy link
Contributor

Personally, the thing I like most about Prometheus's client libraries is that they easily lead to standard conventions. Metric definitions are visually distinctive and are almost always defined at the top of a file so it's very easy when looking at source code to see exactly what metrics are defined. With tally, unfortunately, there is a lot less standardization with respect to defining metrics and this leads to a lot of different conventions. My hesitation towards supporting package global metrics, therefore, is that we might be adding to the current confusion of how to define metrics. I think we should start by doing a much better job with the documentation, in particular by codifying best practices and providing more examples and getting started guides. Then I think users would be less likely to reach for this functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants