feat: Implement dual lifecycle management for eventWorker #785
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@chilli13 Can you help test this PR? |
|
@chilli13 Can you verify this PR? Does it solve the issue of events being split? |
|
@chilli13 如果这解决了你的一部分问题，我可以直接推送到你的分支上面吗 |
|
感觉UUIDSSLDataCount 的定义不是很合理，只通过count == 7 判断包含socket 来决定LifeCycleState 的状态会让人迷惑，是否可以通过增加一个字段了识别event类型来决定使用哪种LifeCycleState ？ |
|
我尝试看看再优化下 |
94b1f2d to
dfb6f47
Compare
|
改为了前缀判断逻辑 |
|
直接合并会导致现有逻辑无法正常工作，可以把 uuid 改为原本格式再合并 |
|
近期工作比较忙，周末我review一下。感谢 |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces dual lifecycle management for eventWorker, allowing workers to be cleaned up either by an internal timer or via an external socket-driven signal.
- Changed UUID delimiter in
TcSkbEventto underscores for consistency. - Added
SocketLifecycleUUIDPrefixand updatedConnDataEvent.GetUUIDto optionally prefix socket-bound UUIDs. - Extended
eventWorkerwith aLifeCycleState,closeChan, and new methods to support external shutdown, and updated the run loop to respect both timer and external signals.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| user/event/event_openssl_tc.go | Updated GetUUID delimiter from “-” to “_forTcSkbEvent`. |
| user/event/event_openssl.go | Introduced SocketLifecycleUUIDPrefix and commented socket-prefixed UUID in ConnDataEvent. |
| pkg/event_processor/iworker.go | Implemented LifeCycleState, added closeChan, CheckLifeCycleState, CloseEventWorker, GetLifeCycleState, and modified Run to handle socket-driven closure. |
Comments suppressed due to low confidence (4)
user/event/event_openssl.go:33
- [nitpick] This string constant is declared alongside
AttachTypeenum values, mixing distinct types. Consider movingSocketLifecycleUUIDPrefixinto its ownconstblock or a clearly labeled grouping to avoid confusion.
SocketLifecycleUUIDPrefix = "sock"
user/event/event_openssl_tc.go:93
- Changing the UUID delimiter from
-to_is a breaking change for any consumers parsing the old format. Ensure this is documented in release notes and update any dependent code to use the new format.
return fmt.Sprintf("%d_%d_%s", te.Pid, te.Ifindex, te.Comm)
pkg/event_processor/iworker.go:119
- There are no tests covering the new external-close lifecycle (
CloseEventWorker,closeChan, andLifeCycleStateSocket). Adding unit tests for both default timer shutdown and socket-driven shutdown would ensure this behavior remains correct.
func (ew *eventWorker) CloseEventWorker() {
pkg/event_processor/iworker.go:111
- The code references
event.SocketLifecycleUUIDPrefixbut theeventpackage isn’t imported, which will cause a compile error. Please add the correct import for theuser/eventpackage (or adjust the alias) so this line resolves.
if strings.HasPrefix(uuid, event.SocketLifecycleUUIDPrefix) {
user/event/event_openssl.go
Outdated
| @@ -30,6 +30,7 @@ type AttachType int64 | |||
| const ( | |||
| ProbeEntry AttachType = iota | |||
| ProbeRet | |||
| SocketLifecycleUUIDPrefix = "sock" | |||
There was a problem hiding this comment.
把SocketLifecycleUUIDPrefix放到ievent.go里更合适。
pkg/event_processor/iworker.go
Outdated
| // 导出方法，用于发送信号量 | ||
| func (ew *eventWorker) CloseEventWorker() { | ||
| if ew.closeChan != nil { | ||
| close(ew.closeChan) |
我已经做了修改，希望您多保重身体，感谢回复 |
#772
给这个PR提供了一个解决方案，eventWorker拥有了两套生命周期设计
1.默认的定时器销毁设计
2.外部调用销毁设计
在上述PR中，为了从sock定位到对应eventWorker，可以先遍历队列，找到
GetLifeCycleState()返回为LifeCycleStateSocket的worker，然后再从UUID来取出sock字段但是有个缺点，
CheckLifeCycleState(uuid string) LifeCycleState函数的设计沿用了原PR的GetSock设计，由UUID要素数量来判断是否来自SSLDataEvent，可能不是最健壮的方式