I think it is better to calculate fltrProcessors upfront instead of
doing it in a lazy manner. Reasons:
- No locks/synchronization in the hot-path. Even though the performance
overhead is not big (in practice it will be usually unnoticeable, in my
benchmark execution it is less than a nanosecond) I think it is good to
minimize locking on the hot path.
- Simpler code (subjective, but at least less code by 9 lines)
```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
LoggerEnabled-20 6.010n ± 1% 5.364n ± 1% -10.73% (p=0.000 n=10)
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
LoggerEnabled-20 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
LoggerEnabled-20 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
```
Closes#5425
Our current log `Processor` interface contains more functionality than
the [OTel
spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#logrecordprocessor-operations).
The additional functionality allows processors to report back to the API
if a Record should be constructed and emitted or not, which is quite
helpful[^1][^2][^3][^4][^5].
This removes the `Enabled` method from the `Processor` type. It adds
this functionality a new optional and experimental `FilterProcessor`
interface type. The logger and provider are updated to check for this
optional interface to be implemented with the configured processors and
uses them to back the `Logger.Enabled` method, preserving existing
functionality.
By making this change:
- The `Processor` interface is now compliant with the OTel spec and does
not contain any additional unspecified behavior.
- All `Processor` implementations are no longer required to implement an
`Enabled` method. The default, when they do not implement this method,
is to assume they are enabled.
### Benchmark
```terminal
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
│ old.txt │ new7.txt │
│ sec/op │ sec/op vs base │
LoggerEnabled-8 133.30n ± 3% 32.36n ± 3% -75.72% (p=0.000 n=10)
│ old.txt │ new7.txt │
│ B/op │ B/op vs base │
LoggerEnabled-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
│ old.txt │ new7.txt │
│ allocs/op │ allocs/op vs base │
LoggerEnabled-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
```
This is a significant performance improvement due to the `Record` no
longer being converted from the API version to the SDK version.
[^1]: https://pkg.go.dev/go.opentelemetry.io/contrib/processors/minsev
[^2]:
https://pkg.go.dev/go.opentelemetry.io/otel/sdk/log#BatchProcessor.Enabled
[^3]:
https://pkg.go.dev/go.opentelemetry.io/otel/sdk/log#SimpleProcessor.Enabled
[^4]:
af75717ac4/bridges/otelslog/handler.go (L206-L211)
[^5]:
d0309ddd8c/bridges/otelzap/core.go (L142-L146)
---------
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Sam Xie <sam@samxie.me>
* Replace Record lim methods with DroppedAttributes
* Add changelog entry
* Add TestRecordDroppedAttributes
* Add TestRecordCompactAttr
* Add an indexPool
* Fix gramatical error
* Apply feedback
Reduce indentation level.
* Apply feedback
Comment compactAttr and deduplicate.
* Deduplicate all attributes when added
* Comment why head is not used
* Clarify comments
* Move TestAllocationLimits to new file
Do not run this test when the race detector is on.
* Comment follow-up task