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

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
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants