1
0
mirror of https://github.com/open-telemetry/opentelemetry-go.git synced 2025-01-16 02:47:20 +02:00
Commit Graph

15 Commits

Author SHA1 Message Date
Robert Pająk
8e8ad092cc
sdk/log: Processor.OnEmit accetps a Record pointer (#5636)
## What

Change `Processor.OnEmit` methods to accept a record pointer.

## Why

Fixes https://github.com/open-telemetry/opentelemetry-go/issues/5219

This would be specification compliant according to discussions around
https://github.com/open-telemetry/opentelemetry-specification/pull/4067

This is inline of how processors Go span processors works and how log
processors work in other languages.

If the performance (an additional heap allocation during log processing)
would occur to be a significant problem for some users, we have at few
possible solutions:

1. Utilize PGO which may also lead to decreasing heap allocations
(sources:
https://landontclipp.github.io/blog/2023/08/25/profile-guided-optimizations-in-go/#devirtualization,
https://andrewwphillips.github.io/blog/pgo.html). Currently it does not
but I expect it may change in future.
2. Improve the Go compilers escape analysis (related to previous point)
3. introduce new "chaining processor" which can be also desirable in
other languages

## Benchstat

`old` is from `main`.
`new` is from current branch.

`new-pgo` is from current branch with PGO optimization. I first run
benchmarks to generate a CPU profile using `go test -run=^$ -bench=.
-count=10 -cpuprofile default.pgo` and then I rerun the tests with PGO.
Currently, the number of heap allocations is the same.

```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
                                  │    old.txt    │                new.txt                │              new-pgo.txt              │
                                  │    sec/op     │    sec/op     vs base                 │    sec/op     vs base                 │
BatchProcessorOnEmit-16              402.7n ± 18%   382.0n ±  7%         ~ (p=0.247 n=10)   376.7n ± 14%         ~ (p=0.210 n=10)
Processor/Simple-16                  350.9n ±  9%   782.5n ±  6%  +123.00% (p=0.000 n=10)   755.1n ±  5%  +115.19% (p=0.000 n=10)
Processor/Batch-16                   1.333µ ± 15%   1.497µ ± 11%   +12.27% (p=0.000 n=10)   1.528µ ±  8%   +14.63% (p=0.000 n=10)
Processor/SetTimestampSimple-16      329.5n ± 15%   711.6n ±  4%  +115.93% (p=0.000 n=10)   721.9n ±  5%  +119.04% (p=0.000 n=10)
Processor/SetTimestampBatch-16       1.163µ ±  2%   1.524µ ±  3%   +31.03% (p=0.000 n=10)   1.461µ ±  5%   +25.57% (p=0.000 n=10)
Processor/AddAttributesSimple-16     408.7n ±  3%   810.1n ±  4%   +98.21% (p=0.000 n=10)   830.1n ±  4%  +103.11% (p=0.000 n=10)
Processor/AddAttributesBatch-16      1.270µ ±  2%   1.623µ ±  4%   +27.71% (p=0.000 n=10)   1.597µ ±  7%   +25.66% (p=0.000 n=10)
Processor/SetAttributesSimple-16     436.2n ± 10%   796.1n ±  3%   +82.50% (p=0.000 n=10)   817.6n ±  4%   +87.43% (p=0.000 n=10)
Processor/SetAttributesBatch-16      1.202µ ±  2%   1.552µ ±  2%   +29.06% (p=0.000 n=10)   1.659µ ± 11%   +37.96% (p=0.000 n=10)
LoggerNewRecord/5_attributes-16      366.6n ±  3%   363.7n ±  7%         ~ (p=0.952 n=10)   426.2n ±  7%   +16.27% (p=0.000 n=10)
LoggerNewRecord/10_attributes-16     1.711µ ±  2%   1.909µ ± 18%   +11.57% (p=0.000 n=10)   2.077µ ± 10%   +21.39% (p=0.000 n=10)
LoggerProviderLogger-16              650.1n ±  4%   690.1n ±  8%    +6.15% (p=0.019 n=10)   737.6n ± 13%   +13.47% (p=0.004 n=10)
WalkAttributes/1_attributes-16       5.264n ± 12%   5.510n ±  8%         ~ (p=0.812 n=10)   5.865n ±  5%   +11.41% (p=0.011 n=10)
WalkAttributes/10_attributes-16      5.440n ±  8%   5.881n ±  7%    +8.12% (p=0.004 n=10)   6.104n ±  7%   +12.21% (p=0.005 n=10)
WalkAttributes/100_attributes-16     5.403n ±  9%   5.894n ±  9%    +9.10% (p=0.029 n=10)   5.783n ±  6%         ~ (p=0.052 n=10)
WalkAttributes/1000_attributes-16    5.196n ±  4%   5.860n ±  8%   +12.79% (p=0.000 n=10)   5.981n ± 13%   +15.13% (p=0.002 n=10)
SetAddAttributes/SetAttributes-16    181.2n ± 14%   208.1n ± 12%   +14.85% (p=0.005 n=10)   209.9n ± 11%   +15.87% (p=0.007 n=10)
SetAddAttributes/AddAttributes-16    156.7n ± 14%   161.1n ± 16%         ~ (p=0.190 n=10)   165.5n ± 15%         ~ (p=0.315 n=10)
SimpleProcessorOnEmit-16            11.775n ± 10%   9.027n ± 17%   -23.33% (p=0.000 n=10)   9.389n ± 18%   -20.26% (p=0.002 n=10)
geomean                              169.1n         209.6n         +23.98%                  215.5n         +27.48%

                        │     old.txt     │               new.txt                │              new-pgo.txt              │
                        │       B/s       │     B/s       vs base                │      B/s       vs base                │
BatchProcessorOnEmit-16   1004.39Mi ± 15%   79.88Mi ± 7%  -92.05% (p=0.000 n=10)   81.06Mi ± 12%  -91.93% (p=0.000 n=10)

                                  │   old.txt    │                new.txt                 │               new-pgo.txt               │
                                  │     B/op     │    B/op      vs base                   │     B/op      vs base                   │
BatchProcessorOnEmit-16             0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
Processor/Simple-16                   0.0 ± 0%      417.0 ± 0%         ? (p=0.000 n=10)      417.0 ±  0%         ? (p=0.000 n=10)
Processor/Batch-16                  621.5 ± 2%     1057.5 ± 1%   +70.15% (p=0.000 n=10)     1064.5 ±  1%   +71.28% (p=0.000 n=10)
Processor/SetTimestampSimple-16       0.0 ± 0%      417.0 ± 0%         ? (p=0.000 n=10)      418.0 ±  0%         ? (p=0.000 n=10)
Processor/SetTimestampBatch-16      626.5 ± 3%     1049.5 ± 1%   +67.52% (p=0.000 n=10)     1057.5 ±  2%   +68.79% (p=0.000 n=10)
Processor/AddAttributesSimple-16      0.0 ± 0%      417.0 ± 0%         ? (p=0.000 n=10)      417.0 ±  0%         ? (p=0.000 n=10)
Processor/AddAttributesBatch-16     616.5 ± 3%     1053.0 ± 2%   +70.80% (p=0.000 n=10)     1048.5 ±  2%   +70.07% (p=0.000 n=10)
Processor/SetAttributesSimple-16    48.00 ± 0%     466.00 ± 0%  +870.83% (p=0.000 n=10)     466.00 ±  0%  +870.83% (p=0.000 n=10)
Processor/SetAttributesBatch-16     648.0 ± 3%     1089.5 ± 1%   +68.13% (p=0.000 n=10)     1087.5 ±  4%   +67.82% (p=0.000 n=10)
LoggerNewRecord/5_attributes-16     0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
LoggerNewRecord/10_attributes-16    610.0 ± 0%      610.0 ± 0%         ~ (p=1.000 n=10) ¹    610.0 ±  0%         ~ (p=1.000 n=10) ¹
LoggerProviderLogger-16             354.5 ± 6%      368.0 ± 7%         ~ (p=0.288 n=10)      391.0 ± 29%         ~ (p=0.239 n=10)
WalkAttributes/1_attributes-16      0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
WalkAttributes/10_attributes-16     0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
WalkAttributes/100_attributes-16    0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
WalkAttributes/1000_attributes-16   0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
SetAddAttributes/SetAttributes-16   48.00 ± 0%      48.00 ± 0%         ~ (p=1.000 n=10) ¹    48.00 ±  0%         ~ (p=1.000 n=10) ¹
SetAddAttributes/AddAttributes-16   0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
SimpleProcessorOnEmit-16            0.000 ± 0%      0.000 ± 0%         ~ (p=1.000 n=10) ¹    0.000 ±  0%         ~ (p=1.000 n=10) ¹
geomean                                        ²                ?                       ²                 ?                       ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                                  │   old.txt    │                new.txt                │              new-pgo.txt              │
                                  │  allocs/op   │ allocs/op   vs base                   │ allocs/op   vs base                   │
BatchProcessorOnEmit-16             0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
Processor/Simple-16                 0.000 ± 0%     1.000 ± 0%         ? (p=0.000 n=10)     1.000 ± 0%         ? (p=0.000 n=10)
Processor/Batch-16                  0.000 ± 0%     1.000 ± 0%         ? (p=0.000 n=10)     1.000 ± 0%         ? (p=0.000 n=10)
Processor/SetTimestampSimple-16     0.000 ± 0%     1.000 ± 0%         ? (p=0.000 n=10)     1.000 ± 0%         ? (p=0.000 n=10)
Processor/SetTimestampBatch-16      0.000 ± 0%     1.000 ± 0%         ? (p=0.000 n=10)     1.000 ± 0%         ? (p=0.000 n=10)
Processor/AddAttributesSimple-16    0.000 ± 0%     1.000 ± 0%         ? (p=0.000 n=10)     1.000 ± 0%         ? (p=0.000 n=10)
Processor/AddAttributesBatch-16     0.000 ± 0%     1.000 ± 0%         ? (p=0.000 n=10)     1.000 ± 0%         ? (p=0.000 n=10)
Processor/SetAttributesSimple-16    1.000 ± 0%     2.000 ± 0%  +100.00% (p=0.000 n=10)     2.000 ± 0%  +100.00% (p=0.000 n=10)
Processor/SetAttributesBatch-16     1.000 ± 0%     2.000 ± 0%  +100.00% (p=0.000 n=10)     2.000 ± 0%  +100.00% (p=0.000 n=10)
LoggerNewRecord/5_attributes-16     0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
LoggerNewRecord/10_attributes-16    4.000 ± 0%     4.000 ± 0%         ~ (p=1.000 n=10) ¹   4.000 ± 0%         ~ (p=1.000 n=10) ¹
LoggerProviderLogger-16             1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹   1.000 ± 0%         ~ (p=1.000 n=10) ¹
WalkAttributes/1_attributes-16      0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
WalkAttributes/10_attributes-16     0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
WalkAttributes/100_attributes-16    0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
WalkAttributes/1000_attributes-16   0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
SetAddAttributes/SetAttributes-16   1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹   1.000 ± 0%         ~ (p=1.000 n=10) ¹
SetAddAttributes/AddAttributes-16   0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
SimpleProcessorOnEmit-16            0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹   0.000 ± 0%         ~ (p=1.000 n=10) ¹
geomean                                        ²               ?                       ²               ?                       ²
¹ all samples are equal
² summaries must be >0 to compute geomean
```
2024-08-01 10:13:43 +02:00
Anton Manakin
8d4d3c875c
sdk/log: Fix TestBatchProcessor/ForceFlush/ErrorPartialFlush flaky test (#5416)
Fix #5342

Run of `go test -v -timeout 10s -short -count=10000 ./...
-run='TestBatchProcessor/ForceFlush/ErrorPartialFlush'`
**Before**:
Failed with `panic: test timed out after 10s`
**After**:
Passed

Problem was with `bufferExporter.input` chan. 
In test expected:
> 1 export being performed, 1 export in buffer chan, >1 batch

But buffer chan wasn't checked. Now we ensure, that record is in buffer
chan

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
2024-05-30 07:40:19 -07:00
Anton Manakin
982e96dd9b
sdk/log: Fix TestBatchProcessor/DroppedLogs flaky test (#5421)
Fix #5384

Run of `go test  -count=1000000 -run="TestBatchProcessor/DroppedLogs"`
**Before**:
Failed with either `Condition never satisfied` or panic
**After**:
Passed

First, bytes.Buffer is not thread-safe, so writing log and reading
(`bytes.String()`) caused panic. Added `concurrentBuffer`
Second, fixed flaky test with 4 records:
1. Record goes to `testExporter.Export` function and blocks in this
function because of `ExportTrigger`
(19ee6d4775/sdk/log/exporter_test.go (L87))
2. Record goes to `bufferExporter.input`
(7c5e64cccc/sdk/log/exporter.go (L129))
3. Record goes to `BatchProcessor.q` queue and it could not be enqueued
to export, because `bufferExporter.input` is full
4. Record goes to `BatchProcessor.q` and because of overfill, drops
third record

---------

Co-authored-by: Sam Xie <sam@samxie.me>
2024-05-29 11:19:57 -07:00
Tyler Yahn
281aeb5c64
Recheck log message in TestBatchProcessor (#5386)
Fix #5384

On single-threaded, or in general slow, systems where the log message
may not be emitted, use assert.Everything to allow multiple lookups for
the expected log message.

Co-authored-by: Sam Xie <sam@samxie.me>
2024-05-21 13:28:05 -07:00
Tyler Yahn
ae06a80417
Log records dropped by the BatchProcessor (#5276)
* Add dropped count to queue

* Log dropped records

* Add changelog entry

---------

Co-authored-by: Robert Pająk <pellared@hotmail.com>
2024-05-08 07:42:09 -07:00
Tyler Yahn
29e1c7e3e4
Add custom ring implementation to the BatchProcessor (#5237) 2024-04-24 12:07:15 +02:00
Tyler Yahn
b34cfc47c4
Default implementation for empty BatchProcessor (#5239)
Ensure an empty BatchProcessor does not panic when any method is called.
Default an empty BatchProcessor as being shut down.
2024-04-22 16:21:49 +02:00
Tyler Yahn
94eb27fb40
Rename batching configuration (#5236)
Match the BatchProcessor and name these decls "batch*."
2024-04-19 18:56:31 +02:00
Robert Pająk
ed666f7713
sdk/log: Rename BatchingProcessor to BatchProcessor (#5229) 2024-04-19 08:34:30 +02:00
Tyler Yahn
4af9c20a80
Implement the BatchingProcessor (#5093)
* [WIP] Implement the BatchingProcessor

* Add TestExportSync

* Add TestChunker

* Test export error default to ErrorHandler

* Fix lint

* Fix chunk smaller than size error

* Add batch tests

* Fix lint

* Update OnEmit test

Check the len of records in eventually assertion given that is what we
are going to measure.

* Revert unneeded change to BatchingProcessor doc

* Add batch type

* Refactor testing of batching config

The BatchingProcessor is not expected to ultimately contain
configuration fields for queue size or export parameters (see #5093).
This will break TestNewBatchingProcessorConfiguration which tests the
configuration by evaluating the BatchingProcessor directly.

Instead, test the batchingConfig and rename the test to
TestNewBatchingConfig to match what is being tested.

* Implement the BatchingProcessor without polling

* Add TestBatchingProcessor

* Add ConcurrentSafe test

* Expand Shutdown tests

* Test context canceled for ForceFlush

* Refactor batch to queue

* Use exportSync

* Update docs and naming

* Split buffered export to its own type

* Update comments and naming

* Fix lint

* Remove redundant triggered type

* Add interval polling

* Refactor test structure

* Add custom ring implimementation

* Add BenchmarkBatchingProcessor

* Fix merge

* Remove custom ring impl

* Remove BenchmarkBatchingProcessor

* Update dev docs

* Test nil exporter

* Update OnEmit test

Ensure the poll goroutine will completely flush the queue of batches.

* Test RetriggerFlushNonBlocking

* Update ascii diagram

* Fix flaky OnEmit

* Revert unnecessary change to test pkg name

* Use batching term in docs

* Document EnqueueExport

* Return from EnqueueExport if blocked

Do not wait for the enqueue to succeed.

* Do not drop failed flush log records

* Use cancelable ctx in concurrency test

* Fix comments

* Apply feedback

Do not spawn a goroutine for the flush operation.

* Return true from EnqueueExport when stopped

* Update sdk/log/batch.go

Co-authored-by: Robert Pająk <pellared@hotmail.com>

* Remove TODO

* Comment re-trigger in poll

---------

Co-authored-by: Robert Pająk <pellared@hotmail.com>
2024-04-18 07:48:19 -07:00
Robert Pająk
054a63fef2
sdk/log: Add options to NewSimpleProcessor (#5185) 2024-04-10 19:47:40 +02:00
Tyler Yahn
98d961b141
Clamp batch size <= queue size (#5157) 2024-04-05 16:02:32 -07:00
Tyler Yahn
6c6e1e7416
Add queue for BatchingProcessor (#5131) 2024-04-03 13:53:16 +02:00
Tyler Yahn
6033938c87
Refactor testing of batching config (#5106)
The BatchingProcessor is not expected to ultimately contain
configuration fields for queue size or export parameters (see #5093).
This will break TestNewBatchingProcessorConfiguration which tests the
configuration by evaluating the BatchingProcessor directly.

Instead, test the batchingConfig and rename the test to
TestNewBatchingConfig to match what is being tested.
2024-03-26 13:22:53 +01:00
Tyler Yahn
32e3a3d994
Implement the BatchingProcessor configuration (#5088)
* Implement the batching config

* Unify on setting type

* Add setting_test.go

* Test NewBatchingProcessor

* Comment setting

* Fix lint

* Check invalid after envar

---------

Co-authored-by: Sam Xie <sam@samxie.me>
2024-03-25 07:50:19 -07:00