-
Notifications
You must be signed in to change notification settings - Fork 870
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
Add LogRecord observed timestamp field #5370
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. While I can't imagine that there are huge numbers of use cases for this field in the java side of things, it's best to have the api match the spec for completeness.
...nternal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/FakeTelemetryUtil.java
Show resolved
Hide resolved
sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java
Show resolved
Hide resolved
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #5370 +/- ##
============================================
+ Coverage 91.20% 91.23% +0.02%
- Complexity 4876 4882 +6
============================================
Files 549 549
Lines 14402 14425 +23
Branches 1352 1354 +2
============================================
+ Hits 13136 13160 +24
Misses 877 877
+ Partials 389 388 -1
... and 1 file 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. |
b873381
to
679c034
Compare
The log data model differentiates between timestamp and observed timestamp.
Timestamp is the time at which the log occurred. Observed timestamp is the time at which the log was recorded in the collection system.
Timestamp is different than observed timestamp if the logs are being processed asynchronously. For example, maybe the logs are being read from a file, or are being processed on a different thread than where they occur. These will be very frequently the same for the SDK, but occasionally they will be different.
We could wait to add setters in the API until users request it, but we really should minimally be including getters on LogRecordData, and we really should populate the LogRecord#observed_time_unix_nano when serializing OTLP payloads.
IMO, we might as well go all the way and add the API setters.