use Date.now() for instrument recording timestamps

[MisterSquishy]

Which problem is this PR solving?

Fixes metric timestamps in the distant past in the browser, related to timestamp drift described extensively in #3279. However, the fix is much simpler here than in the tracing case -- as per this comment, we have no need for monotonicity here and can safely just use Date.now()

Fixes #3383

Short description of the changes

changes the implementation of SyncInstrument._record to use timeInputToHrTime(Date.now()) instead of hrTime(). The latter uses the performance API, which is overkill in this situation (we don't need monotonicity) and is often way behind real time on many browsers.

This comment indicates that we should wait for consensus here, as we might instead introduce some sort of global clock API. I don't object to waiting, but it seems to me that we should fix the bug before the (probably lengthy) debate/implementation of a clock API. The default clock is broken in the browser context, and that should take priority over nontraditional runtimes that can't use Date. But I don't mind being overruled here; it's not like this fix took me very long to implement.

Type of change

• Bug fix (non-breaking change which fixes an issue)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: MisterSquishy / name: Pete Davids (5821090, d1693ac)

[codecov]

Codecov Report

Merging #3514 (3cac2ba) into main (c87a304) will increase coverage by 0.75%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3514      +/-   ##
==========================================
+ Coverage   93.03%   93.79%   +0.75%     
==========================================
  Files         242      249       +7     
  Lines        7167     7640     +473     
  Branches     1502     1589      +87     
==========================================
+ Hits         6668     7166     +498     
+ Misses        499      474      -25     

Impacted Files	Coverage Δ
packages/sdk-metrics/src/Instruments.ts	95.23% <100.00%> (ø)
packages/sdk-metrics/src/aggregator/LastValue.ts	100.00% <100.00%> (ø)
packages/sdk-metrics/src/state/MetricCollector.ts	100.00% <100.00%> (ø)
...ry-context-zone-peer-dep/src/ZoneContextManager.ts	85.00% <0.00%> (ø)
...mentation-xml-http-request/src/enums/EventNames.ts	100.00% <0.00%> (ø)
...ation-xml-http-request/src/enums/AttributeNames.ts	100.00% <0.00%> (ø)
...-instrumentation-fetch/src/enums/AttributeNames.ts	100.00% <0.00%> (ø)
...emetry-instrumentation-xml-http-request/src/xhr.ts	97.59% <0.00%> (ø)
...es/opentelemetry-context-zone-peer-dep/src/util.ts	100.00% <0.00%> (ø)
...s/opentelemetry-instrumentation-fetch/src/fetch.ts	97.02% <0.00%> (ø)
... and 1 more

[legendecas]

Thank you for working on this!

I find 2 additional call sites of hrTime in the sdk-metrics that can be replaced:

• https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/sdk-metrics/src/aggregator/LastValue.ts#L40
• https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/sdk-metrics/src/state/MetricCollector.ts#L39

Would you mind updating these call sites too?

[pichlermarc]

Whoops, may have miscommunicated - no need to even update the version, all versions are updated automatically via lerna when we create a new release. 🙂
Leaving versions as-is is fine 🙂

[MisterSquishy]

Whoops, may have miscommunicated - no need to even update the version, all versions are updated automatically via lerna when we create a new release. 🙂 Leaving versions as-is is fine 🙂

oh wow even easier!!

[pichlermarc]

Looks good, thank you for contributing 🙂