ddtrace/tracer: don't use UTC() when getting time

[evanj]

Unix time is defined as the duration since January 1, 1970 UTC.
Calling .UTC() does unnecessary work to convert the "location" field
of the time.Time object. The disassembly shows this new
implementation is a few instructions shorter, but I doubt this will
have any measurable performance impact. This is more about
simplifying the existing code.

I tried adding a test and benchmark for this, but the test was pretty silly, and it is unclear if the benchmark showed a measurable difference.

[gbbr]

Since you claim yourself that this doesn't have any measurable impact, why make this change and risk some unexpected regression?

[evanj]

It will save future people the time spent trying to figure out if .UTC() actually does anything (it does not). It likely will also be slightly faster: less code means the compiler is more likely to inline it, it will take up less space in cache, etc, but IIRC the difference was something like 60 instructions vs 50 instructions in the disassembly. This is used in the hottest code path for tracing (e.g. twice on each span), so it might not be measurable, but it might help a bit!

If it helps, I could add the test I created that verifies that .UTC does not matter. It does not change the output of UnixNano at all:

func TestUnixUTC(t *testing.T) {
	now := time.Now()
	unix := now.UnixNano()
	unixUTC := now.UTC().UnixNano()
	if unix != unixUTC {
		t.Errorf("now=%#v unix=%#v != unixUTC=%#v", now, unix, unixUTC)
	}
}

[dianashevchenko]

@gbbr I might be wrong but if we remove .UTC(), and someone decides to look at all the spans from all the instances around the world within a specific timeframe, won't that be faulty? 🤔

[evanj]

@dianashevchenko .UnixNano is defined as "nanoseconds since January 1st 00:00:00 UTC. It is already timezone independent. The value does not change when the timezone of the system changes. The test that I posted in my comment passes, which shows that calling .UTC() does not change the value of .UnixNano().

I should add: I'm totally okay with this change being rejected. I just hate code that does nothing, and find this excess .UTC() to be confusing.

[gbbr]

You're right. It makes sense. Thanks for explaining.