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

Added the --all option to journald and cleaned up wait groups #414

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

jmwilliams89
Copy link
Contributor

Description of Changes

  • Added the --all flag to the journald operator. This will prevent entries exceeding 4096 bytes from being encoded as null.
  • Corrected the wait group to be incremented outside of the goroutine to avoid race conditions.
  • Removed superfluous call to the wait group in the polling function.

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.6379586 +0.2931447 128.74757 +1.1807632
1 5000 4.741427 -0.74136305 134.32126 -5.673218
1 10000 10.310347 -0.08634853 144.07866 -2.139557
1 50000 45.655136 -5.380577 173.9278 -3.2761383
1 100000 99.10428 -14.548264 230.9472 -16.017242
10 100 1.9482856 -0.034596443 134.28287 +1.6105957
10 500 5.5689564 -0.55186987 136.1409 -1.7766724
10 1000 13.379389 +1.292983 146.99771 -0.17012024
10 5000 57.031597 +3.6684532 180.3486 +1.2672424
10 10000 107.85927 -2.5794296 220.19882 -9.51265

Copy link
Member

@jsirianni jsirianni left a comment

Choose a reason for hiding this comment

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

Looks good and works well. I see this in my openshift agent's ps -aux output:

journalctl --utc --output=json --all --no-pager --directory /var/log/journal --after-cursor. . . . .

@codecov
Copy link

codecov bot commented Sep 8, 2021

Codecov Report

Merging #414 (509bf35) into master (5fc3cd8) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #414      +/-   ##
==========================================
- Coverage   73.13%   73.08%   -0.05%     
==========================================
  Files         124      124              
  Lines        8095     8094       -1     
==========================================
- Hits         5920     5915       -5     
- Misses       1668     1673       +5     
+ Partials      507      506       -1     
Impacted Files Coverage Δ
operator/builtin/input/journald/journald.go 60.58% <100.00%> (-0.29%) ⬇️
operator/builtin/input/file/file.go 77.04% <0.00%> (-1.53%) ⬇️
operator/builtin/output/newrelic/newrelic.go 73.55% <0.00%> (-0.83%) ⬇️

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 5fc3cd8...509bf35. Read the comment docs.

@jmwilliams89 jmwilliams89 merged commit fe917a6 into master Sep 8, 2021
@jmwilliams89 jmwilliams89 deleted the journald-fix branch September 8, 2021 20:55
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