-
Notifications
You must be signed in to change notification settings - Fork 624
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
Implement LabelSet for metrics #258
Changes from 15 commits
3629f5a
6a433fe
d816da8
df80425
c960056
893640a
b3985bd
7e9e7ad
f2842ff
f390f87
4c050d2
2fe06ca
30e5e5a
f375128
ce33371
792449e
03e072c
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 |
---|---|---|
|
@@ -13,14 +13,23 @@ | |
# limitations under the License. | ||
|
||
import logging | ||
from typing import Sequence, Tuple, Type | ||
from collections import OrderedDict | ||
from typing import Dict, Sequence, Tuple, Type | ||
|
||
from opentelemetry import metrics as metrics_api | ||
from opentelemetry.util import time_ns | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
# pylint: disable=redefined-outer-name | ||
class LabelSet(metrics_api.LabelSet): | ||
"""See `opentelemetry.metrics.LabelSet.""" | ||
|
||
def __init__(self, labels: Dict[str, str] = None): | ||
self.labels = labels | ||
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. Is it required to use a particular LabelSet implementation with a specific Meter implementation? I notice that there's no methods or properties for the API, but this is exposing the "labels" and "encoded" properties which are being utilized. 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. That is a good question. Yes, a particular LabelSet implementation should only work with a specific Meter implementation. I've included validation in the metric methods for this. If they do not match, we return the empty label set. |
||
|
||
|
||
class BaseHandle: | ||
def __init__( | ||
self, | ||
|
@@ -107,14 +116,14 @@ def __init__( | |
self.monotonic = monotonic | ||
self.handles = {} | ||
|
||
def get_handle(self, label_values: Sequence[str]) -> BaseHandle: | ||
def get_handle(self, label_set: LabelSet) -> BaseHandle: | ||
"""See `opentelemetry.metrics.Metric.get_handle`.""" | ||
handle = self.handles.get(label_values) | ||
handle = self.handles.get(label_set) | ||
if not handle: | ||
handle = self.HANDLE_TYPE( | ||
self.value_type, self.enabled, self.monotonic | ||
) | ||
self.handles[label_values] = handle | ||
self.handles[label_set] = handle | ||
return handle | ||
|
||
def __repr__(self): | ||
|
@@ -155,11 +164,9 @@ def __init__( | |
monotonic=monotonic, | ||
) | ||
|
||
def add( | ||
self, label_values: Sequence[str], value: metrics_api.ValueT | ||
) -> None: | ||
def add(self, label_set: LabelSet, value: metrics_api.ValueT) -> None: | ||
"""See `opentelemetry.metrics.Counter.add`.""" | ||
self.get_handle(label_values).add(value) | ||
self.get_handle(label_set).add(value) | ||
|
||
UPDATE_FUNCTION = add | ||
|
||
|
@@ -193,11 +200,9 @@ def __init__( | |
monotonic=monotonic, | ||
) | ||
|
||
def set( | ||
self, label_values: Sequence[str], value: metrics_api.ValueT | ||
) -> None: | ||
def set(self, label_set: LabelSet, value: metrics_api.ValueT) -> None: | ||
"""See `opentelemetry.metrics.Gauge.set`.""" | ||
self.get_handle(label_values).set(value) | ||
self.get_handle(label_set).set(value) | ||
|
||
UPDATE_FUNCTION = set | ||
|
||
|
@@ -231,26 +236,31 @@ def __init__( | |
monotonic=monotonic, | ||
) | ||
|
||
def record( | ||
self, label_values: Sequence[str], value: metrics_api.ValueT | ||
) -> None: | ||
def record(self, label_set: LabelSet, value: metrics_api.ValueT) -> None: | ||
"""See `opentelemetry.metrics.Measure.record`.""" | ||
self.get_handle(label_values).record(value) | ||
self.get_handle(label_set).record(value) | ||
|
||
UPDATE_FUNCTION = record | ||
|
||
|
||
# Used when getting a LabelSet with no key/values | ||
EMPTY_LABEL_SET = LabelSet() | ||
|
||
|
||
class Meter(metrics_api.Meter): | ||
"""See `opentelemetry.metrics.Meter`.""" | ||
|
||
def __init__(self): | ||
self.labels = {} | ||
|
||
def record_batch( | ||
self, | ||
label_values: Sequence[str], | ||
label_set: LabelSet, | ||
record_tuples: Sequence[Tuple[metrics_api.Metric, metrics_api.ValueT]], | ||
) -> None: | ||
"""See `opentelemetry.metrics.Meter.record_batch`.""" | ||
for metric, value in record_tuples: | ||
metric.UPDATE_FUNCTION(label_values, value) | ||
metric.UPDATE_FUNCTION(label_set, value) | ||
|
||
def create_metric( | ||
self, | ||
|
@@ -275,5 +285,22 @@ def create_metric( | |
monotonic=monotonic, | ||
) | ||
|
||
def get_label_set(self, labels: Dict[str, str]): | ||
"""See `opentelemetry.metrics.Meter.create_metric`. | ||
lzchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This implementation encodes the labels to use as a map key. | ||
|
||
Args: | ||
labels: The dictionary of label keys to label values. | ||
""" | ||
if len(labels) == 0: | ||
return EMPTY_LABEL_SET | ||
# Use simple encoding for now until encoding API is implemented | ||
encoded = tuple(sorted(labels.items())) | ||
# If LabelSet exists for this meter in memory, use existing one | ||
if encoded not in self.labels: | ||
self.labels[encoded] = LabelSet(labels=labels) | ||
return self.labels[encoded] | ||
|
||
|
||
meter = Meter() |
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.
Is this required? Should LabelSet be made an ABC?
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.
Just like how
metric
andhandle
have default implementations, we don't want to have themeter
functions to return None objects (it would beget_label_set
in this case). Also a LabelSet has no methods so I didn't think it was necessary to use an ABC (unless my understanding of the benefits is mistaken).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.
My point here is that since
LabelSet
is not an ABC we technically don't needDefaultLabelSet
since we could return a plainLabelSet
instance fromMeter.get_label_set
.