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

unify timestamps #16

Open
libor-peltan-cznic opened this issue May 30, 2024 · 9 comments
Open

unify timestamps #16

libor-peltan-cznic opened this issue May 30, 2024 · 9 comments

Comments

@libor-peltan-cznic
Copy link

There is a mess in various absolute and relative timestamps throughout the schema. There are two main questions with several options:

  1. time units used:
    a. nanoseconds
    b. microseconds
    c. configurable by time_units_per_sec in the header

  2. baseline for absolute timestamps (e.g. "since" and "until" in the interval results):
    a. 1970-01-01
    b. start of measurement
    c. whatever random timestamp that might be anything but fixed for the whole measurement (aka CLOCK_MONOTINIC)
    d. like b. with the start of measurement relativized to 1970-01-01 in the header
    e. like c. with the "anything" relativized to 1970-01-01 in the header

In 1), I personally vote for b. (usecs). In 2), I have no strong preference, possibly c. is OK for me.

@pspacek
Copy link

pspacek commented May 30, 2024

Ad time units - I would go with simplest form, i.e. not configurable. Nanoseconds are probably fine as it is the default resolution nowadays (clock_gettime()).

Ad baseline value - thinking out loud:
A timestamps are easier for generator, relative for postprocessing as it often needs to combine and compare data from multiple runs. Having said that, I kinda like the idea of having single well specified timestamp in the header and then all other timestamp values relative to that. (I'm not sure which variant that is in your list.)

@nicki-krizek
Copy link

  1. I'm okay with both (a) and (b). I prefer to settle on one option rather than making it configurable (c).
  2. I think having a record of when the test took place can be useful, so I like (a). However, (d) and (e) would also be sufficient for that. Perhaps the baseline could be an optional field in the header - if your tool cares about it, then fill it in.

@Spiffyk
Copy link

Spiffyk commented May 30, 2024

  1. Both (a) and (b) sound fine to me - no strong inclination towards either. (c) feels like a needless complication.

  2. I personally like (d) the best, as it:

    a. potentially helps one see how things went during the single run while just looking at the stream even in the JSON not yet processed into charts - from the PoV of Shotgun, it may be easier to see patterns related to the replayed PCAP when replayed multiple times.

    b. still keeps record of the absolute time. I agree with @nicki-krizek about the baseline's optionality - it may not necessarily be relevant for all tools.

@Spiffyk
Copy link

Spiffyk commented May 30, 2024

@pspacek: Having said that, I kinda like the idea of having single well specified timestamp in the header and then all other timestamp values relative to that. (I'm not sure which variant that is in your list.)

Sounds like (d), just that the header timestamp is not necessarily defined to be the start of measurement, which may have some merit - just call it time_offset or something like that, and let the tool decide what that actually means... ?

(Or, a wild idea, probably without practical use: have a separate time_offset, then start_of_measurement relative to that time_offset. When in doubt, add another layer of indirection 😅)

@libor-peltan-cznic
Copy link
Author

OK, so let's conclude what shall be done (according to seemingly consensus):

  1. all timestamps should be in nanosecond precision
  2. all common timestamps are relative to an arbitrary but fixed baseline
  3. in the header, there MAY be optionally specification of the baseline relatively to unixtime
  4. in the header, there MAY be optionally specification of the start of measurement relatively to the baseline

@pspacek
Copy link

pspacek commented May 30, 2024

I think we are over-complicating it. It already sounds like something designed by a committee!

What about this:

{"type": "header", "start_time_unix": 123456789123456789}
{"type": "stats_periodic", "since_reltime": 0, ...}
{"type": "stats_periodic", "since_reltime": 1000000000, ...}

And be done with it?

Of course names can be discussed to death. The "start_time_unix" can be optional.

@pspacek
Copy link

pspacek commented May 30, 2024

Or wait for it. The initial timestamp could be an ISO string instead of ill-defined unix timestamp :-)

@nicki-krizek
Copy link

I don't see the use-case for having a separate start of measurement (relative to baseline), so I'd rather keep it simple and just have the baseline in the header.

Or wait for it. The initial timestamp could be an ISO string instead of ill-defined unix timestamp :-)

Again, I'd keep it simple. JSON is not for really meant viewing / human consumption. Having a readable time format is a display feature and it's something the processing tools should take care of. IMO adding time formatting complexity to all the generator tools (written in C/C++, lua, rust, go or who knows what) isn't worth a "nice" value in the header.

@pspacek
Copy link

pspacek commented May 30, 2024

I agree, I was just trolling with even more complex proposal. Are we agreement then? Something stupidly simple as #16 (comment)?

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

No branches or pull requests

4 participants