Predefined various timestamp formats #321

gsnw · 2024-06-01T18:48:03Z

This extension allows you to change the format of the timestamp when using fping -D -l 127.0.0.1.
Example: fping -D -l --timestamp-format=ctime 127.0.0.1

Issue: #310

coveralls · 2024-06-01T18:54:02Z

coverage: 85.469% (+0.4%) from 85.117%
when pulling d7a1dce on gsnw:issue/310
into 2f2ff0b on schweikert:develop.

schweikert · 2024-06-09T07:04:38Z

src/fping.c

@@ -2992,6 +3036,7 @@ void usage(int is_error)
    fprintf(out, "   -C, --vcount=N     same as -c, report results (not stats) in verbose format\n");
    fprintf(out, "   -d, --rdns         show targets by name (force reverse-DNS lookup)\n");
    fprintf(out, "   -D, --timestamp    print timestamp before each output line\n");
+    fprintf(out, "       --timestamp-format=FORMAT  show timestamp using the given format (-D are required): [ctime|iso|reltime]\n");


-D is required or -D required to shorten it.

To shorten it a bit more:

show timestamp in the given format (-D required): ctime|iso|reltime\n

So maybe wrapping can be avoided.

Thanks for the feedback and the suggestion.
I have adopted the change and hope the rest of the code is also ok

auerswal

All in all, this looks OK. Please consider my comments, but you do not need to make any changes on my behalf.

Thanks!

auerswal · 2024-06-09T12:00:00Z

ci/test-05-options-c-e.pl

@@ -1,6 +1,6 @@
 #!/usr/bin/perl -w

-use Test::Command tests => 33;
+use Test::Command tests => 42;


Please consider adding two error test cases for the new option:

omitting the format name, and

using an unsupported format name.

I have add two negativ test --timestamp-format and --timestamp-format="%Y-%m-%d %H:%M:%S" both ending in function usage() with exit code 1

auerswal · 2024-06-09T12:02:44Z

src/fping.c

