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.

Sign up for GitHub

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

Jump to bottom

Implement multi bvar feature #1608

Merged
merged 9 commits into from
Aug 15, 2022
Merged

Implement multi bvar feature #1608

merged 9 commits into from
Aug 15, 2022

Conversation

serverglen
Copy link
Contributor

Related issue: #1322

@serverglen
Copy link
Contributor Author

serverglen commented Nov 17, 2021 •
edited
Loading

代码以及文档都提交了。

@serverglen serverglen mentioned this pull request Nov 18, 2021
Closed
@feng-y
Copy link

feng-y commented Mar 2, 2022

这个mr 大概什么时候可以提交？

@serverglen
Copy link
Contributor Author

这个mr 大概什么时候可以提交？

你现在使用这个功能了吗？

@tcp9
Copy link

tcp9 commented Jul 6, 2022

这个mr 大概什么时候可以提交？

你现在使用这个功能了吗？

想问下 为啥不在brpc_metrics导出呢？

@serverglen
Copy link
Contributor Author

这个mr 大概什么时候可以提交？

你现在使用这个功能了吗？

想问下 为啥不在brpc_metrics导出呢？

有可能metrics和维度非常多，目前还没有通过brpc_metrics导出，不过这个实现起来应该不复杂

@tcp9
Copy link

tcp9 commented Jul 7, 2022

想问下 为啥不在brpc_metrics导出呢？

谢谢@serverglen；是的，我目前基于这个PR已经导出来了，改动比较小，就是metrics维度多的话容易影响/brpc_metrics 性能；

@serverglen
Copy link
Contributor Author

想问下 为啥不在brpc_metrics导出呢？

谢谢@serverglen；是的，我目前基于这个PR已经导出来了，改动比较小，就是metrics维度多的话容易影响/brpc_metrics 性能；

可以支持按照条件过滤，这样导出的数据应该比较小。

@cdjingit
Copy link
Contributor

cdjingit commented Aug 1, 2022

想问下 为啥不在brpc_metrics导出呢？

谢谢@serverglen；是的，我目前基于这个PR已经导出来了，改动比较小，就是metrics维度多的话容易影响/brpc_metrics 性能；

可以支持按照条件过滤，这样导出的数据应该比较小。

这个是不是支持比较好，现在bvar创建本身也没有考虑限制一个数量对于性能带来的影响，由用户自己评估

cdjingit
cdjingit reviewed Aug 4, 2022
View reviewed changes
src/bvar/mvariable.cpp Outdated Show resolved Hide resolved
gydong
gydong reviewed Aug 5, 2022
View reviewed changes
src/bvar/multi_dimension_inl.h
// latency_percentiles
// p1/p2/p3
int latency_percentiles[3] {FLAGS_bvar_latency_p1, FLAGS_bvar_latency_p2, FLAGS_bvar_latency_p3};
for (auto lp : latency_percentiles) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的分位值是固定只支持三个9和4四个9，不支持任意配置的？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

内部暂时是这么实现的，后面可以改成可配置。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改，辛苦再帮review一下

gydong
gydong reviewed Aug 5, 2022
View reviewed changes
src/bvar/mvariable.h Outdated Show resolved Hide resolved
wwbmmm
wwbmmm reviewed Aug 11, 2022
View reviewed changes
docs/cn/mbvar_c++.md Outdated Show resolved Hide resolved
@wwbmmm wwbmmm merged commit f303256 into apache:master Aug 15, 2022
@serverglen serverglen deleted the feature-mbvar branch August 15, 2022 03:51
@Huixxi Huixxi added the feature new feature label Oct 10, 2022
@cdjingit
Copy link
Contributor

想问下 为啥不在brpc_metrics导出呢？

谢谢@serverglen；是的，我目前基于这个PR已经导出来了，改动比较小，就是metrics维度多的话容易影响/brpc_metrics 性能；

hello @tcp9 brpc_metric支持导出多维度metics的，能提交一个mr，贡献出来吗？

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wwbmmm wwbmmm wwbmmm left review comments

@gydong gydong Awaiting requested review from gydong

@cdjingit cdjingit Awaiting requested review from cdjingit

Assignees
No one assigned
Labels
feature new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@serverglen @feng-y @tcp9 @cdjingit @gydong @wwbmmm @Huixxi