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

fits the quantile value metric for the distribution class metric in googlecloud #627

Merged
merged 3 commits into from
Aug 30, 2023

Conversation

resurgence72
Copy link
Contributor

@resurgence72 resurgence72 commented Aug 29, 2023

为 googlecloud 中的 distribution 类指标适配分位值指标;当前看只记录了 xxx_sum 的 count 值。

  1. 单独记录 mean 意义不大,因为 mean 会拉平 distribution,最好结合 bucket 算 quantiles。
  2. 默认算 p50/75/90/95/99/999 quantiles + mean;分位值指标目前以 label 的形式区分。

PS: 当前开发环境暂无gcp账号,需要 categraf maintainers 验证 + review下。

@@ -7,10 +7,10 @@ import (
"time"

"cloud.google.com/go/monitoring/apiv3/v2/monitoringpb"
"flashcat.cloud/categraf/inputs/googlecloud/internal"
Copy link
Collaborator

Choose a reason for hiding this comment

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

import 顺序一般是系统库 、第三方库 、自己的库

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it. 这块应该是idea没format好

// append mean quantile to slice
for i, qt := range append(
quantile.GetQuantiles(),
point.GetValue().GetDistributionValue().Mean,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mean  这里应该是GetMean()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GetMean() 和 Mean 我看了下都可以,但是为了保持一致性,我这里后续会统一调整为方法调用。晚上我统一再调整下代码

@resurgence72
Copy link
Contributor Author

@kongfei605 import 顺序已经调整;Mean 已经调整为 GetMean()。

@kongfei605
Copy link
Collaborator

Thank you @resurgence72

@kongfei605 kongfei605 merged commit bd1ec58 into flashcatcloud:main Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants