Remove log record timestamp default #5374
Conversation
| @@ -100,6 +115,7 @@ private SdkEventEmitter(Logger delegateLogger, String eventDomain) { | |||
| public void emit(String eventName, Attributes attributes) { | |||
| delegateLogger | |||
| .logRecordBuilder() | |||
| .setTimestamp(clock.now(), TimeUnit.NANOSECONDS) | |||
There was a problem hiding this comment.
In contrast to the log API, the event API should automatically assign the timestamp. In part because its inconvenient for the user to specify it, but we also currently don't accept any parameters besides the event name and attribute when emitting an event.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #5374 +/- ##
============================================
+ Coverage 91.20% 91.23% +0.02%
- Complexity 4877 4878 +1
============================================
Files 549 549
Lines 14402 14404 +2
Branches 1352 1351 -1
============================================
+ Hits 13136 13141 +5
+ Misses 877 875 -2
+ Partials 389 388 -1
... and 3 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
The spec issue discussing this has been resolved. This PR now reflects the intent of the spec. |
Reflects spec issue #3386.
As mentioned here, this further demonstrates that the Bridge API is not for end users. It's trivial for a appender component to map the timestamp from the log record. It will be inconvenient and awkward for an end user to specify the timestamp on each log if they try to misuse the log bridge API.
Without this default behavior, there's no need for SdkLoggerProvider to have a reference to a
Clock, so the code simplifies somewhat.