opentelemetry-java icon indicating copy to clipboard operation
opentelemetry-java copied to clipboard

Allow logs to be mutated by LogProcessor

Open jack-berg opened this issue 2 years ago • 2 comments

Implements spec PR#2681. Resolves #4550.

Summary:

  • Rename LogBuilder to LogRecordBuilder to better reflect the data model and the naming we can expect to see in the log appender / event API.
  • Change LogProcessor#emit to onEmit to better match trace SDK.
  • Introduce ReadWriteLogRecord interface, which is passed to LogProcessor#onEmit instead of LogData. This is roughly aligned with tracing. However, in tracing ReadWriteSpan extends Span and ReadableSpan. This separation of Span for updating and ReadableSpan for reading isn't needed in the log SDK because there isn't a LogProcessor onStart and onEnd which have different read / write semantics.
  • ReadWriteLogRecord has a toLogData() method, which is called by the simple and batch log processor to convert the data to an immutable representation before calling LogExporter#export(Collection<LogData>).
  • Add single attribute setters (setAttribute(AttributeKey<T>, T)), rename multi attribute setter to setAllAttributes(Attributes) to mirror tracing

jack-berg avatar Jul 27 '22 22:07 jack-berg

Codecov Report

Merging #4643 (caff25a) into main (77be2e0) will decrease coverage by 0.01%. The diff coverage is 83.33%.

@@             Coverage Diff              @@
##               main    #4643      +/-   ##
============================================
- Coverage     90.08%   90.06%   -0.02%     
- Complexity     5074     5085      +11     
============================================
  Files           584      586       +2     
  Lines         15667    15707      +40     
  Branches       1504     1507       +3     
============================================
+ Hits          14114    14147      +33     
- Misses         1100     1103       +3     
- Partials        453      457       +4     
Impacted Files Coverage Δ
...n/java/io/opentelemetry/sdk/logs/LogProcessor.java 85.71% <ø> (ø)
...lemetry/sdk/logs/SdkLogEmitterProviderBuilder.java 100.00% <ø> (ø)
...va/io/opentelemetry/sdk/logs/LogRecordBuilder.java 60.00% <60.00%> (ø)
.../opentelemetry/sdk/logs/SdkReadWriteLogRecord.java 80.00% <80.00%> (ø)
...entelemetry/sdk/logs/export/BatchLogProcessor.java 88.72% <85.71%> (ø)
...io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java 97.50% <92.85%> (ø)
...a/io/opentelemetry/sdk/logs/MultiLogProcessor.java 90.90% <100.00%> (ø)
...va/io/opentelemetry/sdk/logs/NoopLogProcessor.java 100.00% <100.00%> (ø)
.../java/io/opentelemetry/sdk/logs/SdkLogEmitter.java 100.00% <100.00%> (ø)
...ntelemetry/sdk/logs/export/SimpleLogProcessor.java 88.57% <100.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jul 27 '22 23:07 codecov[bot]

Hey @jkwatson - wondering what your thoughts are about trying to get this in before the 1.17.0 release. It technically depends on this spec PR, but I think its extremely likely that the spec goes in this direction and that its current state of not allowing mutable logs is accidental. Additionally, both the spec and the sdk are experimental so I think its ok for us to experiment (within reason!).

jack-berg avatar Aug 04 '22 16:08 jack-berg