json: add UNIX timestamp in milliseconds #1846
Conversation
src/iperf_api.c
Outdated
| if (test->json_output) | ||
| cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s timesecs: %d", now_str, (int64_t) now_secs)); | ||
| cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s timesecs: %d timestamp_ms: %d", now_str, (int64_t) time_spec.tv_sec, now_millisecs)); |
There was a problem hiding this comment.
hint for reviewers: The %d is correct as iperf_json_printf() only knows %d for json numbers, no %lld or similar
phip1611
force-pushed
the
timestamp
branch
2 times, most recently
from
f8f2e44 to
1f6e6f3
Compare
There was a problem hiding this comment.
I see the need for a more accurate timestamp than second granularity occasionally. I like that this adds a separate new key instead of increasing the precision of existing timestamps. This should cause less churn for tools working with the existing schema.
I found an issue in the timestamp computation though.
| if (test->json_output) | ||
| cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s timesecs: %d", now_str, (int64_t) now_secs)); | ||
| cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s timesecs: %d timemillisecs: %d", now_str, (int64_t) time_spec.tv_sec, now_millisecs)); |
There was a problem hiding this comment.
nit: I won't claim I like that new key name. I see it fits with the existing one. Shorter alternatives would be time_ms or timemsecs ... I'm fine with using the proposed one here.
There was a problem hiding this comment.
Any preference by the code owners? @bmah888
This extends the "timestamp" field with a UNIX timestamp in milliseconds
representing the current local wall clock time.
The "timestamp" field now has a new subkey:
"timestamp": {
"time": "Tue, 25 Feb 2025 11:49:59 GMT",
"timesecs": 1740484199,
// new
"timemillisecs": 1740484199926
},
This is helpful when one does fine-grained network tests with iperf,
where iperf events need to be aligned with events coming from other
resources. A time resolution based on seconds is too coarse-grained
for that.
It is enough to use milliseconds here, as networking always comes
with small timing offsets and delays. Using microseconds wouldn't
add a benefit.
It is sufficient to only change the code at this place, as the rel time
offset for all further events already includes milliseconds. Currently,
iperf3 prints this time offset in seconds encoded as a double with
a microsecond granularity.
Signed-off-by: Philipp Schuster <[email protected]>
|
Thanks for the pull request! This seems like a nice, straightforward addition, so we'll try to get to it soon. |
json: add UNIX timestamp in milliseconds
This extends the "timestamp" field with a UNIX timestamp in milliseconds
representing the current local wall clock time.
The "timestamp" field now has a new subkey:
This is helpful when one does fine-grained network tests with iperf,
where iperf events need to be aligned with events coming from other
resources. A time resolution based on seconds is too coarse-grained
for that.
It is enough to use milliseconds here, as networking always comes
with small timing offsets and delays. Using microseconds wouldn't
add a benefit.
It is sufficient to only change the code at this place, as the rel time
offset for all further events already includes milliseconds. Currently,
iperf3 prints this time offset in seconds encoded as a double with
a microsecond granularity.
Signed-off-by: Philipp Schuster [email protected]
PLEASE NOTE the following text from the iperf3 license. Submitting a
pull request to the iperf3 repository constitutes "[making]
Enhancements available...publicly":
The complete iperf3 license is available in the
LICENSEfile in thetop directory of the iperf3 source tree.
Version of iperf3 (or development branch, such as
masteror3.1-STABLE) to which this pull request applies:Issues fixed (if any):
Brief description of code changes (suitable for use as a commit message):