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

Better JSON output support #37

Closed
pouriyajamshidi opened this issue Jul 10, 2022 · 8 comments · Fixed by #74
Closed

Better JSON output support #37

pouriyajamshidi opened this issue Jul 10, 2022 · 8 comments · Fixed by #74
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@pouriyajamshidi
Copy link
Owner

Summary

We have introduced a preliminary JSON output support with which can be activated using the -j flag. For instance:

{"message":"TCPinging example.com on port 443"}
{"message":"Reply from example.com (93.184.216.34) on port 443 TCP_conn=1 time=128 ms"}
{"message":"Reply from example.com (93.184.216.34) on port 443 TCP_conn=2 time=108 ms"}
{"message":"Reply from example.com (93.184.216.34) on port 443 TCP_conn=3 time=108 ms"}
^C{"message":"example.com TCPing statistics: 3 probes transmitted, 3 received"}
{"message":"0.00% packet loss"}
{"message":"successful probes: 3"}
{"message":"unsuccessful probes: 0"}
{"message":"last successful probe:   2022-07-10 17:57:29"}
{"message":"last unsuccessful probe: Never failed"}
{"message":"total uptime: 3 seconds"}
{"message":"total downtime: 0 second"}
{"message":"longest consecutive uptime: 2 seconds from 2022-07-10 17:57:27 to 2022-07-10 17:57:29"}
{"message":"retried to resolve hostname 0 times"}
{"message":"rtt min/avg/max: 108/114.666664/128"}
{"message":"started at: 2022-07-10 17:57:27 ended at: 2022-07-10 17:57:30 duration (HH:MM:SS): 00:00:02"}

The ultimate goal is to have the output in a way that is easily extractable in form of keys and values so with tools such as jq to feed to the monitoring solutions.

Further discussion might be necessary to determine how and what exactly needs to be emitted.

@pouriyajamshidi pouriyajamshidi added enhancement New feature or request good first issue Good for newcomers labels Jul 10, 2022
@doclai
Copy link

doclai commented Jul 22, 2022

We may not just keep them simply as "message" but, for example {"message":"total uptime: 3 seconds"} as key "total uptime" with value "3 seconds". Then we can easily jq the result, for example
echo '{ "total uptime": "3 seconds" }' | jq '."total uptime"'
We may clean some long strings, like "started at: 2022-07-10 17:57:27 ended at: 2022-07-10 17:57:30 duration (HH:MM:SS): 00:00:02"

@pouriyajamshidi
Copy link
Owner Author

Indeed. The goal of this issue is to exactly do that.

@pouriyajamshidi pouriyajamshidi linked a pull request Jul 25, 2022 that will close this issue
@ravsii
Copy link
Contributor

ravsii commented May 7, 2023

Is this still open?

@pouriyajamshidi
Copy link
Owner Author

pouriyajamshidi commented May 7, 2023

Is this still open?

Yes, feel free to reach out in case you wonder what needs to be done.

My example on top is not really "JSON" standard.

@ravsii
Copy link
Contributor

ravsii commented May 7, 2023

@pouriyajamshidi
I did read the code of printers, so here are my thoughts:

  1. Do you want to keep it all in the same file (statsprinter.go)? Maybe we split it into separate plane/json printers?
  2. I'm thinking a good solution for this will be using a one big struct with a bunch of fields that have json:"...,omitempty". There are a lot of info we can put in there, yet keep unnecessary(not related to a specific event) fields empty

Something like

type JSONEvent struct{
	// Mandatory
	EventType string `json:"event_type"` // something like "ping", "stop", "stats", "retry", ... basically one for each "event type"
	Message string `json:"message"` // user-readable message, we could reuse the same message that we have now

	// Optional fields for better parsing.
	// This would duplicate data from message for 3rd party programs to parse
	// so that they don't need to use string split or regex to give info from our messages
	TotalUptime time.duration `json:"total_update,omitempty"`
	TotalDowntime time.duration `json:"total_downtime,omitempty"`
	// other fields...
}

