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: Add missing AWS/Prometheus metrics #54990

Merged
merged 1 commit into from Sep 23, 2022
Merged

CloudWatch: Add missing AWS/Prometheus metrics #54990

merged 1 commit into from Sep 23, 2022

Conversation

jangaraj
Copy link
Contributor

@jangaraj jangaraj commented Sep 9, 2022

@jangaraj jangaraj requested a review from a team as a code owner September 9, 2022 14:47
@jangaraj jangaraj requested review from fridgepoet and kevinwcyu and removed request for a team September 9, 2022 14:47
@grafanabot grafanabot added area/backend datasource/CloudWatch pr/external This PR is from external contributor labels Sep 9, 2022
@iwysiu iwysiu added this to the 9.1.6 milestone Sep 14, 2022
@iwysiu iwysiu added add to changelog backport v9.1.x Bot will automatically open backport PR labels Sep 14, 2022
Copy link
Contributor

@iwysiu iwysiu left a comment

Choose a reason for hiding this comment

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

Hi @jangaraj ! Thanks for the pr. I have questions about where a couple of the metrics came from, but otherwise it looks good!

@@ -368,6 +368,7 @@ var metricsMap = map[string][]string{
"AWS/Polly": {"2XXCount", "4XXCount", "5XXCount", "RequestCharacters", "ResponseLatency"},
"AWS/PrivateLinkEndpoints": {"ActiveConnections", "BytesProcessed", "NewConnections", "PacketsDropped", "RstPacketsReceived"},
"AWS/PrivateLinkServices": {"ActiveConnections", "BytesProcessed", "EndpointsCount", "NewConnections", "RstPacketsReceived"},
"AWS/Prometheus": {"AlertManagerAlertsReceived", "AlertManagerNotificationsFailed", "AlertManagerNotificationsThrottled", "DiscardedSamples", "RuleEvaluations", "RuleEvaluationFailures", "RuleGroupIterationsMissed"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did AlertManagerNotificationsFailed come from? I didn't see it in the linked docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch and expected question. AWS doc is not always good and this is an example. AlertManagerNotificationsFailed is valid metrics (I requested also AWS to update their doc):
image

@@ -484,6 +485,7 @@ var dimensionsMap = map[string][]string{
"AWS/Polly": {"Operation"},
"AWS/PrivateLinkEndpoints": {"Endpoint Type", "Service Name", "Subnet Id", "VPC Endpoint Id", "VPC Id"},
"AWS/PrivateLinkServices": {"Az", "Load Balancer Arn", "Service Id", "VPC Endpoint Id"},
"AWS/Prometheus": {"Reason", "RuleGroup", "Workspace"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Also did not see these in the linked docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again AWS doc is not good, but these dimensions are used:
image

Copy link
Contributor

@iwysiu iwysiu left a comment

Choose a reason for hiding this comment

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

Thanks for the screenshots! Nice work.

@xlson xlson modified the milestones: 9.1.6, 9.1.7 Sep 20, 2022
Copy link
Member

@sarahzinger sarahzinger left a comment

Choose a reason for hiding this comment

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

the ci here is failing, but I believe this is due to an issue where external contributors do not have access to the tokens necessary to run our ci checks, and not due to the code itself.

Thanks for the contribution! 🎉

@sarahzinger sarahzinger merged commit b1b4110 into grafana:main Sep 23, 2022
grafanabot pushed a commit that referenced this pull request Sep 23, 2022
sarahzinger pushed a commit that referenced this pull request Sep 23, 2022
(cherry picked from commit b1b4110)

Co-authored-by: Jan Garaj <jan.garaj@gmail.com>
xlson added a commit that referenced this pull request Sep 23, 2022
* main: (496 commits)
  Alerting: Add alert preview to cloud rules editor (#54950)
  Chore: Add methods from sqlstore to org service interface (#55635)
  Frontend: Update frontend styleguide emotion example (#55608)
  Update grabpl version to v3.0.9 (#55621)
  Update dependency @kusto/monaco-kusto to v5.2.0 (#54134)
  CloudWatch: Add missing AWS/Prometheus metrics (#54990)
  Search: create a separate HTTP endpoint (#55634)
  NavTree: Refactor out the navtree building from api/index.go and into it's own service  (#55552)
  Update CODEOWNERS with specific docs responsibilities (#55522)
  Docs: Clarify "supported data sources" (#54337)
  Docs: Fix relrefs in access control API docs (#51940)
  Docs: Note issue #13399 in database install docs (#55596)
  Tempo: Wrap the autocomplete value for a tag in double quotes (#55610)
  Canvas: Add metric value element type (#55205)
  GrafanaUI: Add icon to links on Plugin configuration page (#55581)
  Chore: Move team store implementation to a separate package (#55514)
  Chore: Copy methods from sqlstore to org store (#55615)
  Plugins: Display "renderer" and "secretsmanager" plugin types under plugin catalog "Application" filter (#55597)
  ci: Update CODEOWNERS for as-code team (#55334)
  Navigation: use pageNav and subTitle in Dashboards > Settings > Links (#55510)
  ...
@jangaraj jangaraj deleted the patch-6 branch September 23, 2022 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/backend backport v9.1.x Bot will automatically open backport PR datasource/CloudWatch pr/external This PR is from external contributor
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants