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.

Sign up for GitHub

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

Jump to bottom

metrics: improve README #417

Merged
merged 2 commits into from
Dec 18, 2016
Merged

metrics: improve README #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9a1b5e
metrics: improve README
peterbourgon Dec 17, 2016
200cb17
metrics: minor improvements to package docs
peterbourgon Dec 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions metrics/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,5 @@ func exportGoroutines(g metrics.Gauge) {
}
}
```

For more information, see [the package documentation](https://godoc.org/github.com/go-kit/kit/metrics).
42 changes: 33 additions & 9 deletions metrics/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,35 @@
// Package metrics provides a framework for application instrumentation. All
// metrics are safe for concurrent use. Considerable design influence has been
// taken from https://github.com/codahale/metrics and https://prometheus.io.
// Package metrics provides a framework for application instrumentation. It's
// primarily designed to help you get started with good and robust
// instrumentation, and to help you migrate from a less-capable system like
// Graphite to a more-capable system like Prometheus. If your organization has
// already standardized on an instrumentation system like Prometheus, and has no
// plans to change, it may make sense to use that system's instrumentation
// library directly.
//
// This package contains the common interfaces. Your code should take these
// interfaces as parameters. Implementations are provided for different
// instrumentation systems in the various subdirectories.
// This package provides three core metric abstractions (Counter, Gauge, and
// Histogram) and implementations for almost all common instrumentation
// backends. Each metric has an observation method (Add, Set, or Observe,
// respectively) used to record values, and a With method to "scope" the
// observation by various parameters. For example, you might have a Histogram to
// record request durations, parameterized by the method that's being called.
Copy link
Contributor

@willfaught willfaught Dec 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's useful to note that for impls that support With, the semantics are different for instantiating them than for those that don't. For example, for Prometheus, you'd do something like NewPrometheusCounter("request_duration").With("foo_service").With("bar_endpoint"), whereas for Statsd you'd do something like NewStatsdCounter("foo_service_bar_endpoint_request_duration") (since With is useless for distinguishing metrics in that case). At least, that's my limited understanding; correct me if I'm wrong.

Something like "Note that for implementations that do not support With, the full metric name and context must be provided when creating the instance, not using With"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label names seem to be implementation-dependent as well (please correct me if I'm wrong!). It seems, IIUC, that for Prometheus, you should use underscores as separators, whereas for Statsd you should use periods. I imagine it's possible for another implementation that actually uses With to expect periods too. So for gokit, the label value format for With is implementation-specific, even though the interfaces are supposed to abstract away those details. Kinda defeats the purpose of the interfaces, IMO. Maybe the With impls should accept a common format and transform it if necessary to the underlying impl's format?

//
// var requestDuration metrics.Histogram
// // ...
// requestDuration.With("method", "MyMethod").Observe(time.Since(begin))
//
// This allows a single high-level metrics object (requestDuration) to work with
// many code paths somewhat dynamically. The concept of With is fully supported
// in some backends like Prometheus, and not supported in other backends like
// Graphite. So, With may be a no-op, depending on the concrete implementation
// you choose.
//
// Usage
//
// Metrics are dependencies and should be passed to the components that need
// Metrics are dependencies, and should be passed to the components that need
// them in the same way you'd construct and pass a database handle, or reference
// to another component. So, create metrics in your func main, using whichever
// concrete implementation is appropriate for your organization.
// to another component. Metrics should *not* be created in the global scope.
// Instead, instantiate metrics in your func main, using whichever concrete
// implementation is appropriate for your organization.
//
// latency := prometheus.NewSummaryFrom(stdprometheus.SummaryOpts{
// Namespace: "myteam",
Expand Down Expand Up @@ -40,8 +58,14 @@
// api := NewAPI(store, logger, latency)
// http.ListenAndServe("/", api)
//
// Note that metrics are "write-only" interfaces.
//
// Implementation details
//
// All metrics are safe for concurrent use. Considerable design influence has
// been taken from https://github.com/codahale/metrics and
// https://prometheus.io.
//
// Each telemetry system has different semantics for label values, push vs.
// pull, support for histograms, etc. These properties influence the design of
// their respective packages. This table attempts to summarize the key points of
Expand Down