Fix throughput/octetDeltaCount issue in flow visualization #2089

zyiou · 2021-04-14T01:45:41Z

IsPresent field of connection should be updated before having
IsConnectionDying check otherwise IsConnectionDying will always
return true, which makes existing connections cannot be updated and
octetDeltaCount always return 0.

This commit also changes the delta count of first record from zero
to total delta count, modifies throughput calculation of first
record in logstash config and changes names of thoughput diagram
from throughput to cumulative bytes.

fixes #2085

zyiou · 2021-04-14T01:46:51Z

@antoninbas Please have a quick check whether the fix works for your setup. It works for me locally. Thanks!

antoninbas

So it fixes the issue with flows not being reported at all. However, if I use connections that last 30 seconds, and keep the export interval at 60 seconds everywhere, all the flow records still have delta counts set to 0, which in turn means that the throughput graphs are still all empty.

This doesn't seem right. I don't know if this is something that should be fixed by including non-zero delta counts in the first flow record for a connection, or if this is something that needs to be fixed on the ELK configuration side. @srikartati

codecov-io · 2021-04-14T03:32:22Z

Codecov Report

Merging #2089 (820eaab) into main (b118f2d) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2089      +/-   ##
==========================================
- Coverage   60.97%   60.94%   -0.03%     
==========================================
  Files         270      270              
  Lines       20366    20365       -1     
==========================================
- Hits        12418    12412       -6     
- Misses       6649     6670      +21     
+ Partials     1299     1283      -16