I didn't really think deep about the format, it's just a general IDEA.

P.S. Why are comments using a /* ... */ format?

@pouriyajamshidi
Copy link
Owner Author

pouriyajamshidi commented May 8, 2023

Hey @ravsii,

Do you want to keep it all in the same file (statsprinter.go)? Maybe we split it into separate plane/json printers?

I rather keep things simple and easier to work with. If your proposal of splitting it in another file leads to a cleaner code base, I have no objections.

My main issue is with programs that end up with like 15 different files for a simple thing. Rather common among OOP lovers. 😄

I'm thinking a good solution for this will be using a one big struct with a bunch of fields that have json:"...,omitempty". There are a lot of info we can put in there, yet keep unnecessary(not related to a specific event) fields empty

It is fine. As stated above, it is better to keep things simple. So if some information is not required, there is no need to cram it there.

Nonetheless, think of this JSON output as something closely similar to what we send out to STDOUT. This JSON output will be consumed by a service that is looking for similar information to feed into monitoring systems.

Let me know if you wanna brainstorm further and take your time thinking about a clean solution.

P.S. Why are comments using a /* ... */ format?

Not quite sure if I understood this one. If you mean in the code, you can do multiline comments with that. instead of using // on every line. If I did not understand correctly, feel free to enlighten me.

Thanks a lot!

@ravsii
Copy link
Contributor

ravsii commented May 8, 2023

I rather keep things simple and easier to work with. If your proposal of splitting it in another file leads to a cleaner code base, I have no objections.
My main issue is with programs that end up with like 15 different files for a simple thing. Rather common among OOP lovers. 😄

Guess that's me then, haha. I asked because I often saw such CLI's code is being in the root / directory(so that it could be imported easily), while the entryping for the CLI itself is in /cmd/cli/main.go or something. I guess the main advantage of such approach is not having to specify new files in go build while also splitting CLI and the core logic.

But I was just asking here, it's not a suggestion.

Nonetheless, think of this JSON output as something closely similar to what we send out to STDOUT. This JSON output will be consumed by a service that is looking for similar information to feed into monitoring systems.

Let me know if you wanna brainstorm further and take your time thinking about a clean solution.

How about I try to implement it and submit a draft pull request, so you can see how it turned out and make suggestions? I think there are so many nuances (mainly "results" screen) and it's better to address each individually.

P.S. Why are comments using a /* ... */ format?

Not quite sure if I understood this one. If you mean in the code, you can do multiline comments with that. instead of using // on every line. If I did not understand correctly, feel free to enlighten me.

Sorry, just a nitpick from me. I've never seen /* ... */ comments in a golang codebase, probably because the official article always uses the // style, except for the doc.go file.

Thanks for the answers @pouriyajamshidi, I'll start working on it then!

@pouriyajamshidi
Copy link
Owner Author

Guess that's me then, haha. I asked because I often saw such CLI's code is being in the root / directory(so that it could be imported easily), while the entryping for the CLI itself is in /cmd/cli/main.go or something. I guess the main advantage of such approach is not having to specify new files in go build while also splitting CLI and the core logic.

Right. different projects use different approaches. This project was an excuse for me to learn Go at the time and since there was no official guide on how to structure your program, I just put everything in the root. 😄

How about I try to implement it and submit a draft pull request, so you can see how it turned out and make suggestions? I think there are so many nuances (mainly "results" screen) and it's better to address each individually.

Sounds good to me! Thanks in advance.

Sorry, just a nitpick from me. I've never seen /* ... */ comments in a golang codebase, probably because the official article always uses the // style, except for the doc.go file.

Ah, I see. My C "background" has spilt into Go. 😄

We had another very active contributor who also used that /* */. I think this is more of a personal preference and I don't oppose against any form. They get stripped by the compiler anyway.

Thanks for the answers @pouriyajamshidi, I'll start working on it then!

My pleasure. Thanks and good luck.

P.s. Please make sure to rebase to version 1.21.2. Just pushed some stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants