Skip to content

Conversation

@ronensc
Copy link
Contributor

@ronensc ronensc commented Oct 18, 2022

This PR adds the _IsFirst field to connection records (newConnection, updateConnection and endConnection).
It's not added to flowLog records.
The field is set to true only on the first record of the connection.
It is useful in cases where newConnection records are not outputted (to reduce the number output records)
and there is a need to count the total number of connections: simply counting _IsFirst=true

This solves #298

@KalmanMeth
Copy link
Collaborator

I don't understand how this helps. By counting the number of _IsFirst=true we get the total number of newConnections from the beginning of measurement. So this is essentially equivalent to counting the times we get newConnection (if we get each one). Can't we continue to count newConnection, and allow to have a newConnection reported on the first output record (even if we also have endConnection and/or updateConnection reported on the same record)?

@ronensc
Copy link
Contributor Author

ronensc commented Oct 19, 2022

@KalmanMeth When the outputRecordTypes is configured with [newConnection, updateConnection, endConnection], i.e. to output newConnection records, then the _IsFirst flag will be set to true only on them. So counting _IsFirst="true" is equivalent to counting _RecordType="newConnection" as you mentioned.
But, when the outputRecordTypes is configured with [updateConnection, endConnection], i.e. to not output newConnection records, then, counting _IsFirst="true" is my suggest way to count the number of connections. Counting _RecordType="newConnection" will return zero.

Comment on lines +552 to +553
"20s: no update report",
startTime.Add(20 * time.Second),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps this should be 19 or less. 20 seems too borderline and it might just get an additional update report.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a close look at the tests!
Since the update report was at time 11s, the next update should be at time 21s. So 21s is the borderline. That's why I check a second before (at 20s) that there are no updates and a second after (at 22s) that there is an update.

Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

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

lgtm!

@ronensc ronensc merged commit 45e8801 into netobserv:main Oct 24, 2022
@ronensc ronensc deleted the conntrack-first-report branch October 24, 2022 13:06
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