Skip to content

Commit

Permalink
Get rid of LogsBatchQueryResult (Azure#20418)
Browse files Browse the repository at this point in the history
* Get rid of LogsBatchQueryResult

* Update sdk/monitor/azure-monitor-query/README.md

* oops

* comments
  • Loading branch information
Rakshith Bhyravabhotla authored and hildurhodd committed Aug 30, 2021
1 parent 4f84957 commit 93549d1
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 91 deletions.
4 changes: 2 additions & 2 deletions sdk/monitor/azure-monitor-query/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
- Rename `batch_query` to `query_batch`.
- Rename `LogsBatchQueryRequest` to `LogsBatchQuery`.
- `include_render` is now renamed to `include_visualization` in the query API.
- `LogsQueryResult` and `LogsBatchQueryResult` now return `visualization` instead of `render`.
- `LogsQueryResult` now returns `visualization` instead of `render`.
- `start_time`, `duration` and `end_time` are now replaced with a single param called `timespan`
- `resourceregion` is renamed to `resource_region` in the MetricResult type.
- `top` is renamed to `max_results` in the metric's `query` API.
Expand All @@ -30,11 +30,11 @@
- `AggregationType` is renamed to `MetricAggregationType`.
- Removed `LogsBatchResultError` type.
- `LogsQueryResultTable` is named to `LogsTable`
- `LogsQueryResultColumn` is renamed to `LogsTableColumn`
- `LogsTableColumn` is now removed. Column labels are strings instead.
- `start_time` in `list_metric_namespaces` API is now a datetime.
- The order of params in `LogsBatchQuery` is changed. Also, `headers` is no longer accepted.
- `timespan` is now a required keyword-only argument in logs APIs.
- batch api now returns a list of `LogsQueryResult` objects.

### Bugs Fixed

Expand Down
6 changes: 2 additions & 4 deletions sdk/monitor/azure-monitor-query/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,12 @@ for rsp in response:

#### Handling the response for Logs Query

The `query` API returns the `LogsQueryResult` while the `batch_query` API returns the `LogsBatchQueryResult`.
The `query` API returns the `LogsQueryResult` while the `batch_query` API returns list of `LogsQueryResult`.

Here is a heirarchy of the response:

```
LogsQueryResult / LogsBatchQueryResult
|---id (this exists in `LogsBatchQueryResult` object only)
|---status (this exists in `LogsBatchQueryResult` object only)
LogsQueryResult
|---statistics
|---visualization
|---error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

from ._models import (
MetricAggregationType,
LogsBatchQueryResult,
LogsQueryResult,
LogsTable,
MetricsResult,
Expand All @@ -30,7 +29,6 @@
__all__ = [
"MetricAggregationType",
"LogsQueryClient",
"LogsBatchQueryResult",
"LogsQueryResult",
"LogsTable",
"LogsBatchQuery",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,9 @@ def process_error(exception):
raise_error = HttpResponseError
raise raise_error(message=exception.message, response=exception.response)

def order_results(request_order, responses):
mapping = {item.id: item for item in responses}
def order_results(request_order, mapping, obj):
ordered = [mapping[id] for id in request_order]
return ordered
return [obj._from_generated(rsp) for rsp in ordered] # pylint: disable=protected-access

def construct_iso8601(timespan=None):
if not timespan:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from ._generated.models import BatchRequest, QueryBody as LogsQueryBody
from ._helpers import get_authentication_policy, process_error, construct_iso8601, order_results
from ._models import LogsQueryResult, LogsBatchQuery, LogsBatchQueryResult
from ._models import LogsBatchQuery, LogsQueryResult

if TYPE_CHECKING:
from azure.core.credentials import TokenCredential
Expand Down Expand Up @@ -128,16 +128,16 @@ def query(self, workspace_id, query, **kwargs):

@distributed_trace
def query_batch(self, queries, **kwargs):
# type: (Union[Sequence[Dict], Sequence[LogsBatchQuery]], Any) -> List[LogsBatchQueryResult]
# type: (Union[Sequence[Dict], Sequence[LogsBatchQuery]], Any) -> List[LogsQueryResult]
"""Execute a list of analytics queries. Each request can be either a LogQueryRequest
object or an equivalent serialized model.
The response is returned in the same order as that of the requests sent.
:param queries: The list of queries that should be processed
:type queries: list[dict] or list[~azure.monitor.query.LogsBatchQuery]
:return: List of LogsBatchQueryResult, or the result of cls(response)
:rtype: list[~azure.monitor.query.LogsBatchQueryResult]
:return: List of LogsQueryResult, or the result of cls(response)
:rtype: list[~azure.monitor.query.LogsQueryResult]
:raises: ~azure.core.exceptions.HttpResponseError
.. admonition:: Example:
Expand All @@ -160,11 +160,8 @@ def query_batch(self, queries, **kwargs):
request_order = [req['id'] for req in queries]
batch = BatchRequest(requests=queries)
generated = self._query_op.batch(batch, **kwargs)
return order_results(
request_order,
[
LogsBatchQueryResult._from_generated(rsp) for rsp in generated.responses # pylint: disable=protected-access
])
mapping = {item.id: item for item in generated.responses}
return order_results(request_order, mapping, LogsQueryResult)

def close(self):
# type: () -> None
Expand Down
66 changes: 11 additions & 55 deletions sdk/monitor/azure-monitor-query/azure/monitor/query/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from ._helpers import construct_iso8601, process_row
from ._generated.models import (
BatchQueryRequest as InternalLogQueryRequest,
BatchQueryResponse
)


Expand Down Expand Up @@ -47,46 +48,6 @@ def _from_generated(cls, generated):
)


class LogsQueryResult(object):
"""Contains the tables, columns & rows resulting from a query.
:ivar tables: The list of tables, columns and rows.
:vartype tables: list[~azure.monitor.query.LogsTable]
:ivar statistics: This will include a statistics property in the response that describes various
performance statistics such as query execution time and resource usage.
:vartype statistics: object
:ivar visualization: This will include a visualization property in the response that specifies the type of
visualization selected by the query and any properties for that visualization.
:vartype visualization: object
:ivar error: Any error info.
:vartype error: ~azure.core.exceptions.HttpResponseError
"""
def __init__(self, **kwargs):
# type: (Any) -> None
self.tables = kwargs.get("tables", None)
self.statistics = kwargs.get("statistics", None)
self.visualization = kwargs.get("visualization", None)
self.error = kwargs.get("error", None)

@classmethod
def _from_generated(cls, generated):
if not generated:
return cls()
tables = None
if generated.tables is not None:
tables = [
LogsTable._from_generated( # pylint: disable=protected-access
table
) for table in generated.tables
]
return cls(
tables=tables,
statistics=generated.statistics,
visualization=generated.render,
error=generated.error
)


class MetricsResult(object):
"""The response to a metrics query.
Expand Down Expand Up @@ -193,13 +154,9 @@ def _to_generated(self):
workspace=self.workspace
)

class LogsBatchQueryResult(object):
"""The LogsBatchQueryResult.
class LogsQueryResult(object):
"""The LogsQueryResult.
:ivar id: the request id of the request that was sent.
:vartype id: str
:ivar status: status code of the response.
:vartype status: int
:ivar tables: The list of tables, columns and rows.
:vartype tables: list[~azure.monitor.query.LogsTable]
:ivar statistics: This will include a statistics property in the response that describes various
Expand All @@ -215,31 +172,30 @@ def __init__(
self,
**kwargs
):
self.id = kwargs.get('id', None)
self.status = kwargs.get('status', None)
self.tables = kwargs.get('tables', None)
self.error = kwargs.get('error', None)
self.statistics = kwargs.get('statistics', None)
self.visualization = kwargs.get('visualization', None)

@classmethod
def _from_generated(cls, generated):

if not generated:
return cls()
tables = None
if generated.body.tables is not None:
if isinstance(generated, BatchQueryResponse):
generated = generated.body
if generated.tables is not None:
tables = [
LogsTable._from_generated( # pylint: disable=protected-access
table
) for table in generated.body.tables
) for table in generated.tables
]
return cls(
id=generated.id,
status=generated.status,
tables=tables,
statistics=generated.body.statistics,
visualization=generated.body.render,
error=generated.body.error
statistics=generated.statistics,
visualization=generated.render,
error=generated.error
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from .._generated.models import BatchRequest, QueryBody as LogsQueryBody
from .._helpers import process_error, construct_iso8601, order_results
from .._models import LogsQueryResult, LogsBatchQuery, LogsBatchQueryResult
from .._models import LogsQueryResult, LogsBatchQuery
from ._helpers_asyc import get_authentication_policy

if TYPE_CHECKING:
Expand Down Expand Up @@ -115,16 +115,16 @@ async def query_batch(
self,
queries: Union[Sequence[Dict], Sequence[LogsBatchQuery]],
**kwargs: Any
) -> List[LogsBatchQueryResult]:
) -> List[LogsQueryResult]:
"""Execute a list of analytics queries. Each request can be either a LogQueryRequest
object or an equivalent serialized model.
The response is returned in the same order as that of the requests sent.
:param queries: The list of queries that should be processed
:type queries: list[dict] or list[~azure.monitor.query.LogsBatchQuery]
:return: list of LogsBatchQueryResult objects, or the result of cls(response)
:rtype: list[~azure.monitor.query.LogsBatchQueryResult]
:return: list of LogsQueryResult objects, or the result of cls(response)
:rtype: list[~azure.monitor.query.LogsQueryResult]
:raises: ~azure.core.exceptions.HttpResponseError
"""
try:
Expand All @@ -138,11 +138,8 @@ async def query_batch(
request_order = [req['id'] for req in queries]
batch = BatchRequest(requests=queries)
generated = await self._query_op.batch(batch, **kwargs)
return order_results(
request_order,
[
LogsBatchQueryResult._from_generated(rsp) for rsp in generated.responses # pylint: disable=protected-access
])
mapping = {item.id: item for item in generated.responses}
return order_results(request_order, mapping, LogsQueryResult)

async def __aenter__(self) -> "LogsQueryClient":
await self._client.__aenter__()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
for response in responses:
try:
table = response.tables[0]
df = pd.DataFrame(table.rows, columns=[col.name for col in table.columns])
df = pd.DataFrame(table.rows, columns=table.columns)
print(df)
print("\n\n-------------------------\n\n")
except TypeError:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ async def test_logs_query_batch_raises_on_no_timespan():

@pytest.mark.live_test_only
@pytest.mark.asyncio
async def test_logs_query_batch():
async def test_logs_query_batch_default():
client = LogsQueryClient(_credential())

requests = [
Expand All @@ -85,14 +85,22 @@ async def test_logs_query_batch():
workspace_id= os.environ['LOG_WORKSPACE_ID']
),
LogsBatchQuery(
query= "AppRequests | take 2",
query= "Wrong query | take 2",
workspace_id= os.environ['LOG_WORKSPACE_ID'],
timespan=None
),
]
response = await client.query_batch(requests)

assert len(response) == 3
r0 = response[0]
assert r0.tables[0].columns == ['count_']
r1 = response[1]
assert r1.tables[0].columns[0] == 'TimeGenerated'
assert r1.tables[0].columns[1] == '_ResourceId'
assert r1.tables[0].columns[2] == 'avgRequestDuration'
r2 = response[2]
assert r2.error is not None

@pytest.mark.skip('https://github.com/Azure/azure-sdk-for-python/issues/19382')
@pytest.mark.live_test_only
Expand Down
13 changes: 11 additions & 2 deletions sdk/monitor/azure-monitor-query/tests/test_logs_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def test_logs_server_timeout():
assert 'Gateway timeout' in e.value.message

@pytest.mark.live_test_only
def test_logs_query_batch():
def test_logs_query_batch_default():
client = LogsQueryClient(_credential())

requests = [
Expand All @@ -92,14 +92,23 @@ def test_logs_query_batch():
workspace_id= os.environ['LOG_WORKSPACE_ID']
),
LogsBatchQuery(
query= "AppRequests | take 2",
query= "Wrong query | take 2",
workspace_id= os.environ['LOG_WORKSPACE_ID'],
timespan=None
),
]
response = client.query_batch(requests)

assert len(response) == 3

r0 = response[0]
assert r0.tables[0].columns == ['count_']
r1 = response[1]
assert r1.tables[0].columns[0] == 'TimeGenerated'
assert r1.tables[0].columns[1] == '_ResourceId'
assert r1.tables[0].columns[2] == 'avgRequestDuration'
r2 = response[2]
assert r2.error is not None

@pytest.mark.live_test_only
def test_logs_single_query_with_statistics():
Expand Down
2 changes: 0 additions & 2 deletions sdk/monitor/azure-monitor-query/tests/test_logs_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,3 @@ def test_query_response_types():
assert isinstance(result.tables[0].rows[0][1], six.string_types) # _ResourceId generated is a string
assert isinstance(result.tables[0].rows[0][2], bool) # Success generated is a bool
assert isinstance(result.tables[0].rows[0][3], int) # ItemCount generated is a int
assert isinstance(result.tables[0].rows[0][4], float) # DurationMs generated is a real

0 comments on commit 93549d1

Please sign in to comment.