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.

Sign up for GitHub

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

Jump to bottom

Fix(traffic-log): remove extra endline in traffic log default template #2514

Merged
merged 2 commits into from
Aug 24, 2021
Merged

Fix(traffic-log): remove extra endline in traffic log default template #2514

merged 2 commits into from
Aug 24, 2021

Conversation

lahabana
Copy link
Contributor

@lahabana lahabana commented Aug 6, 2021

We were adding a new line twice. Once in the default template and once in
convertLoggingBackend this was introducing empty lines in log files

@lahabana lahabana requested a review from a team as a code owner August 6, 2021 15:45
jpeach
jpeach approved these changes Aug 8, 2021
View reviewed changes
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

LGTM once tests are fixed up.

jakubdyszkiewicz
jakubdyszkiewicz reviewed Aug 9, 2021
View reviewed changes
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz left a comment

Choose a reason for hiding this comment

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

Please test TCP Logging with Logstash before merging. I have a feeling that it might be a reason to include this intentional newline that we are removing

@lahabana
Fix(traffic-log): remove extra endline in traffic log default template
5792133 
We were adding a new line twice. Once in the default template and once in
`convertLoggingBackend` this was introducing empty lines in log files

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana
Copy link
Contributor Author

@jakubdyszkiewicz I've checked an it also introduces empty lines and the empty lines are removed with this patch

@lahabana
Fix tests
cb701ff 
Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana lahabana merged commit 3665e51 into kumahq:master Aug 24, 2021
mergify bot pushed a commit that referenced this pull request Aug 24, 2021
@lahabana
Fix(traffic-log): remove extra endline in traffic log default template (
2795af5 
#2514)

We were adding a new line twice. Once in the default template and once in
`convertLoggingBackend` this was introducing empty lines in log files

Signed-off-by: Charly Molter <charly.molter@konghq.com>
(cherry picked from commit 3665e51)
@mergify mergify bot mentioned this pull request Aug 24, 2021
Closed
@lahabana lahabana deleted the fix/traffic-log-newline branch March 29, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakubdyszkiewicz jakubdyszkiewicz jakubdyszkiewicz left review comments

@jpeach jpeach jpeach approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lahabana @jpeach @jakubdyszkiewicz