Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Goflow fix failed to parse ip #336

Merged
merged 4 commits into from
Jun 24, 2021
Merged

Conversation

jsirianni
Copy link
Member

@jsirianni jsirianni commented Jun 23, 2021

Description of Changes

A user has the following error:

{
  "level": "error",
  "ts": "2021-06-23T15:13:23.145-0500",
  "msg": "Failed to parse netflow message%!(EXTRA zapcore.Field={error 26 0  error converting DstAddr to string: cannot convert byte slice to ip address, expected length of 4 or 16 got 0})",
  "operator_id": "$.dec6c632-c843-467f-a262-75fddb2f7a8e.goflow_input",
  "operator_type": "goflow_input"
}

When an ip address field is length zero, we get this error when instead we should be ignoring the field. I was able to replicate this error by adding the test cases in this PR. The issue is resolved by skipping ip address parsing if the value is empty.

  • Log correct key that is failing to convert to an IP
  • If ip address field is empty, skip parsing it instead of returning an error
  • Add test cases for empty ip address fields
  • Add test case for invalid ip address length (malformed address)

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Add a changelog entry (for non-trivial bug fixes / features)
  • CI passes

@djaglowski
Copy link
Member

Log Files Logs / Second CPU Avg (%) CPU Avg Δ (%) Memory Avg (MB) Memory Avg Δ (MB)
1 1000 1.4827853 +0.034441948 127.25377 -1.6217651
1 5000 5.034576 +0.32750034 139.26993 +5.1365814
1 10000 9.138108 -0.82742786 143.97481 -2.7619934
1 50000 46.2082 -2.0851402 176.06493 +3.6578674
1 100000 103.89996 +5.6227493 233.84294 -3.1095123
10 100 1.9482563 +0.17234683 131.39049 -2.4598694
10 500 5.896599 +0.41374874 139.6805 -0.43089294
10 1000 11.500098 -0.70541763 146.74165 -2.9137878
10 5000 50.481827 -5.9502563 177.28705 -0.18843079
10 10000 108.28174 -10.375099 228.3816 -5.027603

@jsirianni jsirianni requested a review from djaglowski June 23, 2021 22:20
@codecov
Copy link

codecov bot commented Jun 23, 2021

Codecov Report

Merging #336 (2f58c66) into master (f1682b4) will increase coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #336      +/-   ##
==========================================
+ Coverage   69.53%   69.73%   +0.19%     
==========================================
  Files         123      123              
  Lines        6489     6491       +2     
==========================================
+ Hits         4512     4526      +14     
+ Misses       1503     1488      -15     
- Partials      474      477       +3     
Impacted Files Coverage Δ
operator/builtin/input/goflow/parse.go 90.48% <100.00%> (+10.48%) ⬆️
operator/builtin/output/newrelic/newrelic.go 71.03% <0.00%> (-0.93%) ⬇️
operator/builtin/output/forward/forward.go 56.52% <0.00%> (+1.45%) ⬆️
operator/builtin/output/otlp/otlp.go 65.43% <0.00%> (+3.70%) ⬆️
operator/builtin/input/tcp/tcp.go 73.68% <0.00%> (+4.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1682b4...2f58c66. Read the comment docs.

@djaglowski
Copy link
Member

Log Files Logs / Second CPU Avg (%) CPU Avg Δ (%) Memory Avg (MB) Memory Avg Δ (MB)
1 1000 1.4310942 +0.12073064 127.726295 -0.7308731
1 5000 4.7069917 -0.43104744 137.61409 +0.029769897
1 10000 10.672364 +1.344574 145.40652 -1.2831268
1 50000 47.449066 +1.3091011 177.3684 +3.1423645
1 100000 89.052216 -0.9149475 227.33742 +0.35560608
10 100 1.8793962 +0.17245066 133.83728 +0.30818176
10 500 5.569044 -0.06899023 141.56883 -0.8162689
10 1000 11.534627 +0.77590847 146.86166 -1.0040436
10 5000 53.6561 +2.799549 176.32678 +1.8263855
10 10000 99.07312 +1.7139511 220.7003 +3.3544006

@jsirianni jsirianni merged commit d0de115 into master Jun 24, 2021
@jsirianni jsirianni deleted the goflow-fix-failed-to-parse-ip branch June 24, 2021 14:35
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.

2 participants