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

Google Cloud Monitor: Fix mem usage for dropdown #67683

Merged
merged 4 commits into from May 5, 2023

Conversation

asimpson
Copy link
Contributor

@asimpson asimpson commented May 2, 2023

Previously the Metric name dropdown would attempt to load all the available metric names into the Select which would eventually crash the browser if the dataset was large enough.

We can fix this by using AsyncSelect and making another query once a Service is selected and the user types a few characters.

Which issue(s) does this PR fix?:
#67682

Previously the Metric name dropdown would attempt to load _all_ the
available metric names into the Select which would eventually crash the
browser if the dataset was large enough.

We can fix this by using AsyncSelect and making another query once a
Service is selected _and_ the user types a few characters.
@asimpson asimpson added this to the 10.0.0 milestone May 3, 2023
@asimpson asimpson added backport v9.4.x Mark PR for automatic backport to v9.4.x backport v9.5.x Bot will automatically open backport PR labels May 3, 2023
@grafanabot
Copy link
Contributor

Hello @asimpson!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

1 similar comment
@grafanabot
Copy link
Contributor

Hello @asimpson!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@asimpson asimpson modified the milestones: 10.0.0, 10.0.x May 3, 2023
@asimpson asimpson marked this pull request as ready for review May 3, 2023 02:30
@asimpson asimpson requested a review from a team as a code owner May 3, 2023 02:30
@asimpson asimpson requested review from aangelisc and alyssabull and removed request for a team May 3, 2023 02:30
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2023

Backend code coverage report for PR #67683
No changes

@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2023

Frontend code coverage report for PR #67683

Plugin Main PR Difference
cloud-monitoring 72.73% 72.76% .03%

Copy link
Contributor

@aangelisc aangelisc left a comment

Choose a reason for hiding this comment

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

So I pulled this down to test locally and couldn't get any metric names to show up in the list :/ I don't see the API call in network requests to filter the data, only the initial metric descriptors request.

@asimpson
Copy link
Contributor Author

asimpson commented May 3, 2023

So I pulled this down to test locally and couldn't get any metric names to show up in the list :/ I don't see the API call in network requests to filter the data, only the initial metric descriptors request.

@aangelisc Oh, I think this is because I didn't set defaultOptions 🤦‍♂️ I've done that now, so you should see a subset of names and then as you type it will make an API request for ones that match 🤞

Copy link
Contributor

@aangelisc aangelisc left a comment

Choose a reason for hiding this comment

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

Leggo 💯

@asimpson asimpson merged commit b2e1b3a into main May 5, 2023
13 checks passed
@asimpson asimpson deleted the simpson-cm-async-select branch May 5, 2023 20:48
@grafanabot
Copy link
Contributor

The backport to v9.4.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-67683-to-v9.4.x origin/v9.4.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b2e1b3ad91456fc70b407a52c552b718cfa7890c
# Push it to GitHub
git push --set-upstream origin backport-67683-to-v9.4.x
git switch main
# Remove the local backport branch
git branch -D backport-67683-to-v9.4.x

Then, create a pull request where the base branch is v9.4.x and the compare/head branch is backport-67683-to-v9.4.x.

@grafanabot grafanabot added the backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. label May 5, 2023
grafanabot pushed a commit that referenced this pull request May 5, 2023
* Google Cloud Monitor: Fix mem usage for dropdown

Previously the Metric name dropdown would attempt to load _all_ the
available metric names into the Select which would eventually crash the
browser if the dataset was large enough.

We can fix this by using AsyncSelect and making another query once a
Service is selected _and_ the user types a few characters.

* fix: update tests for AsyncSelect

* fix lint

* fix: add subset of metrics on initial load

(cherry picked from commit b2e1b3a)
asimpson added a commit that referenced this pull request May 5, 2023
Google Cloud Monitor: Fix mem usage for dropdown (#67683)

* Google Cloud Monitor: Fix mem usage for dropdown

Previously the Metric name dropdown would attempt to load _all_ the
available metric names into the Select which would eventually crash the
browser if the dataset was large enough.

We can fix this by using AsyncSelect and making another query once a
Service is selected _and_ the user types a few characters.

* fix: update tests for AsyncSelect

* fix lint

* fix: add subset of metrics on initial load

(cherry picked from commit b2e1b3a)

Co-authored-by: Adam Simpson <adam@adamsimpson.net>
asimpson added a commit that referenced this pull request May 5, 2023
Previously the Metric name dropdown would attempt to load _all_ the
available metric names into the Select which would eventually crash the
browser if the dataset was large enough.

We can fix this by using AsyncSelect and making another query once a
Service is selected _and_ the user types a few characters.

* fix: update tests for AsyncSelect

* fix lint

* fix: add subset of metrics on initial load
asimpson added a commit that referenced this pull request May 11, 2023
Previously the Metric name dropdown would attempt to load _all_ the
available metric names into the Select which would eventually crash the
browser if the dataset was large enough.

We can fix this by using AsyncSelect and making another query once a
Service is selected _and_ the user types a few characters.

* fix: update tests for AsyncSelect

* fix lint

* fix: add subset of metrics on initial load
@zerok zerok modified the milestones: 10.0.x, 10.0.0-preview, 10.1.x May 31, 2023
@zerok zerok mentioned this pull request Jun 27, 2023
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/frontend backport v9.4.x Mark PR for automatic backport to v9.4.x backport v9.5.x Bot will automatically open backport PR backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. datasource/GoogleCloudMonitoring type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants