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

CloudMonitoring: Fixes broken variable queries that use group bys #43914

Merged
merged 1 commit into from
Jan 17, 2022

Conversation

sunker
Copy link
Contributor

@sunker sunker commented Jan 11, 2022

What this PR does / why we need it:
After a recent API change, cloud monitoring ignores the group by in case the crossSeriesReducer is set to REDUCE_NONE. This caused issues in the CloudMonitoring plugin when loading labels that were used in variables queries and in queries that return labels that would be used in the filter section. This PR ensures a crossSeriesReducer other than REDUCE_NONE is always specified in case a group by was provided.
Which issue(s) this PR fixes:

Fixes #43504

@sunker sunker added this to the 8.3.4 milestone Jan 11, 2022
@sunker sunker requested review from a team, fridgepoet, yaelleC, sarahzinger and andresmgot and removed request for a team January 11, 2022 16:56
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

👍 one minor comment

const labels = await this.datasource.getLabels(selectedMetricType, refId, projectName, [labelKey]);
const labels = await this.datasource.getLabels(selectedMetricType, refId, projectName, {
groupBys: [labelKey],
crossSeriesReducer: 'REDUCE_MEAN',
Copy link
Contributor

Choose a reason for hiding this comment

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

a comment (or better a test) to explain why this is set would be helpful

@andresmgot andresmgot added backport v8.3.x Mark PR for automatic backport to v8.3.x and removed backport v8.3.x Mark PR for automatic backport to v8.3.x labels Jan 17, 2022
@andresmgot
Copy link
Contributor

merging this to be included in 8.3.4

@andresmgot andresmgot merged commit 715166b into main Jan 17, 2022
@andresmgot andresmgot deleted the cloudmonitoring/labels-fix branch January 17, 2022 14:51
grafanabot pushed a commit that referenced this pull request Jan 17, 2022
andresmgot pushed a commit that referenced this pull request Jan 17, 2022
…3914) (#44116)

(cherry picked from commit 715166b)

Co-authored-by: Erik Sundell <erik.sundell@grafana.com>
xlson added a commit to joanlopez/grafana that referenced this pull request Jan 17, 2022
* origin/main:
  Split prepare-release (grafana#44124)
  Fix issue link (grafana#42891)
  Release: remove bump from prepare release action (grafana#44111)
  Access control: Team role picker (grafana#43418)
  Plugins: Add notices to docs to prevent NPX commands from hanging (grafana#44043)
  Azure Monitor: Improved error messages for variable queries (grafana#43213)
  CloudMonitoring: Fixes broken variable queries that use group bys (grafana#43914)
  Elastic: Allow using long/int as date field for alerts (grafana#44027)
  Prometheus: Fix interpolation of $__rate_interval variable (grafana#44035)
  Alerting: show deleted datasource (grafana#43891)
  Dashboard save interaction evt (grafana#43304)
  Chore: reduces circular dependencies for variables/utils.ts (grafana#44087)
  Chore(CodeQL): Add noopener noreferrer to external links in email templates (grafana#44092)
  Export: Fix error being thrown when exporting dashboards using query variables that reference the default datasource (grafana#44034)
  fix delete plugin dashboard (grafana#44055)
  Access Control: Allow signed in users access to GET data sources endpoints (grafana#43338)
  small fix 🙏 (grafana#44089)
  Update dependency marked to v4.0.10 [SECURITY] (grafana#44078)
  Remove Macaron ParamsInt64 function from code base (grafana#43810)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google Cloud Monitoring metadata missing
3 participants