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

WIP: refactory cpp metric #914

Closed
wants to merge 14 commits into from
Closed

WIP: refactory cpp metric #914

wants to merge 14 commits into from

Conversation

linrunqi08
Copy link
Collaborator

No description provided.


std::atomic_long timestamp;
};
public:
Copy link
Collaborator

Choose a reason for hiding this comment

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

代码风格好像不符合clangformat

@@ -222,6 +227,14 @@ func GetContainerMeta(containerID string) *C.struct_containerMeta {
return returnStruct
}

//export GetPipelineMetrics
func GetPipelineMetrics(piplineID string) *C.struct_pipelineMetric {
Copy link
Collaborator

Choose a reason for hiding this comment

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

后面go这边可能是一把把所有metric都返回吧


void ILogtailMetric::deletePipelineMetric(PipelineMetricPtr pipelineMetric) {
std::lock_guard<std::mutex> lock(mMetricsLock);
mPipelineMetrics.remove(pipelineMetric);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这是O(n)的,我感觉要O(1)删的话,可能得自己实现双向链表并持有其Node,而非val

BaseMetric();
BaseMetric(MetricObj* metricObj);
~BaseMetric();
MetricObj* mMetricObj;
Copy link
Collaborator

Choose a reason for hiding this comment

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

有没有必要分2层?是想如何扩展这个结构呢?

class PipelineMetric {
public:
std::unordered_map<std::string, BaseMetricPtr> mBaseMetrics;
std::unordered_map<std::string, std::string> mLabels;
Copy link
Collaborator

Choose a reason for hiding this comment

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

有没有必要使用map?这个数据是不是第一次创建后就不变的?也没有查的需求


class PipelineMetric {
public:
std::unordered_map<std::string, BaseMetricPtr> mBaseMetrics;
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个也不太确定是否有必要用map

return;
}
mSnapshotPipelineMetrics.clear();
std::lock_guard<std::mutex> lock(ILogtailMetric::GetInstance()->mMetricsLock);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里我们再想一想,需要优化一下,读不需要锁

@@ -81,7 +81,7 @@ include(${CMAKE_CURRENT_SOURCE_DIR}/dependencies.cmake)
# Subdirectories (modules).
set(SUB_DIRECTORIES_LIST
aggregator app_config checkpoint common config config_manager config_server_pb
controller event event_handler event_listener helper log_pb logger monitor
controller event event_handler event_listener helper log_pb logger monitor ilogtail_metric
Copy link
Collaborator

Choose a reason for hiding this comment

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

加一个指标开发文档

@linrunqi08 linrunqi08 closed this Jul 31, 2023
@linrunqi08 linrunqi08 reopened this Jul 31, 2023
@linrunqi08 linrunqi08 closed this Jul 31, 2023
@linrunqi08 linrunqi08 added this to the v1.7 milestone Aug 8, 2023
@messixukejia messixukejia deleted the feature/taiye/add_metric branch August 16, 2023 06:29
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.

3 participants