-
Notifications
You must be signed in to change notification settings - Fork 349
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
refactor: self metric apis #1485
base: main
Are you sure you want to change the base?
Conversation
86cc213
to
b228f61
Compare
Change-Id: I304aeec2992f77394f67da31efbeb711d6789576
Change-Id: I1d9f44b2388b7eed01cb90d7d9ba5ef40e965d99
Change-Id: I7760b0c85fe4bccdf676864e0626ce2b9da93b64
Change-Id: I2830d5c3b8b6aa67d672de40422684a8bfe0141e
62dcdc7
to
8dfb0ff
Compare
…stage-1 Change-Id: I5dc62d9c751d1096595e888feaa204f7752a9860
Change-Id: Ib1876ab352975a4fcf25cd279108137a28d685f7
Change-Id: I7428a399fc31502295bd4437c6f0f391910ee30a
Change-Id: I0b9650aae723bed41a8787f1ff6bc6c2881a7987
newMetric := NewMetric(v.Type(), v, index) | ||
acV, loaded = v.LoadOrStore(k, newMetric) | ||
if loaded { | ||
v.bytesPool.Put(buffer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果buffer超过128B,put回去还对吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
没事的,Get方法中取出对象都会清空的。
*res = (*res)[:0]
Change-Id: Ic188eb9f62eae63e344059062fdb7dfbbb692ac9
type Counter interface { | ||
Metric | ||
Add(int64) error // return error when WithLabels returns an invalid metric. | ||
Get() MetricValue[float64] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里Get是纯获取语义吗,会重置计数吗
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
目前是纯Get,因为对齐了此前的行为,其实改成重置计数会好一点
|
||
type Counter interface { | ||
Metric | ||
Add(int64) error // return error when WithLabels returns an invalid metric. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WithLabels什么情况下会报错?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
传入一个未定义的Label的时候,比如NewCounterVector的时候声明了[]string{status_code}
WithLabel({"invalid_label_name", "label_value"})就会返回一个ErrorMetric,"invalid_label_name"没有被声明过。
|
||
SaveCheckPoint(key string, value []byte) error | ||
GetCheckPoint(key string) (value []byte, exist bool) | ||
SaveCheckPointObject(key string, obj interface{}) error | ||
GetCheckPointObject(key string, obj interface{}) (exist bool) | ||
|
||
// APIs for self monitor | ||
GetMetricRecord() *MetricsRecord // for v1.8.8 compatible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里是GetMetricRecord和RegisterMetricRecord放在一起了是吗,我觉得还是分开比较好,这样方便后面合入
pkg/helper/self_metrics_v2_imp.go
Outdated
} | ||
|
||
// ErrorMetrics always return error. | ||
func NewErrorMetric(metricType pipeline.SelfMetricType, err error) pipeline.Metric { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我感觉为了error实现一套接口,是不是有点得不偿失。在中间直接返回error会比较好?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
也可以,可以讨论一下,我们实现ErrorMetric可以方便插件开发者,但是确实代码要写得多一些。
metrics := metricCollector.Collect() | ||
for _, metric := range metrics { | ||
log := &protocol.Log{} | ||
metric.Serialize(log) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Serialize()和Clear()分两步走是有问题的,非原子性。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这是很好的提议,其实我们实践经验,metrics没必要有clear这个API,每次get都会清空状态。
重构iLogtail自监控 API
MetricsRecords ---> MetricsVector ----> Metric[Counter, Gauge, Latency, Str]
用法:
声明 Metric 或者 MetricVector
MetricVector 需要使用 WithLabel API获取 一个 Metric:
Metric API 和原先基本保持一致。