Add CSV Printer to tcping

[SYSHIL]

Important

Please provide the requested parameters below to help us review your pull request.

Pull requests without a body or explanation will be rejected.

Describe your changes

Issue ticket number and link

Closes #
github.com//issues/240

Checklist before requesting a review

• I have performed a self-review of my code
• If it is a core feature, I have added tests.
• I have run make check and there are no failures.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

[SYSHIL]

Hey @pouriyajamshidi could you review this whenever you find time? Thanks !

[pouriyajamshidi]

Hey @pouriyajamshidi could you review this whenever you find time? Thanks !

Hey @SYSHIL

Yes, it is on my radar. Will get to it probably this weekend.

Thanks for your help

[pouriyajamshidi]

I think the gentleman who has opened this issue wanted a similar result as to the terminal output rather than the DB output we currently offer.

He has made a video here:

www.youtube.com/watch?v=_bXs0YFcknw

[SYSHIL]

@pouriyajamshidi Got it thanks for the information, I will pick this later this week if that's okay? Work has been a lil busy lately :)

[pouriyajamshidi]

@pouriyajamshidi Got it thanks for the information, I will pick this later this week if that's okay? Work has been a lil busy lately :)

Sure! Thanks for your help and hope things get easier a bit on your end.

[SYSHIL]

Hey @pouriyajamshidi Thanks for your patience, I got some time now so will be working on this :)
I have a question though how do you think we should handle statistics output while working with csv files?
I see two options,

1. PrintStats function can be implemented by our csv printer but we should maybe pipe the output to a separate csv file with a different schema compared to the csv file we'll send our terminal output to
2. Ignore printing statistics in case our printer is of type csv

[pouriyajamshidi]

Hey @pouriyajamshidi Thanks for your patience, I got some time now so will be working on this :) I have a question though how do you think we should handle statistics output while working with csv files? I see two options,

1. PrintStats function can be implemented by our csv printer but we should maybe pipe the output to a separate csv file with a different schema compared to the csv file we'll send our terminal output to
2. Ignore printing statistics in case our printer is of type csv

Hey @SYSHIL,

Welcome back!

That is a good question, and I also had the same thought when I was reading your question.

Perhaps you can also ask it on the issue from him?

Ultimately, I think it is better to have that output since other printers are providing it. A separate file sounds good.

[SYSHIL]

Hey @pouriyajamshidi 👋 Could you please review once you find sometime?

Ultimately, I think it is better to have that output since other printers are providing it. A separate file sounds good.

The gentleman that requested the feature is mostly looking at getting the regular output through CSV but I totally agree with your point, We should keep consistency between other printers

[pouriyajamshidi]

Hey @pouriyajamshidi 👋 Could you please review once you find sometime?

Ultimately, I think it is better to have that output since other printers are providing it. A separate file sounds good.

The gentleman that requested the feature is mostly looking at getting the regular output through CSV but I totally agree with your point, We should keep consistency between other printers

Hey @SYSHIL,

Thanks, will do.

[pouriyajamshidi]

Hey @SYSHIL,

Thanks for yet another contribution.

Overall it looks good.

Since I am preparing for a release, I need this merged. However, there are some inconsistencies like local address not being printed when there's a probe failure and minor other nuances that I will address in a separate PR because changes to this PR by me will overwrite your name in squash merge.

Thanks again mate!

[SYSHIL]

Hey @pouriyajamshidi, Oh okay thank you for pointing that out :)

[pouriyajamshidi]

Hey @SYSHIL,

If you are free to contrib more, maybe add the missing "local address not being printed when there's a probe failure"?

If you can do this today or tomorrow, that'd be perfect. Otherwise, I will add it myself.

[SYSHIL]

Heyy @pouriyajamshidi sure, I should be able to pick this up later today

[pouriyajamshidi]

Heyy @pouriyajamshidi sure, I should be able to pick this up later today

Thanks mate

[SYSHIL]

Heyy @pouriyajamshidi I was about to work on adding the local address during probe failures but I see that its also omitted with the statsprinter, I think that's why I left it out in my PR 😅

Here's some demo output,

Reply from google.com (2404:6800:4007:81e::200e) on port 80 using [2401:4900:4df6:b4a5:b56d:6c95:1b82:db1a]:52087 TCP_conn=4 time=102.435 ms
Reply from google.com (2404:6800:4007:81e::200e) on port 80 using [2401:4900:4df6:b4a5:b56d:6c95:1b82:db1a]:52088 TCP_conn=5 time=52.676 ms
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=1
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=2
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=3
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=4

[pouriyajamshidi]

Heyy @pouriyajamshidi I was about to work on adding the local address during probe failures but I see that its also omitted with the statsprinter, I think that's why I left it out in my PR 😅

Here's some demo output,

Reply from google.com (2404:6800:4007:81e::200e) on port 80 using [2401:4900:4df6:b4a5:b56d:6c95:1b82:db1a]:52087 TCP_conn=4 time=102.435 ms
Reply from google.com (2404:6800:4007:81e::200e) on port 80 using [2401:4900:4df6:b4a5:b56d:6c95:1b82:db1a]:52088 TCP_conn=5 time=52.676 ms
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=1
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=2
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=3
No reply from google.com (2404:6800:4007:81e::200e) on port 80 TCP_conn=4

Hey @SYSHIL

My bad then! Sorry.

[SYSHIL]

np @pouriyajamshidi, I did have some improvements in mind though. Could you please review this small PR whenver you find time? #276

[pouriyajamshidi]

np @pouriyajamshidi, I did have some improvements in mind though. Could you please review this small PR whenver you find time? #276

sure