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

Include a newline at the end of operation event logs #2693

Merged
merged 3 commits into from
Dec 20, 2022

Conversation

pirxthepilot
Copy link
Contributor

@pirxthepilot pirxthepilot commented Nov 24, 2022

Description

Operation event logs do not terminate with a newline, which causes certain log forwarding agents not to process the line. In addition, not having the newline seems to break the POSIX definition of a line, defined as:

A sequence of zero or more non- characters plus a terminating character.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I verified that adding os.linesep after json.dumps() works. I also expect the test_writing_event_logs_to_disk test to pass without issue.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code

@pirxthepilot
Copy link
Contributor Author

Adding for context: vectordotdev/vector#15339

@elegantmoose elegantmoose self-assigned this Dec 13, 2022
@pirxthepilot
Copy link
Contributor Author

Hmm, not sure how to fix those build errors. Looks like failure in the builds themselves?

@elegantmoose
Copy link
Contributor

@pirxthepilot it's not you, existing issue with branches from forks. No worries, hoping to look at PR later this week when I have a sec.

@elegantmoose elegantmoose merged commit 31feaaa into mitre:master Dec 20, 2022
@elegantmoose
Copy link
Contributor

Ty @pirxthepilot

@pirxthepilot
Copy link
Contributor Author

@elegantmoose thanks as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants