Add support for isMetadataFilled for aggregation process #196
Conversation
Codecov Report
@@ Coverage Diff @@
## main #196 +/- ##
==========================================
- Coverage 78.66% 78.48% -0.19%
==========================================
Files 17 17
Lines 2133 2143 +10
==========================================
+ Hits 1678 1682 +4
- Misses 304 308 +4
- Partials 151 153 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pkg/intermediate/aggregate.go
Outdated
| @@ -331,6 +340,12 @@ func (a *AggregationProcess) addOrUpdateRecordInMap(flowKey *FlowKey, record ent | |||
| } | |||
| if !correlationRequired { | |||
| aggregationRecord.ReadyToSend = true | |||
| // if no correlation is required for inter-Node record, K8s metadata will not be filled. | |||
There was a problem hiding this comment.
Could you rephrase this? Also please explicitly mention that for Intra-node flows and ToExternal flows, we can set the MetaDataFilled as true by default.
zyiou
force-pushed
the
zyiou/fill_metadata
branch
from
8b723e8
to
f49449a
Compare
zyiou
force-pushed
the
zyiou/fill_metadata
branch
from
f49449a
to
06cdd37
Compare
pkg/intermediate/aggregate.go
Outdated
| // if no correlation is required for inter-Node record, K8s metadata will not | ||
| // be filled. For Intra-Node flows and ToExternal flows, isMetaDataFilled is set | ||
| // to true by default. | ||
| if isRecordFromSrc(record) || isRecordFromDst(record) { |
There was a problem hiding this comment.
Not your change? Do we still need isRecordFromSrc and isRecordFromDst? We already have flow type in the flow record. Can we use that to make code simple?
There was a problem hiding this comment.
Sure. We can switch to using flowType here. But function isRecordFromSrc, isRecordFromDst and areRecordsFromSameNode need to be kept because this information is not revealed from flowType.
There was a problem hiding this comment.
Yes we still need those methods. Moving to flowType here makes the code easily readable.
pkg/intermediate/aggregate.go
Outdated
| @@ -331,6 +340,14 @@ func (a *AggregationProcess) addOrUpdateRecordInMap(flowKey *FlowKey, record ent | |||
| } | |||
| if !correlationRequired { | |||
| aggregationRecord.ReadyToSend = true | |||
| // if no correlation is required for inter-Node record, K8s metadata will not | |||
There was a problem hiding this comment.
"If no correlation is required for an inter-Node record, K8s metadata is expected to be not completely filled."
zyiou
force-pushed
the
zyiou/fill_metadata
branch
from
ac6439f
to
5aef325
Compare
If correlation is required, we will set
isMetadataFilledto true.if correlation is not required, we will set
isMetadataFilledto true only when the flow is intra-Node.isMetadataFilledis always true for Intra-Node and ToExternal flows and only applicable for Inter-Node flows that are not required to be correlated. (e.g. flows with Egress deny rule applied)