[Flow Aggregator] Fix throughput calculation in Logstash #2432
Conversation
zyiou
added
area/flow-visibility/elk
| if newConn.StartTime.IsZero() { | ||
| newConn.StartTime = time.Now() | ||
| } |
There was a problem hiding this comment.
Every time we poll, we change the start time. This is not correct. We should set it the first time we see the connection. It is different from StopTime.
There was a problem hiding this comment.
Got it. Then there is possibility that startTime is later than stopTime?
There was a problem hiding this comment.
Yes if the connection is only polled once.
To resolve this, let's re-update the stopTime again after updating startTime when the connection is added to the map for the first time.
This should be done only for cases when there is zero startTime.
| @@ -278,7 +278,8 @@ func flowStringToAntreaConnection(flow string, zoneFilter uint16) (*flowexporter | |||
| return nil, nil | |||
| } | |||
|
|
|||
| // Add current time as stop time. | |||
| // Add current time as start and stop time. | |||
| conn.StartTime = time.Now() | |||
Codecov Report
@@ Coverage Diff @@
## main #2432 +/- ##
==========================================
+ Coverage 59.63% 59.82% +0.18%
==========================================
Files 284 284
Lines 22178 22171 -7
==========================================
+ Hits 13226 13263 +37
+ Misses 7527 7486 -41
+ Partials 1425 1422 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Signed-off-by: zyiou <zyiou@vmware.com>
|
You may want to cherry-pick this to release-1.2 as well |
|
/skip-all |
This PR fixes throughput divided by 0 issue happened in Logstash.
Since we introduce
conn.StartTime.IsZero()check and update StartTime totime.Now()in previous fix #2308, there are chances thatStartTimewill be equal or later thanStopTime, which may cause incorrect throughput calculation or exception (divided by 0) in Logstash. And we will miss some flow records on Kibana due to this exception when the flow duration is quite short (less than 1 second)