Flag	Coverage Δ
kind-e2e-tests	`51.71% <100.00%> (+0.07%)`	⬆️
unit-tests	`41.35% <100.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/agent/flowexporter/connections/connections.go	`76.05% <100.00%> (-0.17%)`	⬇️
pkg/agent/controller/traceflow/packetin.go	`56.69% <0.00%> (-7.15%)`	⬇️
...agent/controller/traceflow/traceflow_controller.go	`69.46% <0.00%> (-3.82%)`	⬇️
pkg/controller/traceflow/controller.go	`67.27% <0.00%> (-2.43%)`	⬇️
pkg/agent/openflow/pipeline.go	`68.99% <0.00%> (-0.87%)`	⬇️
pkg/agent/route/route_linux.go	`42.44% <0.00%> (+0.58%)`	⬆️
pkg/agent/agent.go	`47.72% <0.00%> (+0.71%)`	⬆️
...ntroller/networkpolicy/networkpolicy_controller.go	`71.75% <0.00%> (+1.62%)`	⬆️
...gent/controller/networkpolicy/status_controller.go	`75.34% <0.00%> (+2.73%)`	⬆️
pkg/agent/flowexporter/exporter/exporter.go	`74.59% <0.00%> (+5.14%)`	⬆️
... and 2 more

zyiou · 2021-04-14T04:04:30Z

Currently delta count of first record will always be 0. Reason is that for some connections established before flow exporter gets deployed, the bytes in conntrack table may be quite large, making the first throughput a peak in the diagram.
In the case that connection lasting time is less than exporting interval, there will be only one record exported and delta count of the record will all be 0.
This can be fixed by latest commit.

srikartati · 2021-04-14T04:42:52Z

So it fixes the issue with flows not being reported at all. However, if I use connections that last 30 seconds, and keep the export interval at 60 seconds everywhere, all the flow records still have delta counts set to 0, which in turn means that the throughput graphs are still all empty.

This doesn't seem right. I don't know if this is something that should be fixed by including non-zero delta counts in the first flow record for a connection, or if this is something that needs to be fixed on the ELK configuration side. @srikartati

For connections whose duration is lower than the activeFlowExportTimeout, we will send the flow record with idleFlowExportTimeout, depending on the exact duration of the short flow. In these cases, when we send one final record we can send non-zero delta values, i.e., for all flow records generated by idleFlowExportTimeout, we will send non-zero delta count, even though it is the first record (need a fix for this).
For flows whose duration is such a value, where there will be one record because of activeFlowExportTimeout and the final record because of inactiveFlowExportTimeout will still have zero delta count and the second record will have the non-zero delta count.

What do you think?

antoninbas · 2021-04-14T18:28:57Z

I don't know if there is anything to change on the Antrea side. I do not want to report non-zero delta counts if it's going to create other issues, e.g. with bandwidth reporting, or if it is not in agreement with the IPFIX specs.

It seems to me that maybe this should be fixed on the ELK configuration side: maybe for these "throughput" graphs showing the cumulative amount of traffic, we should fallback to the total amount if the delta count is 0?

srikartati · 2021-04-14T19:42:20Z

I don't know if there is anything to change on the Antrea side. I do not want to report non-zero delta counts if it's going to create other issues, e.g. with bandwidth reporting, or if it is not in agreement with the IPFIX specs.

It seems to me that maybe this should be fixed on the ELK configuration side: maybe for these "throughput" graphs showing the cumulative amount of traffic, we should fallback to the total amount if the delta count is 0?

I checked RFC again and could not find any relevant specification specific to this scenario. Yes even with change on the flow exporter side with inactiveFlowExportTimeout, we will still have an issue of incorrect bandwidth computation using the existing methodology.

Bandwidth computation for flows with only one record using delta count and time interval of records, which is the current method of computation, does not make sense. Therefore, thinking of providing a solution from the ELK flow collector side is a good alternative. I can think of two options:

Ignore the first flow record for bandwidth computation. The downside is we do not see flows with one flow record in throughput charts.
Compute bandwidth or throughput differently for the first flow record. Use total bytes and difference of flow end time - flow start time for the bandwidth computation corresponding to the first record and use the existing computation for the rest of the flow records. Normalize this for all flows. One con is that I am doubtful about the flow start time correctness as we have not tested this enough on all platforms. In due course of time, it will become robust.

@zyiou I feel the name Pod-to-Pod Flow Throughput (Bytes) is not appropriate as there is no time component at all. It should be changed to something like Pod-to-Pod Flow Cumulative Bytes. Do we use total bytes here or delta count? If delta counts, we should directly use total bytes and keep updating after every record.

zyiou · 2021-04-14T21:51:44Z

@zyiou I feel the name Pod-to-Pod Flow Throughput (Bytes) is not appropriate as there is no time component at all. It should be changed to something like Pod-to-Pod Flow Cumulative Bytes. Do we use total bytes here or delta count? If delta counts, we should directly use total bytes and keep updating after every record.

I agree that the name should be changed.
We use delta count in the diagram. Reason is that the value shown in the diagram is actually the sum of delta count over the time period user chooses. For example, if user wants to view the data of last 5 min, the bytes in the diagram is the sum of delta count over last 5 minutes. Sum of total count over last 5 min does not make sense in this case.

srikartati · 2021-04-14T22:42:28Z

We use delta count in the diagram. Reason is that the value shown in the diagram is actually the sum of delta count over the time period user chooses. For example, if user wants to view the data of last 5 min, the bytes in the diagram is the sum of delta count over last 5 minutes. Sum of total count over last 5 min does not make sense in this case.

In theory, we can do the difference of the total count of the last flow record and first flow record in the last five mins. Instead of changing too much, we can do a mix of the total count and delta counts, i.e., use total count for the very first flow record and delta count for the rest. Till now we are ignoring the bytes that come from the first flow record, which leads to inaccurate cumulative byte count. Do you agree? Can this logic be managed in logstash?

IsPresent field of connection should be updated before having IsConnectionDying check otherwise IsConnectionDying will always return true, which makes existing connections cannot be updated and octetDeltaCount always return 0. This commit also changes the delta count of first record from zero to its total delta count, modifies throughput calculation of first record in logstash config and changes names of thoughput diagram from 'throughput' to 'cumulative bytes'.

antoninbas

what are the Kibana changes (kibana.ndjson and KIBANA_DEFAULTAPPID value)?
did you test the change in your cluster?

srikartati · 2021-04-15T00:36:24Z

build/yamls/elk-flow-collector/elk-flow-collector.yml

@@ -165,7 +165,7 @@ spec:
            - name: ELASTICSEARCH_URL
              value: "http://elasticsearch:9200"
            - name: KIBANA_DEFAULTAPPID
-              value: "dashboard/653cf1e0-2fd2-11e7-99ed-49759aed30f5"
+              value: "dashboard/3b331b30-b987-11ea-b16e-fb06687c3589"


same question as Antonin. Why is this required?

Found that dashboard cannot be found error popping up after importing the Kibana ndjson file.

zyiou · 2021-04-15T00:42:40Z

what are the Kibana changes (kibana.ndjson and KIBANA_DEFAULTAPPID value)?
did you test the change in your cluster?

Found that dashboard cannot be found error popping up after importing the Kibana ndjson file. It does not affect our functionalities. (Probably because my browser setting this error does not show before) Tested the change.

srikartati · 2021-04-15T00:45:26Z

/test-all

srikartati · 2021-04-15T02:15:18Z

/test-ipv6-only-e2e
/test-ipv6-e2e

IsPresent field of connection should be updated before having IsConnectionDying check otherwise IsConnectionDying will always return true, which makes existing connections cannot be updated and octetDeltaCount always return 0. This commit also changes the delta count of first record from zero to its total delta count, modifies throughput calculation of first record in logstash config and changes names of thoughput diagram from 'throughput' to 'cumulative bytes'.

vmwclabot added the cla-not-required label Apr 14, 2021

zyiou force-pushed the zyiou/fix_2085 branch from caacaab to 820eaab Compare April 14, 2021 01:52

antoninbas reviewed Apr 14, 2021

View reviewed changes

zyiou force-pushed the zyiou/fix_2085 branch from 9a36ba4 to b2963d8 Compare April 15, 2021 00:17

antoninbas approved these changes Apr 15, 2021

View reviewed changes

srikartati reviewed Apr 15, 2021

View reviewed changes

srikartati approved these changes Apr 15, 2021

View reviewed changes

srikartati merged commit efddc8d into antrea-io:main Apr 15, 2021

srikartati mentioned this pull request Apr 16, 2021

Update "ELK Flow Collector" documentation to account for the FlowAggregator #2058

Closed

zyiou added area/flow-visibility Issues or PRs related to flow visibility support in Antrea area/flow-visibility/elk Issues or PRs related to the reference ELK configuration for flow visualization labels Jun 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix throughput/octetDeltaCount issue in flow visualization #2089

Fix throughput/octetDeltaCount issue in flow visualization #2089

zyiou commented Apr 14, 2021 •

edited

Loading

zyiou commented Apr 14, 2021

antoninbas left a comment

codecov-io commented Apr 14, 2021 •

edited

Loading

zyiou commented Apr 14, 2021

srikartati commented Apr 14, 2021 •

edited

Loading

antoninbas commented Apr 14, 2021

srikartati commented Apr 14, 2021

zyiou commented Apr 14, 2021

srikartati commented Apr 14, 2021 •

edited

Loading

antoninbas left a comment

srikartati Apr 15, 2021

zyiou Apr 15, 2021

zyiou commented Apr 15, 2021

srikartati commented Apr 15, 2021

srikartati commented Apr 15, 2021

Fix throughput/octetDeltaCount issue in flow visualization #2089

Fix throughput/octetDeltaCount issue in flow visualization #2089

Conversation

zyiou commented Apr 14, 2021 • edited Loading

zyiou commented Apr 14, 2021

antoninbas left a comment

Choose a reason for hiding this comment

codecov-io commented Apr 14, 2021 • edited Loading

Codecov Report

zyiou commented Apr 14, 2021

srikartati commented Apr 14, 2021 • edited Loading

antoninbas commented Apr 14, 2021

srikartati commented Apr 14, 2021

zyiou commented Apr 14, 2021

srikartati commented Apr 14, 2021 • edited Loading

antoninbas left a comment

Choose a reason for hiding this comment

srikartati Apr 15, 2021

Choose a reason for hiding this comment

zyiou Apr 15, 2021

Choose a reason for hiding this comment

zyiou commented Apr 15, 2021

srikartati commented Apr 15, 2021

srikartati commented Apr 15, 2021

zyiou commented Apr 14, 2021 •

edited

Loading

codecov-io commented Apr 14, 2021 •

edited

Loading

srikartati commented Apr 14, 2021 •

edited

Loading

srikartati commented Apr 14, 2021 •

edited

Loading