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

Fix prettylog piping #644

Merged
merged 2 commits into from
Feb 1, 2024
Merged

Fix prettylog piping #644

merged 2 commits into from
Feb 1, 2024

Conversation

freak12techno
Copy link
Contributor

Fixes #628.

Basically, instead of directly piping the output from pipe, it now reads it via bufio.Scanner, which splits events by newline, so if the input is buffered, it groups logs by splitting events by newline, and in case it fails, it would just output the logs directly.

Example on testing it:

  1. broken output (e.g. the whole string is invalid JSON as it's essentially split into two chunks by a newline) - displays the input as it is:
➜  prettylog git:(fix-prettylog-piping) ✗ cat test.sh
while true; do
echo '{"level":"info","module":"txindex","height":18397037,"tim';
sleep 0.5;
echo 'e":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}'
sleep 0.5
done

➜  prettylog git:(fix-prettylog-piping) ✗ bash test.sh | ./prettylog
{"level":"info","module":"txindex","height":18397037,"tim
e":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"tim
e":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
  1. chunked output - displays output as pretty logs
➜  prettylog git:(fix-prettylog-piping) ✗
➜  prettylog git:(fix-prettylog-piping) ✗ cat test.sh
while true; do
echo -n '{"level":"info","module":"txindex","height":18397037,"tim';
sleep 0.5;
echo 'e":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}'
sleep 0.5
done

➜  prettylog git:(fix-prettylog-piping) ✗ bash test.sh | ./prettylog
2:13AM INF indexed block exents height=18397037 module=txindex
2:13AM INF indexed block exents height=18397037 module=txindex
2:13AM INF indexed block exents height=18397037 module=txindex
2:13AM INF indexed block exents height=18397037 module=txindex
2:13AM INF indexed block exents height=18397037 module=txindex
  1. ideal output (nothing is buffered, whatever is piping data into prettylog splits it by the newline) - displays output as pretty logs
➜  prettylog git:(fix-prettylog-piping) ✗ cat test.sh
while true; do
echo '{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}'
sleep 0.5
done
➜  prettylog git:(fix-prettylog-piping) ✗ bash test.sh | ./prettylog
2:13AM INF indexed block exents height=18397037 module=txindex
2:13AM INF indexed block exents height=18397037 module=txindex
2:13AM INF indexed block exents height=18397037 module=txindex
2:13AM INF indexed block exents height=18397037 module=txindex


fmt.Printf("%s\n", bytesToWrite)
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please make this a function and reuse it for the code below (for file arg)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought about this, but it's slightly different in terms of how the errors are treated. What do you think would be the best way to do in both cases if error occurs, crash the app and display the error message, or display the output as is?

Copy link
Owner

Choose a reason for hiding this comment

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

Showing the line as-is is probably sensible for such a tool.

@freak12techno
Copy link
Contributor Author

@rs fixed your comments above, here's how it works now:

➜  prettylog git:(fix-prettylog-piping) ✗ cat test.sh
echo "First case: invalid JSON"
for i in {1..5}; do
echo '{"level":"info","module":"txindex","height":18397037,"ti'
echo 'me":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}'
sleep 0.5
done

echo "Second case: chunked valid JSON"
for i in {1..5}; do
echo -n '{"level":"info","module":"txindex","height":18397037,"ti'
echo 'me":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}'
sleep 0.5
done

echo "Third case: not chunked valid JSON"
for i in {1..5}; do
echo '{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}'
sleep 0.5
done

➜  prettylog git:(fix-prettylog-piping) ✗ bash test.sh | ./prettylog
First case: invalid JSON
{"level":"info","module":"txindex","height":18397037,"ti
me":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"ti
me":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"ti
me":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"ti
me":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"ti
me":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
Second case: chunked valid JSON
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Third case: not chunked valid JSON
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex

➜  prettylog git:(fix-prettylog-piping) ✗ cat test2.txt
{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":1839
7037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00",
"message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}

➜  prettylog git:(fix-prettylog-piping) ✗ ./prettylog test2.txt test2.txt
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
{"level":"info","module":"txindex","height":1839
7037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00",
"message":"indexed block exents"}
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
{"level":"info","module":"txindex","height":1839
7037,"time":"2023-12-23T02:13:47+03:00","message":"indexed block exents"}
{"level":"info","module":"txindex","height":18397037,"time":"2023-12-23T02:13:47+03:00",
"message":"indexed block exents"}
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex
Sat, 23 Dec 2023 02:13:47 MSK INF indexed block exents height=18397037 module=txindex

➜  prettylog git:(fix-prettylog-piping) ✗ ./prettylog test3.txt test2.txt
test3.txt open: open test3.txt: no such file or directory%

My concerns here:

  1. if there are multiple files and some of them could not be opened (e.g. due to permissions or file being missing or something else) the program will fail and other files won't be processed (like in the last example above), is that expected? This logic wasn't changed with this PR, just want to make sure it's expected (or not)

  2. is there a point of supporting multiple files when it can be done via bash query, like this:

for filename in folder/*.json; do
prettylog $filename
done

?

  1. is there a point of even supporting the file input when it also can be done via the bash query, like cat filename.json | prettylog?

(IMO it makes the tool extra complex without the reason, but there may be reasoning I am not aware of, wonder what's your opinion on all of that)

@freak12techno freak12techno requested a review from rs January 31, 2024 13:15
@rs rs merged commit 147ae65 into rs:master Feb 1, 2024
5 checks passed
@freak12techno freak12techno deleted the fix-prettylog-piping branch February 4, 2024 00:13
madkins23 pushed a commit to madkins23/zerolog that referenced this pull request Mar 2, 2024
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.

prettylog only displays the first line of the output
2 participants