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

Update add_metric API to be backwards compatible #2525

Merged
merged 4 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 12 additions & 7 deletions ts/metrics/caching_metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from ts.metrics.dimension import Dimension
from ts.metrics.metric_abstract import MetricAbstract
from ts.metrics.metric_type_enum import MetricTypes

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -36,7 +37,7 @@ def __init__(
unit can be one of ms, percent, count, MB, GB or a generic string

dimension_names list
list of dimension names which should be strings
list of dimension name strings

metric_type MetricTypes
Type of metric Counter, Gauge, Histogram
Expand All @@ -57,9 +58,11 @@ def _validate_and_get_dimensions(
values corresponding to the metrics dimension names
Returns
-------
list of dimension objects or ValueError
list of Dimension objects or ValueError
"""
if dimension_values is None or len(dimension_values) != len(self.dimension_names):
if dimension_values is None or len(dimension_values) != len(
self.dimension_names
):
raise ValueError(
f"Dimension values: {dimension_values} should "
f"correspond to Dimension names: {self.dimension_names}"
Expand Down Expand Up @@ -97,8 +100,10 @@ def emit_metrics(
value
dimension_string
"""
metric_str = f"[METRICS]{self.metric_name}.{self.unit}:{value}|#{dimension_string}|" \
f"#hostname:{socket.gethostname()},{int(time.time())}"
metric_str = (
f"[METRICS]{self.metric_name}.{self.unit}:{value}|#{dimension_string}|"
f"#hostname:{socket.gethostname()},{int(time.time())}"
)
if request_id:
logger.info(f"{metric_str},{request_id}")
else:
Expand All @@ -118,7 +123,7 @@ def add_or_update(
value : int, float
metric to be updated
dimension_values : list
list of dimension values
list of dimension value strings
request_id : str
request id to be associated with the metric
"""
Expand Down Expand Up @@ -152,7 +157,7 @@ def update(
request_id : str
request id to be associated with the metric
dimensions : list
list of dimension values
list of Dimension objects
"""
logger.warning("Overriding existing dimensions")
self.dimension_names = [dim.name for dim in dimensions]
Expand Down
12 changes: 8 additions & 4 deletions ts/metrics/metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import time
from builtins import str
from collections import OrderedDict

from ts.metrics.caching_metric import CachingMetric
from ts.metrics.metric_type_enum import MetricTypes

from ts.metrics.unit import Units

MetricUnit = Units()
Expand Down Expand Up @@ -41,7 +41,7 @@ def __init__(
unit: str
unit can be one of ms, percent, count, MB, GB or a generic string
dimensions: list
list of dimension objects
list of Dimension objects
request_id: str
req_id of metric
metric_method: str
Expand Down Expand Up @@ -73,13 +73,17 @@ def update(self, value):
value : int, float
metric to be updated
"""
self._caching_metric.add_or_update(value, self.dimension_values, request_id=self.request_id)
self._caching_metric.add_or_update(
value, self.dimension_values, request_id=self.request_id
)

def reset(self):
"""
Reset Metric value to 0
"""
self._caching_metric.add_or_update(0, self.dimension_values, request_id=self.request_id)
self._caching_metric.add_or_update(
0, self.dimension_values, request_id=self.request_id
)

def __str__(self):
dims = ",".join([str(d) for d in self.dimensions])
Expand Down
5 changes: 3 additions & 2 deletions ts/metrics/metric_abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
Interface for metric class for TS
"""
import abc
from ts.metrics.unit import Units

from ts.metrics.metric_type_enum import MetricTypes
from ts.metrics.unit import Units

MetricUnit = Units()

Expand Down Expand Up @@ -33,7 +34,7 @@ def __init__(
unit can be one of ms, percent, count, MB, GB or a generic string

dimension_names list
list of dimension names which should be strings
list of dimension name strings

metric_type MetricTypes
Type of metric Counter, Gauge, Histogram
Expand Down
65 changes: 51 additions & 14 deletions ts/metrics/metric_cache_abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"""
import abc
import os
import ts.metrics.metric_cache_errors as merrors

import ts.metrics.metric_cache_errors as merrors
from ts.metrics.dimension import Dimension
from ts.metrics.metric_abstract import MetricAbstract
from ts.metrics.metric_type_enum import MetricTypes
Expand All @@ -27,15 +27,17 @@ def __init__(self, config_file_path):
Name of file to be parsed

"""
self.cache = dict()
self.cache = {}
self.store = []
self.request_ids = None
self.model_name = None
self.config_file_path = config_file_path
try:
os.path.exists(self.config_file_path)
except Exception as exc:
raise merrors.MetricsCacheTypeError(f"Error loading {config_file_path} file: {exc}")
raise merrors.MetricsCacheTypeError(
f"Error loading {config_file_path} file: {exc}"
)

def _add_default_dims(self, idx, dimensions):
dim_names = [dim.name for dim in dimensions]
Expand Down Expand Up @@ -63,11 +65,44 @@ def _get_req(self, idx):
# check if request id for the metric is given, if so use it else have a list of all.
req_id = self.request_ids
if isinstance(req_id, dict):
req_id = ','.join(self.request_ids.values())
req_id = ",".join(self.request_ids.values())
if idx is not None and self.request_ids is not None and idx in self.request_ids:
req_id = self.request_ids[idx]
return req_id

def add_metric(
self,
name: str,
value: int or float,
unit: str,
idx: str = None,
dimensions: list = [],
metric_type: MetricTypes = MetricTypes.COUNTER,
):
"""
Add a generic metric
Default metric type is counter

Parameters
----------
name : str
metric name
value: int or float
value of the metric
unit: str
unit of metric
idx: str
request id to be associated with the metric
dimensions: list
list of Dimension objects for the metric
metric_type MetricTypes
Type of metric Counter, Gauge, Histogram
"""
req_id = self._get_req(idx)
dimensions = self._add_default_dims(req_id, dimensions)
metric = self._get_or_add_metric(name, unit, dimensions, metric_type)
metric.add_or_update(value, [dim.value for dim in dimensions], req_id)

def add_counter(
self,
name: str,
Expand All @@ -87,7 +122,7 @@ def add_counter(
idx: str
request id to be associated with the metric
dimensions: list
list of dimension names for the metric
list of Dimension objects for the metric
"""
req_id = self._get_req(idx)
dimensions = self._add_default_dims(req_id, dimensions)
Expand Down Expand Up @@ -118,7 +153,7 @@ def add_time(
unit: str
unit of metric, default here is ms, s is also accepted
dimensions: list
list of dimension names for the metric
list of Dimension objects for the metric
metric_type MetricTypes
Type of metric Counter, Gauge, Histogram
"""
Expand Down Expand Up @@ -155,7 +190,7 @@ def add_size(
unit: str
unit of metric, default here is 'MB', 'kB', 'GB' also supported
dimensions: list
list of dimensions for the metric
list of Dimension objects for the metric
metric_type MetricTypes
Type of metric Counter, Gauge, Histogram
"""
Expand Down Expand Up @@ -189,7 +224,7 @@ def add_percent(
idx: str
request id to be associated with the metric
dimensions: list
list of dimensions for the metric
list of Dimension objects for the metric
metric_type MetricTypes
Type of metric Counter, Gauge, Histogram
"""
Expand All @@ -215,17 +250,19 @@ def add_error(
value: int or float
value of the metric
dimensions: list
list of dimension objects for the metric
list of Dimension objects for the metric
"""
dimensions = self._add_default_dims(None, dimensions)
metric = self._get_or_add_metric(name, "", dimensions, MetricTypes.COUNTER)
metric.add_or_update(value, [dim.value for dim in dimensions])

def _get_or_add_metric(self, metric_name, unit, dimensions, metric_type) -> MetricAbstract:
def _get_or_add_metric(
self, metric_name, unit, dimensions, metric_type
) -> MetricAbstract:
try:
metric = self.get_metric(metric_name, metric_type)
except merrors.MetricsCacheKeyError:
metric = self.add_metric(
metric = self.add_metric_to_cache(
metric_name=metric_name,
unit=unit,
metric_type=metric_type,
Expand Down Expand Up @@ -269,11 +306,11 @@ def get_metric(
pass

@abc.abstractmethod
def add_metric(
def add_metric_to_cache(
self,
metric_name: str,
unit: str,
dimension_names: list = None,
dimension_names: list = [],
metric_type: MetricTypes = MetricTypes.COUNTER,
) -> MetricAbstract:
"""
Expand All @@ -287,7 +324,7 @@ def add_metric(
unit: str
unit of metric
dimension_names: list
list of dimensions for the metric
list of dimension name strings for the metric
metric_type: MetricTypes
Type of metric

Expand Down
37 changes: 26 additions & 11 deletions ts/metrics/metric_cache_yaml_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
Metrics Cache class for creating objects from yaml spec
"""
import logging

import yaml

import ts.metrics.metric_cache_errors as merrors
from ts.metrics.caching_metric import CachingMetric
from ts.metrics.metric_cache_abstract import MetricCacheAbstract
from ts.metrics.metric_type_enum import MetricTypes

logger = logging.getLogger(__name__)


Expand All @@ -34,7 +36,8 @@ def _parse_yaml_file(self, config_file_path) -> None:

try:
self._parsed_file = yaml.safe_load(
open(config_file_path, "r", encoding="utf-8"))
open(config_file_path, "r", encoding="utf-8")
)
logging.info(f"Successfully loaded {config_file_path}.")
except yaml.YAMLError as exc:
raise merrors.MetricsCachePyYamlError(
Expand Down Expand Up @@ -73,7 +76,9 @@ def initialize_cache(self) -> None:
"""
metrics_section = self._parse_metrics_section("model_metrics")
if not metrics_section:
raise merrors.MetricsCacheValueError("Missing `model_metrics` specification")
raise merrors.MetricsCacheValueError(
"Missing `model_metrics` specification"
)
for metric_type, metrics_list in metrics_section.items():
try:
metric_enum = MetricTypes(metric_type)
Expand All @@ -85,16 +90,18 @@ def initialize_cache(self) -> None:
metric_name = metric["name"]
unit = metric["unit"]
dimension_names = metric["dimensions"]
self.add_metric(
self.add_metric_to_cache(
metric_name=metric_name,
unit=unit,
dimension_names=dimension_names,
metric_type=metric_enum,
)
except KeyError as k_err:
raise merrors.MetricsCacheKeyError(f"Key not found in cache spec: {k_err}")
raise merrors.MetricsCacheKeyError(
f"Key not found in cache spec: {k_err}"
)

def add_metric(
def add_metric_to_cache(
self,
metric_name: str,
unit: str,
Expand All @@ -111,7 +118,7 @@ def add_metric(
unit str
unit can be one of ms, percent, count, MB, GB or a generic string
dimension_names list
list of dimension keys which should be strings, or the complete log of dimensions
list of dimension name strings for the metric
metric_type MetricTypes
Type of metric Counter, Gauge, Histogram
Returns
Expand All @@ -120,16 +127,22 @@ def add_metric(
"""
self._check_type(metric_name, str, "`metric_name` must be a str")
self._check_type(unit, str, "`unit` must be a str")
self._check_type(metric_type, MetricTypes, "`metric_type` must be a MetricTypes enum")
self._check_type(
metric_type, MetricTypes, "`metric_type` must be a MetricTypes enum"
)
if dimension_names:
self._check_type(dimension_names, list, "`dimension_names` should be a list of dimension name strings")
self._check_type(
dimension_names,
list,
"`dimension_names` should be a list of dimension name strings",
)
if metric_type not in self.cache.keys():
self.cache[metric_type] = dict()
self.cache[metric_type] = {}
metric = CachingMetric(
metric_name=metric_name,
unit=unit,
dimension_names=dimension_names,
metric_type=metric_type
metric_type=metric_type,
)
if metric_name in self.cache[metric_type].keys():
logging.warning(f"Overriding existing key {metric_type}:{metric_name}")
Expand Down Expand Up @@ -157,7 +170,9 @@ def get_metric(
Metrics object or MetricsCacheKeyError if not found
"""
self._check_type(metric_name, str, "`metric_name` must be a str")
self._check_type(metric_type, MetricTypes, "`metric_type` must be a MetricTypes enum")
self._check_type(
metric_type, MetricTypes, "`metric_type` must be a MetricTypes enum"
)
try:
metric = self.cache[metric_type][metric_name]
except KeyError:
Expand Down
Loading