Skip to content

sdk-logs: default event timestamp on emit#8114

Closed
jeet7122 wants to merge 4 commits intoopen-telemetry:mainfrom
jeet7122:fix-8097-logrecord-timestamp
Closed

sdk-logs: default event timestamp on emit#8114
jeet7122 wants to merge 4 commits intoopen-telemetry:mainfrom
jeet7122:fix-8097-logrecord-timestamp

Conversation

@jeet7122
Copy link

Fixes #8097

What this changes

*Default event timestamp to clock.now() in SdkLogRecordBuilder.emit() when not explicitly set.

  • This aligns behavior with the setTimestamp javadoc (timestamp should default at emit time).
  • Updated emit_NoFields test accordingly.

Testing

  • Ran :sdk:logs:test

  • Opening as a draft PR to coordinate and avoid duplicate work if someone else is already working on this.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 24, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeet7122 jeet7122 marked this pull request as ready for review February 24, 2026 23:23
@jeet7122 jeet7122 requested a review from a team as a code owner February 24, 2026 23:23
@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (b089521) to head (0e6f815).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8114      +/-   ##
============================================
- Coverage     90.33%   90.32%   -0.01%     
+ Complexity     7614     7612       -2     
============================================
  Files           839      839              
  Lines         22913    22913              
  Branches       2285     2285              
============================================
- Hits          20698    20696       -2     
  Misses         1505     1505              
- Partials        710      712       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jeet7122
Copy link
Author

Thanks for the clarification! I reverted the behavior change and updated the setTimestamp javadoc to align with the spec.

@jeet7122
Copy link
Author

I’ve updated the javadocs to align with the spec and pushed the changes.
Could you please take another look when you get a chance?

@jeet7122 jeet7122 requested a review from jack-berg February 27, 2026 00:35
@jack-berg
Copy link
Member

Closing because the issue is already solved in #8104. In the future, look out for existing PRs for an issue to avoid wasted effort. If there's an existing PR, the approach is reasonable, and the author is responsive, it will be preferred over later PRs. Thanks!

@jack-berg jack-berg closed this Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SdkLogRecordBuilder does not properly set default timestamp

2 participants