-
Notifications
You must be signed in to change notification settings - Fork 617
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
Metrics API with RFC 0003 #87
Changes from 27 commits
6ca4274
b23cec1
981eece
8ea9709
00b4f11
34c87ce
df8ae34
a2561ac
1ece493
ce9268a
f5f9f01
74a1815
0a0b8ee
555bf50
d6b1113
c819109
18cfc71
f646555
a44cb47
94eaad9
fc251b2
262f312
66c0a56
e2c4a7e
2fb7646
eb711cb
baa3a32
5c30a9c
211b20c
bffe040
0759b9a
44d62f8
c5ab2df
50d2de5
d79bc7d
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,5 @@ | ||
opentelemetry.metrics.aggregation module | ||
========================================== | ||
|
||
.. automodule:: opentelemetry.metrics.aggregation | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
opentelemetry.metrics package | ||
============================= | ||
|
||
Submodules | ||
---------- | ||
|
||
.. toctree:: | ||
|
||
opentelemetry.metrics.aggregation | ||
opentelemetry.metrics.time_series | ||
|
||
Module contents | ||
--------------- | ||
|
||
.. automodule:: opentelemetry.metrics |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
opentelemetry.metrics.time\_series module | ||
========================================== | ||
|
||
.. automodule:: opentelemetry.metrics.time_series | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,334 @@ | |
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
""" | ||
The OpenTelemetry metrics API describes the classes used to report raw | ||
measurements, as well as metrics with known aggregation and labels. | ||
|
||
The `Meter` class is used to construct `Metric` s to record raw statistics | ||
as well as metrics with predefined aggregation. | ||
|
||
See the `metrics api`_ spec for terminology and context clarification. | ||
|
||
.. _metrics api: | ||
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-metrics.md | ||
|
||
|
||
""" | ||
|
||
from abc import ABC | ||
from abc import abstractmethod | ||
from typing import List | ||
from typing import Union | ||
|
||
from opentelemetry.metrics.aggregation import Aggregation | ||
from opentelemetry.metrics.time_series import CounterTimeSeries | ||
from opentelemetry.metrics.time_series import GaugeTimeSeries | ||
from opentelemetry.metrics.time_series import MeasureTimeSeries | ||
from opentelemetry.trace import SpanContext | ||
|
||
LabelKeys = List['LabelKey'] | ||
LabelValues = List['LabelValue'] | ||
|
||
|
||
class Meter: | ||
"""An interface to allow the recording of metrics. | ||
|
||
`Metric` s are used for recording pre-defined aggregation (gauge and | ||
counter), or raw values (measure) in which the aggregation and labels | ||
for the exported metric are deferred. | ||
""" | ||
|
||
def create_float_counter(self, | ||
name: str, | ||
description: str, | ||
unit: str, | ||
label_keys: LabelKeys, | ||
span_context: SpanContext = None | ||
) -> 'CounterFloat': | ||
"""Creates a counter type metric that contains float values. | ||
|
||
Args: | ||
name: The name of the counter. | ||
description: Human readable description of the metric. | ||
unit: Unit of the metric values. | ||
label_keys: list of keys for the labels with dynamic values. | ||
Order of the list is important as the same order MUST be used | ||
on recording when suppling values for these labels. | ||
span_context: The `SpanContext` that identifies the `Span` | ||
that the metric is associated with. | ||
|
||
Returns: A new `CounterFloat` | ||
""" | ||
|
||
def create_int_counter(self, | ||
name: str, | ||
description: str, | ||
unit: str, | ||
label_keys: LabelKeys, | ||
span_context: SpanContext = None | ||
) -> 'CounterInt': | ||
"""Creates a counter type metric that contains int values. | ||
|
||
Args: | ||
name: The name of the counter. | ||
description: Human readable description of the metric. | ||
unit: Unit of the metric values. | ||
label_keys: list of keys for the labels with dynamic values. | ||
Order of the list is important as the same order MUST be used | ||
on recording when suppling values for these labels. | ||
span_context: The `SpanContext` that identifies the `Span` | ||
that the metric is associated with. | ||
|
||
Returns: | ||
A new `CounterInt` | ||
""" | ||
|
||
def create_float_gauge(self, | ||
name: str, | ||
description: str, | ||
unit: str, | ||
label_keys: LabelKeys, | ||
span_context: SpanContext = None | ||
) -> 'GaugeFloat': | ||
"""Creates a gauge type metric that contains float values. | ||
|
||
Args: | ||
name: The name of the counter. | ||
description: Human readable description of the metric. | ||
unit: Unit of the metric values. | ||
label_keys: list of keys for the labels with dynamic values. | ||
Order of the list is important as the same order MUST be used | ||
on recording when suppling values for these labels. | ||
span_context: The `SpanContext` that identifies the `Span` | ||
that the metric is associated with. | ||
|
||
Returns: | ||
A new `GaugeFloat` | ||
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. Minor personal opinion here, it might be better to use 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. Makes sense. I think I will leave this for the next PR, seeing as we are getting rid of TimeSeries and these naming changes should be done all at once for consistency. 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. In the current metrics RFC PR open-telemetry/oteps#29, I've renamed |
||
""" | ||
|
||
def create_int_gauge(self, | ||
name: str, | ||
description: str, | ||
unit: str, | ||
label_keys: LabelKeys, | ||
span_context: SpanContext = None | ||
) -> 'GaugeInt': | ||
"""Creates a gauge type metric that contains int values. | ||
|
||
Args: | ||
name: The name of the counter. | ||
description: Human readable description of the metric. | ||
unit: Unit of the metric values. | ||
label_keys: list of keys for the labels with dynamic values. | ||
Order of the list is important as the same order MUST be used | ||
on recording when suppling values for these labels. | ||
span_context: The `SpanContext` that identifies the `Span` | ||
that the metric is associated with. | ||
|
||
Returns: | ||
A new `GaugeInt` | ||
""" | ||
|
||
def create_int_measure(self, | ||
name: str, | ||
description: str, | ||
unit: str, | ||
label_keys: LabelKeys, | ||
aggregation: 'Aggregation', | ||
span_context: SpanContext = None, | ||
) -> 'MeasureInt': | ||
"""Creates a measure used to record raw int values. | ||
|
||
Args: | ||
name: The name of the measure. | ||
description: Human readable description of this measure. | ||
unit: Unit of the measure values. | ||
label_keys: list of keys for the labels with dynamic values. | ||
Order of the list is important as the same order MUST be used | ||
on recording when suppling values for these labels. | ||
aggregation: The type of aggregation to use for this metric. | ||
span_context: The `SpanContext` that identifies the `Span` | ||
that the metric is associated with. | ||
|
||
Returns: | ||
A new `MeasureInt` | ||
""" | ||
|
||
def create_float_measure(self, | ||
name: str, | ||
description: str, | ||
unit: str, | ||
label_keys: LabelKeys, | ||
aggregation: 'Aggregation', | ||
span_context: SpanContext = None, | ||
) -> 'MeasureFloat': | ||
"""Creates a Measure used to record raw float values. | ||
|
||
Args: | ||
name: the name of the measure | ||
description: Human readable description of this measure. | ||
unit: Unit of the measure values. | ||
label_keys: list of keys for the labels with dynamic values. | ||
Order of the list is important as the same order MUST be used | ||
on recording when suppling values for these labels. | ||
aggregation: The type of aggregation to use for this metric. | ||
span_context: The `SpanContext` that identifies the `Span` | ||
that the metric is associated with. | ||
|
||
Returns: | ||
A new `MeasureFloat` | ||
""" | ||
|
||
|
||
class Metric(ABC): | ||
"""Base class for various types of metrics. | ||
|
||
Metric class that inherit from this class are specialized with the type of | ||
time series that the metric holds. Metric is constructed from the meter. | ||
""" | ||
|
||
@abstractmethod | ||
def get_or_create_time_series(self, | ||
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. this is probably for the RFC or OTel spec, but is there a better description than timeseries? I think most metric systems don't expose a nuance between a timeseries and an individual logging of a measure. 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. I think the best explanation for the name "timeseries" is that something got lost in the translation from OC code to the OT spec.
I may be missing something here, but I don't see a reason that these should still be called "time series". |
||
label_values: LabelValues | ||
) -> 'object': | ||
"""Gets and returns a timeseries, a container for a cumulative value. | ||
|
||
If the provided label values are not already associated with this | ||
metric, a new timeseries is returned, otherwise it returns the existing | ||
timeseries with the exact label values. The timeseries returned | ||
contains logic and behaviour specific to the type of metric that | ||
overrides this function. | ||
|
||
Args: | ||
label_values: A map of `LabelValue` s that will be associated | ||
with the return timeseries. | ||
""" | ||
|
||
def remove_time_series(self, | ||
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. I wonder if 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. I have no problem with this. 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. Might revisit this, if we decide to remove 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. If we treat Another argument for keeping this signature: We expect users to call this to stop reporting a metric for a given set of label values. If they've got the time series they can use its label values without making another call to |
||
label_values: LabelValues) -> None: | ||
"""Removes the timeseries from the Metric, if present. | ||
|
||
The timeseries with matching `LabelValue` s will be removed. | ||
|
||
args: | ||
label_values: The list of label values to match against. | ||
""" | ||
|
||
def clear(self) -> None: | ||
"""Removes all timeseries from the `Metric`.""" | ||
|
||
|
||
class CounterFloat(Metric): | ||
"""A counter type metric that holds float values. | ||
|
||
Cumulative values can go up or stay the same, but can never go down. | ||
Cumulative values cannot be negative. | ||
""" | ||
|
||
def get_or_create_time_series(self, | ||
label_values: LabelValues | ||
) -> 'CounterTimeSeries': | ||
"""Gets a `CounterTimeSeries` with a cumulative float value.""" | ||
|
||
|
||
class CounterInt(Metric): | ||
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. Just out of curiosity: how will these APIs be used in practice? I kind of imagine that we wouldn't hook in different implementations of a counter or gauge: the behavior feels pretty clear and might be more beneficial to remain consistent. Again an example that makes opentelemetry-sdk basically a must to use opentelemetry-api 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 see this problem with almost every class in the metrics package. Tracing has separate API and SDK packages largely because vendors want to be able to change the implementation. Metrics has separate API and SDK packages to be consistent with tracing, but if (e.g.) vendors are going to change metrics, they're much more likely to add new aggregations or metric types than to change these built-in classes. This touches on a deeper issue regarding the API/SDK separation. The API already includes some implementation to support context propagation without an SDK, so how do we decide where to draw the line? 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. Sounds like a great question to discuss in the specification. I agree with all the points you've raised here. 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. The current metrics RFC discusses the semantic connection between metrics APIs and trace context. The current specification describes three kinds of metric "instrument", cumulative, gauge, and measure, which are pure API objects. Handles (known as By separating the API from the implementation, it should be possible for to define an exporter for direct integration with existing metrics client libraries. This will require some sort of new metrics event exporter to be specified. |
||
"""A counter type metric that holds int values. | ||
|
||
Cumulative values can go up or stay the same, but can never go down. | ||
Cumulative values cannot be negative. | ||
""" | ||
|
||
def get_or_create_time_series(self, | ||
label_values: LabelValues | ||
) -> 'CounterTimeSeries': | ||
"""Gets a `CounterTimeSeries` with a cumulative int value.""" | ||
|
||
|
||
class GaugeFloat(Metric): | ||
"""A gauge type metric that holds float values. | ||
|
||
Cumulative value can go both up and down. Values can be negative. | ||
""" | ||
|
||
def get_or_create_time_series(self, | ||
label_values: LabelValues | ||
) -> 'GaugeTimeSeries': | ||
"""Gets a `GaugeTimeSeries` with a cumulative float value.""" | ||
|
||
|
||
class GaugeInt(Metric): | ||
"""A gauge type metric that holds int values. | ||
|
||
Cumulative value can go both up and down. Values can be negative. | ||
""" | ||
|
||
def get_or_create_time_series(self, | ||
label_values: LabelValues | ||
) -> 'GaugeTimeSeries': | ||
"""Gets a `GaugeTimeSeries` with a cumulative int value.""" | ||
|
||
|
||
class MeasureFloat(Metric): | ||
"""A measure type metric that holds float values. | ||
|
||
Measure metrics represent raw statistics that are recorded. | ||
""" | ||
|
||
def get_or_create_time_series(self, | ||
label_values: LabelValues | ||
) -> 'MeasureTimeSeries': | ||
"""Gets a `MeasureTimeSeries` with a cumulated float value.""" | ||
|
||
|
||
class MeasureInt(Metric): | ||
"""A measure type metric that holds int values. | ||
|
||
Measure metrics represent raw statistics that are recorded. | ||
""" | ||
|
||
def get_or_create_time_series(self, | ||
label_values: LabelValues | ||
) -> 'MeasureTimeSeries': | ||
"""Gets a `MeasureTimeSeries` with a cumulated int value.""" | ||
|
||
|
||
class MeasureBatch: | ||
|
||
def record(self, | ||
metrics: List['Metric'], | ||
values: List[Union[float, int]]) -> None: | ||
"""Records multiple observed values simultaneously. | ||
|
||
Args: | ||
metric: A list containing the `Metric` s to be recorded | ||
values: A list containing the values to record | ||
""" | ||
|
||
|
||
class LabelKey: | ||
"""The label keys associated with the metric. | ||
|
||
:type key: str | ||
:param key: the key for the label | ||
|
||
:type description: str | ||
:param description: description of the label | ||
""" | ||
def __init__(self, | ||
key: str, | ||
description: str) -> None: | ||
self.key = key | ||
self.description = description | ||
|
||
|
||
class LabelValue: | ||
"""The label values associated with a TimeSeries. | ||
|
||
:type value: str | ||
:param value: the value for the label | ||
""" | ||
def __init__(self, | ||
value: str) -> None: | ||
self.value = value | ||
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. I wonder if there's any good reasons to have mandatory containers for label values? Currently it seems to be engineered like that just for the sake of completeness. As far as I understand majority of industry solutions, there's no support for anything but strings for labels, and it's unlikely to change in the years to come. 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. This is good point and a common ask. I think it will make sense to simply use strings for the label values. 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. I think each language should decide how to handle this question. If there is some kind of built-in support for key-value in the language, they should prefer that. In the Go repository, we have a common I'm not sure the API needs to specify anything about how a vendor should behave when, say, a numeric value is passed as the value for a label (and the compiler didn't prevent it). I would say the default behavior should coerce values to a string silently, not something to worry about. |
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.
thoughts on passing in the float or int as types to the create method?
Might also be a good use case for enums:
def create(self, metric_type=METRIC_TYPE.counter, data_type=float, ...)