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

DateTime vs DateTimeOffset vs long (ticks) for timestamps #4

Closed
cwe1ss opened this issue Aug 24, 2016 · 9 comments
Closed

DateTime vs DateTimeOffset vs long (ticks) for timestamps #4

cwe1ss opened this issue Aug 24, 2016 · 9 comments
Assignees
Milestone

Comments

@cwe1ss
Copy link
Member

cwe1ss commented Aug 24, 2016

We have to decide whether we should use DateTime, DateTimeOffset or long (ticks) for timestamps. Unfortunately, it seems like all of them have certain disadvantages...

Interesting articles:

Some important quotes:

The DateTime structure is suitable for applications that do the following:

  • ..., Work with UTC dates and times only, ...
  • When saving or sharing DateTime data, UTC should be used and the DateTime value's Kind property should be set to DateTimeKind.Utc

The DateTimeOffset type includes all of the functionality of the DateTime type along with time zone awareness. This makes it is suitable for applications that do the following:

  • Uniquely and unambiguously identify a single point in time. The DateTimeOffset type can be used to unambiguously define the meaning of "now", to log transaction times, to log the times of system or application events, and to record file creation and modification times.
  • Note: These uses for DateTimeOffset values are much more common than those for DateTime values. As a result, DateTimeOffset should be considered the default date and time type for application development.

Converting:

For UTC and local DateTime values, the Offset property of the resulting DateTimeOffset value accurately reflects the UTC or local time zone offset.

However, for DateTime values whose Kind property is DateTimeKind.Unspecified, these two conversion methods produce a DateTimeOffset value whose offset is that of the local time zone.

Ifyour application requires that converted DateTime values unambiguously identify a single point in time, you should consider using the DateTimeOffset.UtcDateTime property to handle all DateTimeOffset to DateTime conversions.


This means, DateTime would be possible if we would only use UTC. However, for some reason many developers don't use DateTime.UtcNow and DateTime.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 use DateTime, every trace implementation would have to make sure that it either always calls ToUniversalTime() (or only if Kind 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 or DateTimeOffset 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 regular DateTime.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 use DateTime.Now.Ticks or new 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?

@cwe1ss
Copy link
Member Author

cwe1ss commented Aug 29, 2016

@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:

  • just show 09:00:01 and 09:00:02 (UTC)
  • show different times for different users - e.g. operators in each zone? E.g. 04:00:01, 04:00:02 for the US-based operator and 11:00:01, 11:00:02 for the EU-based operator
    • or even some other user-defined timezone - e.g. 10:00:01, 10:00:01 for a UTC+1 based person

In other words, should the library accept a format which allows tracers to extract the timezone offset?

@yurishkuro
Copy link
Member

@cwe1ss the guideline is to use the most common time capture facility in the language, e.g. in Python it's time.time(), in Go it's time.Now(), etc. Usually those functions return epoch-based value. The timezone is not very interesting at collection time, precision is more important. The UI then can use local timezone to convert the timestamps for presentation.

@cwe1ss
Copy link
Member Author

cwe1ss commented Aug 29, 2016

Thanks for your reply, Yuri! Getting the ticks in .NET requires creating a DateTime/DateTimeOffset instance (e.g. DateTime.UtcNow.Ticks), so we can also just accept DateTime/DateTimeOffset and let the trace implementations decide how they want to handle it.

Since we don't need the offset, I think that using just DateTime instead of DateTimeOffset is better. If not explicitly defined, manually created DateTime/DateTimeOffset objects always end up in local time so the user has to know what he does in every case. And since we don't need the offset later, there's no point in using the "larger" DateTimeOffset construct.

Trace implementations must decide whether they want to validate the Kind parameter and e.g. throw if it is not Utc or convert it themselves.

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)

@bhs
Copy link

bhs commented Aug 29, 2016

@cwe1ss @yurishkuro

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.

@cwe1ss
Copy link
Member Author

cwe1ss commented Aug 29, 2016

yes, it would be up to the trace implementation to call timestamp.UtcTicks (which returns Windows ticks) or timestamp.ToUnixTimeMilliseconds() (which returns the epoch version) - dependent on whatever it needs.

@dawallin
Copy link
Contributor

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.
DateTime.UtcNow and DateTime.Now are definitely not used correctly and I guess most users use local time as default until something goes wrong. I don't think naming the parameter to utcStartTime is enough. I agree that we don't need the extra information in DateTimeOffset but it automatically solves the the problem and gives the time i UTC.
I looked at serilog and they seem to use DateTimeOffset for there logging. [https://github.com/serilog/serilog/blob/ee68faec21694d27da94555811bb587a64b6c8d0/src/Serilog/Events/LogEvent.cs]
I vote for DateTimeOffset.

@cwe1ss
Copy link
Member Author

cwe1ss commented Aug 31, 2016

I agree that naming will not be enough!

I looked further into this and found that there is one big difference between using DateTime and DateTimeOffset in regards of how values with DateTimeKind.Unspecified are handled (e.g. manually created values)!

Let's take an example and assume our server is in UTC+2 and someone passes new DateTime(2016, 8, 31, 10, 00, 00) as a value (which results in kind Unspecified).

The question is, should the trace implementation store 2016-08-31 08:00:00 (assuming the user passed a local time) or 2016-08-31 10:00:00 (assuming the user passed a UTC time)?!

  • If we use DateTime, the Unspecified kind will be passed to the trace implementation and the trace implementation can decide whether it wants to throw an exception, use the value as UTC, or as local.
  • If we use DateTimeOffset, values with kind Unspecified are always automatically resolved within the constructor by assuming them to be a local time! This means the trace implementation will get a value with kind Local and if it calls .UtcDateTime, it will get 2016-08-31 08:00:00.

This means:

  • If we use DateTime,
    • the trace implementation can decide about the Kind.
    • A user can't pass a value with a different offset because DateTime only handles Utc/Local. He would have to convert it first.
    • The data structure is smaller
  • If we use DateTimeOffset,
    • we have to be OK with the internal behavior of always taking Unspecified values as Local values.
    • The trace implementation doesn't need to (actually, it can't) do any checks, it can always just call value.UtcDateTime.
    • Users can also pass DateTimeOffset values with arbitrary offsets.
    • The data structure is bigger - but it's just a wrapper over DateTime with an additional Int16 so I don't think that's a relevant factor

PS: For DateTime, a trace implementation could use the following normalize function:

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!");
}

@cwe1ss
Copy link
Member Author

cwe1ss commented Sep 4, 2016

A quick comparison between calling DateTime.UtcNow and DateTimeOffset.UtcNow:

Iterations: 100,000,000
DateTime.UtcNow: 797ms
DateTimeOffset.UtcNow: 2397ms

Given this and my previous comment, I'd vote for using DateTime for the following main reasons:

  • We can let trace implementations decide whether they want to convert non-UTC values.
  • It's the most-known type for users
  • It's a little bit faster
  • We can switch to DateTimeOffset later without breaking consumers because there's an implicit cast. (we would only break trace implementations)

@dawallin you still vote for DateTimeOffset?

@dawallin
Copy link
Contributor

dawallin commented Sep 7, 2016

@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.
I did some more benchmarks, and the really slow operations are DateTime.Now and .ToUniversalTime() on a DateTime with LocalTime. It shows that you should really try to work with UTC instead of local time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants