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

CloudWatch: Remove references to pkg/infra/metrics #81744

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Conversation

iwysiu
Copy link
Contributor

@iwysiu iwysiu commented Feb 1, 2024

What is this feature?

Replaces the counters that we had in pkg/infra/metrics with the method external plugins use. I'm curious if we're actually using these for anything, considering that the MAwsCloudWatchGetMetricStatistics wasn't ever being incremented.

Why do we need this feature?

We need to decouple the CloudWatch plugin from core datasources

Who is this feature for?

[Add information on what kind of user the feature is for.]

Which issue(s) does this PR fix?:

Fixes #80898

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@iwysiu iwysiu added add to changelog no-backport Skip backport of PR labels Feb 1, 2024
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.4.x milestone Feb 1, 2024
@iwysiu iwysiu changed the title CloudWatch: remove references to pkg/infra/metrics CloudWatch: Remove references to pkg/infra/metrics Feb 1, 2024
@iwysiu iwysiu marked this pull request as ready for review February 2, 2024 19:27
@iwysiu iwysiu requested review from a team as code owners February 2, 2024 19:28
@iwysiu iwysiu requested review from idastambuk, kevinwcyu, papagian, suntala and idafurjes and removed request for a team February 2, 2024 19:28
GetMetricDataLabel = "get_metric_data"
)

var QueriesTotalCounter = promauto.NewCounterVec(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the example this is a global, but is that ok with future multitenant plans?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked with the multi-tenancy group, and it's fine. We want metrics to be across all tenants, and we're not storing sensitive data

@iwysiu iwysiu enabled auto-merge (squash) February 6, 2024 19:09
@iwysiu iwysiu merged commit 3b852bb into main Feb 6, 2024
12 checks passed
@iwysiu iwysiu deleted the iwysiu/80898 branch February 6, 2024 19:21
Ukochka pushed a commit that referenced this pull request Feb 14, 2024
CloudWatch: remove references to pkg/infra/metrics
@aangelisc aangelisc modified the milestones: 10.4.x, 10.4.0 Mar 6, 2024
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.

Remove core imports from grafana/pkg/infra/metrics in CloudWatch
3 participants