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

Cloudwatch: ListMetrics API page limit #31788

Merged
merged 2 commits into from
Mar 10, 2021
Merged

Cloudwatch: ListMetrics API page limit #31788

merged 2 commits into from
Mar 10, 2021

Conversation

sunker
Copy link
Contributor

@sunker sunker commented Mar 8, 2021

What this PR does / why we need it:
Limit the number of pages being returned by the ListMetrics API. By default, the limit will be 500 pages, but that can be overruled by specifying list_metrics_page_limit in the grafana.ini.

Which issue(s) this PR fixes:
Fixes #31597

@sunker sunker added this to the 7.5.0-stable milestone Mar 8, 2021
@sunker sunker requested review from a team as code owners March 8, 2021 20:02
@sunker sunker requested review from aknuds1, ying-jeanne, dsotirakis, oddlittlebird, achatterjee-grafana, osg-grafana and a team and removed request for a team, ying-jeanne and dsotirakis March 8, 2021 20:02
@oddlittlebird
Copy link
Contributor

@sunker, please add a feature write-up and link in What's new in Grafana 7.5. Thank you!

dsotirakis
dsotirakis approved these changes Mar 9, 2021
View reviewed changes
Copy link
Contributor

@dsotirakis dsotirakis left a comment

Choose a reason for hiding this comment

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

LGTM, small comments / suggestions really 👍

pkg/tsdb/cloudwatch/metric_find_query.go Outdated Show resolved Hide resolved
pkg/tsdb/cloudwatch/metric_find_query_test.go Outdated Show resolved Hide resolved
@dsotirakis dsotirakis self-requested a review March 9, 2021 07:10
dsotirakis
dsotirakis reviewed Mar 9, 2021
View reviewed changes
Copy link
Contributor

@dsotirakis dsotirakis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There's also Diana's suggestion, so there's also that to be taken in account before we merge I think!

achatterjee-grafana
achatterjee-grafana suggested changes Mar 9, 2021
View reviewed changes
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

Added some copy-edit suggestions.

docs/sources/administration/configuration.md Outdated Show resolved Hide resolved
docs/sources/datasources/cloudwatch.md Outdated Show resolved Hide resolved
@sunker sunker force-pushed the cloudwatch/31597-list-metrics-limit branch from 77a52e6 to 3671951 Compare March 9, 2021 21:09
achatterjee-grafana
achatterjee-grafana approved these changes Mar 9, 2021
View reviewed changes
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

One small change request, otherwise LGTM.

docs/sources/datasources/cloudwatch.md Outdated Show resolved Hide resolved
@sunker @achatterjee-grafana
Update docs/sources/datasources/cloudwatch.md
eb0386b 
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
@sunker
Copy link
Contributor Author

sunker commented Mar 10, 2021

@sunker, please add a feature write-up and link in What's new in Grafana 7.5. Thank you!

Yes I'll fix later today! :)

@sunker sunker merged commit d512c5a into master Mar 10, 2021
@sunker sunker deleted the cloudwatch/31597-list-metrics-limit branch March 10, 2021 06:41
@grafanabot
Copy link
Contributor

The backport to v7.5.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-31788-to-v7.5.x origin/v7.5.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x d512c5a1b49e295c1e83717c35ee77dc3c7661c3
# Push it to GitHub
git push --set-upstream origin backport-31788-to-v7.5.x
git switch master
# Remove the local backport branch
git branch -D backport-31788-to-v7.5.x

Then, create a pull request where the base branch is v7.5.x and the compare/head branch is backport-31788-to-v7.5.x.

sunker added a commit that referenced this pull request Mar 10, 2021
@sunker
Revert "Cloudwatch: ListMetrics API page limit (#31788)"
1c64d3e 
This reverts commit d512c5a.
@sunker sunker mentioned this pull request Mar 10, 2021
Closed
@sunker sunker restored the cloudwatch/31597-list-metrics-limit branch March 10, 2021 06:57
sunker added a commit that referenced this pull request Mar 10, 2021
@sunker
Cloudwatch: ListMetrics API page limit (#31788)
2f54762 
* add list metrics api page limit

* Update docs/sources/datasources/cloudwatch.md

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
(cherry picked from commit d512c5a)
sunker added a commit that referenced this pull request Mar 10, 2021
@sunker
Cloudwatch: ListMetrics API page limit (#31788) (#31851)
6ddb62f 
* Cloudwatch: ListMetrics API page limit (#31788)

* add list metrics api page limit

* Update docs/sources/datasources/cloudwatch.md

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
(cherry picked from commit d512c5a)

* update go sum

* remove spaces

* revert go.sum
@sunker sunker added the breaking change Relevant for changelog generation label Mar 10, 2021
ryantxu pushed a commit that referenced this pull request Mar 10, 2021
@sunker @achatterjee-grafana @ryantxu
Cloudwatch: ListMetrics API page limit (#31788)
1e7f0b7 
* add list metrics api page limit

* Update docs/sources/datasources/cloudwatch.md

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
ryantxu pushed a commit to dejapong/grafana that referenced this pull request Mar 30, 2021
@sunker @achatterjee-grafana @ryantxu
Cloudwatch: ListMetrics API page limit (grafana#31788)
90a4c78 
* add list metrics api page limit

* Update docs/sources/datasources/cloudwatch.md

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
@sarahzinger sarahzinger mentioned this pull request Aug 10, 2021
Closed
@mtanda mtanda mentioned this pull request Jan 19, 2022
Closed
@kaydelaney kaydelaney mentioned this pull request Sep 23, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsotirakis dsotirakis dsotirakis approved these changes

@achatterjee-grafana achatterjee-grafana achatterjee-grafana approved these changes

@aknuds1 aknuds1 Awaiting requested review from aknuds1

@oddlittlebird oddlittlebird Awaiting requested review from oddlittlebird

@osg-grafana osg-grafana Awaiting requested review from osg-grafana

Assignees
No one assigned
Labels
add to changelog breaking change Relevant for changelog generation datasource/CloudWatch effort/none old backport v7.5.x Mark PR for automatic backport to v7.5.x type/feature-request
Projects
None yet
Milestone
7.5.0-beta2
Development

Successfully merging this pull request may close these issues.

CloudWatch: Limit no of ListMetrics api page requests
5 participants
@sunker @oddlittlebird @grafanabot @dsotirakis @achatterjee-grafana