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

resolve gosec suggestion, use crypo package for randomness #357

Merged
merged 1 commit into from
Jul 8, 2021

Conversation

jsirianni
Copy link
Member

@jsirianni jsirianni commented Jul 8, 2021

Description of Changes

Resolves Gosec suggestion

[/home/jsirianni/go/src/github.com/observiq/stanza/operator/builtin/transformer/filter/filter.go:84] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
    83: 
  > 84: 	if !filtered || rand.Float64() > f.dropRatio {
    85: 		f.Write(ctx, entry)

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.6379282 +0.24136806 129.21364 +0.2909546
1 5000 4.948312 +0.068892 136.23087 -1.8080597
1 10000 10.155442 +0.5001049 141.74124 -4.5172424
1 50000 51.77696 -0.24132538 170.92699 -2.401413
1 100000 99.74889 +2.464981 230.38806 -12.485458
10 100 2.2586708 +0.18969154 130.50337 -4.074356
10 500 5.827721 -0.08597994 138.90167 +0.24960327
10 1000 11.3103695 -0.7933855 148.91473 +4.352371
10 5000 56.873768 +1.9514122 177.35426 +0.75242615
10 10000 110.56446 +8.687332 240.24272 +21.220764

@codecov
Copy link

codecov bot commented Jul 8, 2021

Codecov Report

Merging #357 (7c3e2ae) into master (2113838) will increase coverage by 0.07%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #357      +/-   ##
==========================================
+ Coverage   69.37%   69.44%   +0.07%     
==========================================
  Files         123      123              
  Lines        6519     6525       +6     
==========================================
+ Hits         4522     4531       +9     
+ Misses       1515     1514       -1     
+ Partials      482      480       -2     
Impacted Files Coverage Δ
operator/builtin/transformer/filter/filter.go 57.14% <75.00%> (+1.97%) ⬆️
operator/builtin/output/otlp/otlp.go 61.73% <0.00%> (-2.47%) ⬇️
operator/builtin/output/newrelic/newrelic.go 70.09% <0.00%> (-0.93%) ⬇️
operator/builtin/input/file/file.go 76.43% <0.00%> (+1.91%) ⬆️
operator/builtin/input/tcp/tcp.go 74.14% <0.00%> (+4.31%) ⬆️

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 2113838...7c3e2ae. Read the comment docs.

@jsirianni jsirianni requested a review from djaglowski July 8, 2021 15:26
@jsirianni jsirianni merged commit 7d4488e into master Jul 8, 2021
@jsirianni jsirianni deleted the gosec-filter branch July 8, 2021 16:13
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