-
Notifications
You must be signed in to change notification settings - Fork 73
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
DateTime vs DateTimeOffset vs long (ticks) for timestamps #4
Comments
@bensigelman @yurishkuro Do trace implementations care about timezones in any kind or do they always display everything in UTC? Example: In a multi-region system with "Zone Europe" and "Zone US", in which a message flows from Europe (11:00:01 local [UTC+2]) to US (04:00:02 local [UTC-5]), does the system:
In other words, should the library accept a format which allows tracers to extract the timezone offset? |
@cwe1ss the guideline is to use the most common time capture facility in the language, e.g. in Python it's |
Thanks for your reply, Yuri! Getting the ticks in .NET requires creating a Since we don't need the offset, I think that using just Trace implementations must decide whether they want to validate the To make it even clearer, we could give the parameters a "utc" prefix - e.g. "utcStartTime" instead of "startTimestamp". @dawallin what do you think? PS: Since there also is an implicit cast from DateTime to DateTimeOffset, we could switch to DateTimeOffset later without breaking consumers. (But it would break trace implementations of course) |
While I think it's fine to use the idiomatic time function per-platform, I feel somewhat strongly that timezones should be avoided in monitoring ingest APIs, except when human-user-facing frontends are involved. I.e., whatever time type we use should support something like epoch microseconds. |
yes, it would be up to the trace implementation to call |
I think ticks feels unnatural in C#, and since every implementation can choose to cast the value to the tick type it want I think DateTime/DateTimeOffset is better. |
I agree that naming will not be enough! I looked further into this and found that there is one big difference between using Let's take an example and assume our server is in UTC+2 and someone passes The question is, should the trace implementation store
This means:
PS: For private DateTime Normalize(DateTime timestamp)
{
if (timestamp.Kind == DateTimeKind.Utc)
return timestamp;
if (timestamp.Kind == DateTimeKind.Local)
return timestamp.ToUniversalTime();
// Someone manually created a value. We don't want to convert it and assume it's UTC!
return DateTime.SpecifyKind(timestamp, DateTimeKind.Utc);
// ... or we could throw an exception.
//throw new NotSupportedException("Please specify a Kind!");
} |
A quick comparison between calling
Given this and my previous comment, I'd vote for using
@dawallin you still vote for |
@cwe1ss I have no strong opinion, I have no experience with DateTimeOffset, and as you say, if we choose DateTime, we can change later without breaking the client. |
DateTime instead of DateTimeOffset; resolves #4
We have to decide whether we should use
DateTime
,DateTimeOffset
orlong
(ticks) for timestamps. Unfortunately, it seems like all of them have certain disadvantages...Interesting articles:
Some important quotes:
Converting:
This means,
DateTime
would be possible if we would only use UTC. However, for some reason many developers don't useDateTime.UtcNow
andDateTime.Now
properly. While the former correctly returns a UTC timestamp, the latter returns a timestamp for the current timezone on which the host is running - and this of course is wrong for server scenarios.Since it's not guaranteed that everybody uses
UtcNow
, if we would useDateTime
, every trace implementation would have to make sure that it either always callsToUniversalTime()
(or only ifKind
is "Local"). there's also DateTimeKind.Unspecified which seems to be handled similar to local - this is the kind you get when you manually create a DateTime object.DateTimeOffset
seems to be more explicit about all of this, however it still uses the local server time zone when you pass a date with kind=Unspecified.By using
DateTime
orDateTimeOffset
we would need to define some recommendations - e.g. Users should always use UTC, tracer implementations should convert kind=local to UTC, tracer implementations should throw an exception if kind=Unspecified, ...Using
long ticks
would be an alternative which maybe would make it a bit more obvious for users that they can't just pass a regularDateTime.Now
and instead have to think about the right thing to do. However, I guess you might still run into the same issues if you useDateTime.Now.Ticks
ornew DateTime(...).Ticks
with an unspecified kind. I haven't yet tested this though.Other drawbacks of using ticks are that the trace implementations now can't do any validation and it's not obvious for users and trace implementations, whether the ticks are UNIX epoch ticks or Windows ticks.
Your opinions?
The text was updated successfully, but these errors were encountered: