feat: and start_nanos, end_nanos, timezone to PdlBlock model #285
Conversation
starpit
force-pushed
the
timing
branch
6 times, most recently
from
1dfe5b8 to
5601785
Compare
| """ | ||
| timezone: Optional[str] = "" | ||
| """Timezone of start_nanos and end_nanos | ||
| """ | ||
| # Fields for internal use |
There was a problem hiding this comment.
I would move the new fields under this comment.
| now = datetime.datetime.now() | ||
| local_now = now.astimezone() | ||
| local_tz = local_now.tzinfo | ||
| if local_tz is not None: | ||
| local_tzname = local_tz.tzname(local_now) | ||
| else: | ||
| local_tzname = "UTC" | ||
| d["timezone"] = local_tzname |
There was a problem hiding this comment.
What about computing the time zone once and setting it up in the interpreter in the step_advanced_block_timed function?
There was a problem hiding this comment.
i'm not sure if there is a great solution to this. either we do it outside of block_to_dict(), but then we would either need to pass this value in as a parameter, or we would need to store the timezone as a global variable. by design, it seems that the trace format does not allow for a global wrapper, where we could compute and store the timezone (and any other trace metadata, e.g. who created it, when it was created, etc.).
an alternate solution would be to store the start and end timestamps as formatted timestamps (this is how kubernetes does it), so that the timezone is encoded into each timestamp. but then we are formatting timestamps all the time... is this less expensive than computing the timezone? dunno.
in any case, i propose that this is premature optimization. computing the timezone once per non-scalar block seems like a minor expense compared to the cost of serializing the block.
also, i intentionally placed the timezone code in the dumper, to pull it out of the path of the normal interpretation. i.e. this extra expense is only occurred when running in --trace mode.
Also updates the react UI to display this information. Signed-off-by: Nick Mitchell <[email protected]>
Also updates the react UI to display this information.