Add ReadWriteLogRecord#getContext #4888
Conversation
Codecov ReportBase: 90.83% // Head: 90.86% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #4888 +/- ##
============================================
+ Coverage 90.83% 90.86% +0.02%
- Complexity 4857 4861 +4
============================================
Files 556 556
Lines 14475 14477 +2
Branches 1410 1411 +1
============================================
+ Hits 13149 13154 +5
+ Misses 908 906 -2
+ Partials 418 417 -1
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 at Codecov. |
| @@ -25,7 +26,7 @@ class SdkReadWriteLogRecord implements ReadWriteLogRecord { | |||
| private final Resource resource; | |||
| private final InstrumentationScopeInfo instrumentationScopeInfo; | |||
| private final long epochNanos; | |||
| private final SpanContext spanContext; | |||
| private final Context context; | |||
There was a problem hiding this comment.
I think(?) the reason that SdkSpan only contains SpanContext instead of the full Context was to limit memory retention, @jkwatson do you recall anything?
There was a problem hiding this comment.
Ok so there is a subtle difference between holding it in the SdkReadWriteLogRecord and holding it in the SdkLogRecordBuilder such that it can be passed to each log processor as an argument LogRecordProcessor#onEmit.
If its held in SdkReadWriteLogRecord, there's a reference to Context until BatchLogRecordProcessor flushes and converts each log record to LogRecordData, so the reference sticks around for essentially the shorter of the batch interval or the time it takes to fill the batch queue.
If instead the context is held in SdkLogRecordBuilder and passed as an argument to each LogRecordProcessor#onEmit(), then the reference only has to stick around until the last processor has synchronously returns from onEmit. This allows the context to be garbage collected while the corresponding log record is still sitting in the BatchLogRecordProcessor queue.
I think this is a good enough reason to include as an argument to onEmit() instead of having a getter on SdkReadWriteLogRecord.
|
Closing in favor of #4889. |
Resolves #4147.