+        current_time_s = current_time_ns / 1000000000;
+        local_time = localtime(&current_time_s);
+        if(timestamp_format == 1) {
+          strftime(time_buffer, sizeof(time_buffer), "%c", local_time);


Before this patch, strftime is not used in the fping code. Should configure check for availability of strftime?

It's also done

auerswal · 2024-06-09T12:03:27Z

src/fping.c

+    if(timestamp_format >= 1 && timestamp_format <= 3) {
+        current_time_s = current_time_ns / 1000000000;
+        local_time = localtime(&current_time_s);
+        if(timestamp_format == 1) {


I'd think this part (i.e., the inner if) would be more readable using switch.

The if statement has been completely converted into a switch statement. Many thanks for this feedback.
Switch statement with the default is also better than the old if else.
This way, the default is always used if the function is passed incorrectly

auerswal · 2024-06-09T12:42:54Z

Please also consider adding a description to the man page, i.e., fping.pod. Thanks!

auerswal · 2024-06-09T23:38:04Z

I am not so sure any more, especially regarding reltime. With dmesg, --time-format=reltime provides relative time stamps for short time differences, and absolute time stamps for longer gaps, e.g.:

[Jun 9 21:17] …
[ +21,801486] …
[  +0,000004] …
[  +0,000004] …
[  +0,000002] …
[  +0,000001] …
[  +0,091934] …
[  +0,000003] …
[  +0,000002] …
[  +0,000002] …
[  +0,000001] …
[  +0,000004] …
[  +0,574391] …
[Jun 9 22:54] …

Perhaps we should leave out reltime for now?

And a nit: both date --iso-8601 and dmesg --time-format=iso print the timezone offset with a colon between hours and minutes (HH:MM). The %z format specifier does not include the colon, but I did not see a better format specifier in man strftime.

(As another example, GNU date has two quite similar "named" time format options where the output differs mainly with the character between date and time, either a space or a T: date --rfc-3339=s and date --iso-8601=s [--rfc-3339 does not use the locale settings, --iso-8601 does, but fping does not use locale settings (IMHO fping is better in that regard)].)

gsnw · 2024-06-10T06:41:57Z

The formats are not perfect. I have now turned reltime into rfc3339, even with the knowledge that the format may not be one hundred percent perfect.
However, it corresponds to the example from the ticket.

[1] https://medium.easyread.co/understanding-about-rfc-3339-for-datetime-formatting-in-software-engineering-940aa5d5f68a?gi=fc35d513c0fd

auerswal

Thanks for the adjustments, this looks good. :-)

auerswal · 2024-06-15T17:14:05Z

We haven't talked about the process for merging pull requests yet. I would be willing to merge this pull request next weekend, unless there are objections. Is that OK?

gsnw · 2024-06-22T08:21:06Z

The last github merges were probably done with rebase.
Everything has advantages and disadvantages.

All my pull requests are always based on the current state of develop and I always try to do a rebase on all branches in my repository when the develop branch changes here.

So I would prefer rebase and merge

auerswal · 2024-06-22T11:35:53Z

Ah, I see, that is one of the options shown when clicking the arrow next to Merge pull request. I think I (slightly) prefer that option too, because automatic merge commits add some noise, but only little information. I'll go ahead and do that, thanks!

(For the record: I do not like the Squash and merge option, I prefer to keep the individual commits.)

gsnw mentioned this pull request Jun 1, 2024

-D, --timestamp with custom time string eq: %Y-%m-%d %H:%M:%S #310

Closed

gsnw changed the title ~~Issue/310~~ Predefined various timestamp formats Jun 1, 2024

gsnw force-pushed the issue/310 branch 4 times, most recently from 2d2d3a3 to 8ee7a8a Compare June 1, 2024 20:20

schweikert reviewed Jun 9, 2024

View reviewed changes

schweikert approved these changes Jun 9, 2024

View reviewed changes

Handling optparse long option

9029b41

gsnw force-pushed the issue/310 branch from 8ee7a8a to 225de57 Compare June 9, 2024 08:33

auerswal approved these changes Jun 9, 2024

View reviewed changes

gsnw-sebast added 3 commits June 10, 2024 08:17

New parameter timestamp-format for changing the timestamp output

9dc3c7d

Test 05 for --timestamp-format extended

05a36e1

Extended dokumentation for parameter timestamp-format

be4cdc8

gsnw force-pushed the issue/310 branch from 225de57 to be4cdc8 Compare June 10, 2024 06:21

configure.ac check required function strftime

d7a1dce

auerswal approved these changes Jun 15, 2024

View reviewed changes

auerswal merged commit 118cdc5 into schweikert:develop Jun 22, 2024
9 checks passed

gsnw deleted the issue/310 branch June 23, 2024 05:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Predefined various timestamp formats #321

Predefined various timestamp formats #321

gsnw commented Jun 1, 2024

coveralls commented Jun 1, 2024 •

edited

Loading

schweikert Jun 9, 2024

schweikert Jun 9, 2024

gsnw Jun 9, 2024

auerswal left a comment

auerswal Jun 9, 2024

gsnw Jun 10, 2024

auerswal Jun 9, 2024

gsnw Jun 10, 2024

auerswal Jun 9, 2024 •

edited

Loading

gsnw Jun 10, 2024

auerswal commented Jun 9, 2024

auerswal commented Jun 9, 2024

gsnw commented Jun 10, 2024

auerswal left a comment

auerswal commented Jun 15, 2024

gsnw commented Jun 22, 2024

auerswal commented Jun 22, 2024

Predefined various timestamp formats #321

Predefined various timestamp formats #321

Conversation

gsnw commented Jun 1, 2024

coveralls commented Jun 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

auerswal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

auerswal Jun 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

auerswal commented Jun 9, 2024

auerswal commented Jun 9, 2024

gsnw commented Jun 10, 2024

auerswal left a comment

Choose a reason for hiding this comment

auerswal commented Jun 15, 2024

gsnw commented Jun 22, 2024

auerswal commented Jun 22, 2024

coveralls commented Jun 1, 2024 •

edited

Loading

auerswal Jun 9, 2024 •

edited

Loading