-
Notifications
You must be signed in to change notification settings - Fork 164
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
RFC 0003, 0007 updates (Metrics measure type, metrics handle specification), RFC 0004 (configurable aggregation) deleted #29
Changes from 14 commits
e401916
c4a7836
beff7cf
0aa9665
2c749d8
7d5bee5
2e541db
8e9fcc3
5ae1df5
3bebb92
7b08d60
b93d931
a8b7baf
f5e5c39
d97868f
e997cce
54a778f
fd1c027
deb6fac
a1e526d
9aee892
7ae4bd8
18587f6
c3225a2
f1e5972
ec53885
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
# Metric Handle API specification | ||
|
||
**Status:** `proposed` | ||
|
||
Specify the behavior of the Metrics API `Handle` type, for efficient repeated-use of metric instruments. | ||
|
||
## Motivation | ||
|
||
The specification currently names this concept `TimeSeries`, the object returned by `GetOrCreateTimeseries`, which supports binding a metric to a pre-defined set of labels for repeated use. This proposal renames these `Handle` and `GetHandle`, respectively, and adds further detail to the API specification for handles. | ||
|
||
## Explanation | ||
|
||
The `TimeSeries` is renamed to `Handle` as the former name suggests an implementation, not an API concept. `Handle`, we feel, is more descriptive of the intended use. Likewise with `GetOrCreateTimeSeries` to `GetHandle` and `GetDefaultTimeSeries` to `GetDefaultHandle`, these names suggest an implementation and not the intended use. Applications are encouraged to re-use metric handles for efficiency. | ||
jmacd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Handles are useful to reduce the cost of repeatedly recording a metric instrument (cumulative, gauge, or measure) with a pre-defined set of label values. All metric kinds support declaring a set of required label keys. These label keys, by definition, must be specified in every metric `Handle`. We permit "unspecified" label values in cases where a handle is requested but a value was not provided. The default metric handle has all its required keys unspecified. We presume that fast pre-aggregation of metrics data is only possible, in general, when the pre-aggregation keys are a subset of the required keys on the metric. | ||
|
||
`GetHandle` specifies two calling patterns that may be supported: one with ordered label values and one without. The facility for ordered label values is provided as a potential optimization that facilitates a simple lookup for the SDK; in this form, the API is permitted to thrown an exception or return an error when there is a mismatch in the arguments to `GetHandle`. When label values are accepted in any order, some SDKs may perform an expensive lookup to find an existing metrics handle, but they must not throw exceptions. | ||
|
||
`GetHandle` and `GetDefaultHandle` support additional label values not required in the definition of the metric instrument. These optional labels act the same as pre-defined labels in the low-level metrics data representation, only that they are not required. Some SDKs may elect to use additional label values as the "attachment" data on metrics. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I understand "additional label values", do you mean these labels may be extracted from the "Context"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We discussed two cases: I've posted an open question at the bottom. Can we use additional labels to capture the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we start without the support of these extra labels for the moment? Adding a new API is always possible and backwards compatible. Trying to start a bit small :) |
||
|
||
## Internal details | ||
|
||
The names (`Handle`, `GetHandle`, ...) are just language-neutral recommendations. Because each of the metric kinds supports a different operation (`Add()`, `Set()`, and `Record()`), there are logically distinct kinds of handle. Language APIs should feel free to choose type and method names with attention to the language's style. | ||
|
||
An implementation of `GetHandle` may elect to return a unique object to multiple callers for its own purposes, but implementations are not required to do so. When unique objects are a guarantee, implementation should consider additional label values in the uniqueness of the handle, to maintain the low-level metric event respresentation discussed in RFC [0003-measure-metric-type](./0003-measure-metric-tuype.md). | ||
|
||
The `Observer` API for observing gauge metrics on demand via a callback does not support handles. | ||
|
||
## Trade-offs and mitigations | ||
|
||
The addition of additional label values, for handles, is not essential for pre-aggregation purposes, so it may be seen as non-essential in that regard. However, API support for pre-defined labels also benefits program readability because it allows metric handles to be defined once in the source, rather than once per call site. | ||
|
||
This benefit could be extended even further, as a potential future improvement. Instead of creating one handle per instance of a metric with pre-defined values, it may be even more efficient to support pre-defining a set of label values for use constructing multiple metric handles. Consider the code for declaring three metrics: | ||
|
||
``` | ||
var gauge = metric.NewFloat64Gauge("example.com/gauge", metric.WithKeys("a", "b", "c")) | ||
var counter = metric.NewFloat64Cumulative("example.com/counter", metric.WithKeys("a", "b", "c")) | ||
var measure = metric.NewFloat64Measure("example.com/measure", metric.WithKeys("a", "b", "c")) | ||
``` | ||
|
||
and three handles: | ||
|
||
``` | ||
gaugeHandle := gauge.GetHandleOrdered(1, 2, 3) // values for a, b, c | ||
counterHandle := counter.GetHandleOrdered(1, 2, 3) // values for a, b, c | ||
measureHandle := measure.GetHandleOrdered(1, 2, 3) // values for a, b, c | ||
``` | ||
|
||
This can be potential improved as by making the label set a first-class concept. This has the potential to further reduce the cost of getting a group of handles with the same set of labels: | ||
|
||
``` | ||
var commonKeys = metric.DefineKeys("a", "b", "c") | ||
var gauge = metric.NewFloat64Gauge("example.com/gauge", metric.WithKeys(commonKeys)) | ||
var counter = metric.NewFloat64Cumulative("example.com/counter", metric.WithKeys(commonKeys)) | ||
var measure = metric.NewFloat64Measure("example.com/measure", metric.WithKeys(commonKeys)) | ||
|
||
labelSet := commonKeys.Values(1, 2, 3) // values for a, b, c | ||
gaugeHandle := gauge.GetHandleOrdered(labelSet) | ||
counterHandle := counter.GetHandleOrdered(labelSet) | ||
measureHandle := measure.GetHandleOrdered(labelSet) | ||
``` | ||
|
||
## Open questions | ||
|
||
Should the additional scope concept shown above be implemented? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having
gauge.GetHandle(...)
seems to be okay.Having a class/type named
Handle
could be confusing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the Go prototype, there is indeed a
Handle
type but it's for internal use. The types returned by theGetHandle()
methods in Go PR#100 are likeFloat64GaugeHandle
,Int64CumulativeHandle
. Is this sufficiently clear?