Skip to content
This repository was archived by the owner on Aug 7, 2025. It is now read-only.

Conversation

@namannandan
Copy link
Collaborator

@namannandan namannandan commented Aug 11, 2023

Description

Update the custom metrics add_metric API to be backwards compatible with Torchserve versions prior to v0.6.1.

The add_metric method retains the same API signature but the change in semantics is that v0.6.1 introduced types for metrics metricTypes and the default inferred type if not specified when using add_metric API is COUNTER.

add_metric API in versions prior to v0.6.1

 def add_metric(self, name, value, unit, idx=None, dimensions=None)

add_metric API in versions v0.6.1 onwards

    def add_metric(
        self,
        metric_name: str,
        unit: str,
        dimension_names: list = [],
        metric_type: MetricTypes = MetricTypes.COUNTER,
    ) -> CachingMetric

add_metric API introduced in this PR for backwards compatibility with versions prior to v0.6.1

    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
        """

# add_metric method from version v0.6.1 onwards renamed to add_metric_to_cache
    def add_metric_to_cache(
        self,
        metric_name: str,
        unit: str,
        dimension_names: list = [],
        metric_type: MetricTypes = MetricTypes.COUNTER,
    ) -> CachingMetric

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Feature/Issue validation/testing

  • Unit tests included in this PR run as part of CI

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #2525 (f174738) into master (96450b9) will increase coverage by 0.19%.
The diff coverage is 99.14%.

❗ Current head f174738 differs from pull request most recent head f5bcf4e. Consider uploading reports for the commit f5bcf4e to get more accurate results

@@            Coverage Diff             @@
##           master    #2525      +/-   ##
==========================================
+ Coverage   72.66%   72.85%   +0.19%     
==========================================
  Files          78       78              
  Lines        3669     3695      +26     
  Branches       58       58              
==========================================
+ Hits         2666     2692      +26     
  Misses        999      999              
  Partials        4        4              
Files Changed Coverage Δ
ts/metrics/metric.py 80.64% <50.00%> (ø)
ts/metrics/caching_metric.py 89.47% <100.00%> (ø)
ts/metrics/metric_abstract.py 92.85% <100.00%> (ø)
ts/metrics/metric_cache_abstract.py 93.18% <100.00%> (+0.41%) ⬆️
ts/metrics/metric_cache_yaml_impl.py 93.24% <100.00%> (ø)
...sts/metrics_yaml_testing/metric_cache_unit_test.py 99.68% <100.00%> (+0.02%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@msaroufim
Copy link
Member

cc @duk0011

@msaroufim msaroufim marked this pull request as ready for review August 17, 2023 00:13
@msaroufim
Copy link
Member

LGTM feel free to merge when ready @namannandan

@namannandan
Copy link
Collaborator Author

Documentation and example update corresponding to the API change in this PR will be included in PR: #2516

@namannandan namannandan merged commit 58eb2d2 into pytorch:master Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants