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

Metrics should be gettable by name #20878

Merged
merged 2 commits into from
Sep 27, 2021
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
1 change: 1 addition & 0 deletions sdk/monitor/azure-monitor-query/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Added `status` attribute to `LogsQueryResult`.
- Added `LogsQueryStatus` Enum to describe the status of a result.
- Added a new `LogsTableRow` type that represents a single row in a table.
- Items in `metrics` list in `MetricsResult` can now be accessed by metric names.

### Breaking Changes

Expand Down
36 changes: 33 additions & 3 deletions sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,15 @@ def _from_generated(cls, generated):
)


class LogsTableRow(object):
class LogsTableRow(list):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it by design we want to make row as a list?

Shouldn't it a dict? (use col name to access)

Copy link
Contributor Author

@rakshith91 rakshith91 Sep 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - it's by design that we make this a list. It's basically a custom list where you can access by a column label which is a different list by itself.

This is normally consumed as a list of lists on the response.

Also, this way, we are able to pass the type directly into pandas, so there's that too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to update the information in changelog? (and for the new model MetricsList as well)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I purposely didn't add it in changelog. There's no behavior difference from a user perspective.
And for the MetricList, for now, I'm documenting it as a list since it's read only.

"""Represents a single row in logs table.

:ivar int index: The index of the row in the table
"""

def __init__(self, **kwargs):
# type: (Any) -> None
super(LogsTableRow, self).__init__(**kwargs)
_col_types = kwargs["col_types"]
row = kwargs["row"]
self._row = process_row(_col_types, row)
Expand All @@ -76,6 +77,12 @@ def __iter__(self):
"""This will iterate over the row directly."""
return iter(self._row)

def __len__(self):
return len(self._row)

def __repr__(self):
return repr(self._row)

def __getitem__(self, column):
"""This type must be subscriptable directly to row.
Must be gettableby both column name and row index
Expand Down Expand Up @@ -131,11 +138,34 @@ def _from_generated(cls, generated):
granularity=generated.interval,
namespace=generated.namespace,
resource_region=generated.resourceregion,
metrics=[
metrics=MetricsList(metrics=[
Metric._from_generated(m) for m in generated.value # pylint: disable=protected-access
],
]),
)

class MetricsList(list):
"""Custom list for metrics
"""
def __init__(self, **kwargs):
super(MetricsList, self).__init__(**kwargs)
self._metrics = kwargs['metrics']
self._metric_names = {val.name: ind for ind, val in enumerate(self._metrics)}

def __iter__(self):
return iter(self._metrics)

def __len__(self):
return len(self._metrics)

def __repr__(self):
return repr(self._metrics)

def __getitem__(self, metric):
try:
return self._metrics[metric]
except TypeError: # TypeError: list indices must be integers or slices, not str
return self._metrics[self._metric_names[metric]]


class LogsBatchQuery(object):
"""A single request in a batch.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import pytest
import os
from azure.identity.aio import ClientSecretCredential
from azure.monitor.query import MetricAggregationType
from azure.monitor.query import MetricAggregationType, Metric
from azure.monitor.query.aio import MetricsQueryClient

def _credential():
Expand Down Expand Up @@ -42,6 +42,26 @@ async def test_metrics_granularity():
assert response
assert response.granularity == timedelta(minutes=5)


@pytest.mark.live_test_only
@pytest.mark.asyncio
async def test_metrics_list():
credential = _credential()
client = MetricsQueryClient(credential)
response = await client.query_resource(
os.environ['METRICS_RESOURCE_URI'],
metric_names=["MatchedEventCount"],
timespan=timedelta(days=1),
granularity=timedelta(minutes=5),
aggregations=[MetricAggregationType.COUNT]
)
assert response
metrics = response.metrics
assert len(metrics) == 1
assert metrics[0].__class__ == Metric
assert metrics['MatchedEventCount'].__class__ == Metric
assert metrics['MatchedEventCount'] == metrics[0]

@pytest.mark.live_test_only
@pytest.mark.asyncio
async def test_metrics_namespaces():
Expand Down
20 changes: 19 additions & 1 deletion sdk/monitor/azure-monitor-query/tests/test_metrics_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import os
from datetime import datetime, timedelta
from azure.identity import ClientSecretCredential
from azure.monitor.query import MetricsQueryClient, MetricAggregationType
from azure.monitor.query import MetricsQueryClient, MetricAggregationType, Metric

def _credential():
credential = ClientSecretCredential(
Expand Down Expand Up @@ -39,6 +39,24 @@ def test_metrics_granularity():
assert response
assert response.granularity == timedelta(minutes=5)

@pytest.mark.live_test_only
def test_metrics_list():
credential = _credential()
client = MetricsQueryClient(credential)
response = client.query_resource(
os.environ['METRICS_RESOURCE_URI'],
metric_names=["MatchedEventCount"],
timespan=timedelta(days=1),
granularity=timedelta(minutes=5),
aggregations=[MetricAggregationType.COUNT]
)
assert response
metrics = response.metrics
assert len(metrics) == 1
assert metrics[0].__class__ == Metric
assert metrics['MatchedEventCount'].__class__ == Metric
assert metrics['MatchedEventCount'] == metrics[0]

@pytest.mark.live_test_only
def test_metrics_namespaces():
client = MetricsQueryClient(_credential())
Expand Down