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.

Sign up for GitHub

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

Jump to bottom

[Monitor Query] Add batch metrics query integration #31049

Merged
merged 4 commits into from Aug 16, 2023
Merged

[Monitor Query] Add batch metrics query integration #31049

merged 4 commits into from Aug 16, 2023

Conversation

pvaneck
Copy link
Member

@pvaneck pvaneck commented Jul 10, 2023 •
edited

This introduces a new client for querying resources in batches. Based on archboard feedback, this will currently be a separate client since this uses a separate data-plane API as opposed to the ARM API that the MetricsQueryClient uses.

Modifications

  • Introduce MetricsBatchQueryClient for sync/async.
  • Add new swagger file section for client generation.
  • Regenerate code with latest autorest
  • Update README and add samples
  • Add tests and re-record

Fixes #29092

@azure-sdk
Copy link
Collaborator

azure-sdk commented Aug 8, 2023 •
edited

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-monitor-query

@pvaneck pvaneck force-pushed the query-data-plane-metrics branch 3 times, most recently from 051435e to dce44d8 Compare August 15, 2023 02:09
@pvaneck
[Monitor Query] Add batch metrics query integration
a131a33 
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@pvaneck
Copy link
Member Author

pvaneck commented Aug 15, 2023

/azp run python - monitor - tests

@pvaneck pvaneck marked this pull request as ready for review August 15, 2023 18:38
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

kristapratico
kristapratico reviewed Aug 15, 2023
View reviewed changes
Copy link
Member

@kristapratico kristapratico left a comment

Choose a reason for hiding this comment

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

A few questions, but otherwise looks good! Sync feedback generally applies to async as well. Should we add the new client to the changelog?

sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/aio/_metrics_batch_query_client_async.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/samples/async_samples/sample_metrics_batch_query_async.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/samples/async_samples/sample_metrics_batch_query_async.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/samples/async_samples/sample_metrics_batch_query_async.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py Outdated Show resolved Hide resolved
@pvaneck
Address review comments
805ccf1 
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@pvaneck pvaneck requested a review from a team as a code owner August 16, 2023 01:34
pvaneck
pvaneck commented Aug 16, 2023
View reviewed changes
Copy link
Member Author

@pvaneck pvaneck left a comment

Choose a reason for hiding this comment

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

Thanks, @kristapratico for the great feedback! Addressed most comments and replied in-line 😸.

sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Show resolved Hide resolved
sdk/monitor/azure-monitor-query/samples/async_samples/sample_metrics_batch_query_async.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py Outdated Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
@pvaneck
Update changelog
0031529 
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
kristapratico
kristapratico approved these changes Aug 16, 2023
View reviewed changes
Copy link
Member

@kristapratico kristapratico left a comment

Choose a reason for hiding this comment

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

LGTM. Only feedback would be to double-check with architects regarding using ItemPaged for query_batch

sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Show resolved Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_metrics_batch_query_client.py Outdated Show resolved Hide resolved
@pvaneck
Add inline comments
9c8cb4f 
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@pvaneck pvaneck merged commit 4f8dc00 into Azure:main Aug 16, 2023
16 checks passed
@pvaneck pvaneck deleted the query-data-plane-metrics branch August 16, 2023 23:20
lmazuel pushed a commit that referenced this pull request Aug 21, 2023
@pvaneck @lmazuel
[Monitor Query] Add batch metrics query integration (#31049)
1f3153f 
This adds the `MetricsBatchQueryClient` for querying resource URIs in batches.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@scottaddie scottaddie added Monitor Monitor, Monitor Ingestion, Monitor Query and removed Monitor - Query labels Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kristapratico kristapratico kristapratico approved these changes

Assignees
No one assigned
Labels
Monitor Monitor, Monitor Ingestion, Monitor Query
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Monitor Query] Migrate to data plane metrics query API
4 participants
@pvaneck @azure-sdk @kristapratico @scottaddie