Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OneCollector] Add benchmark project #1088

Merged
merged 4 commits into from
Mar 16, 2023

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Mar 16, 2023

Changes

  • Adds a benchmark project for OneCollectorExporter and benchmarks for LogRecord export.

Benchmarks

Method NumberOfBatches NumberOfLogRecordsPerBatch EnableCompression Mean Error StdDev Allocated
Export 1 1000 False 474.8 us 6.18 us 5.16 us 857 B
Export 1 1000 True 1,683.1 us 7.87 us 6.98 us 1066 B
Export 1 10000 False 748.2 us 14.62 us 16.83 us 857 B
Export 1 10000 True 2,558.6 us 33.41 us 31.25 us 1068 B
Export 10 1000 False 4,796.1 us 74.93 us 70.09 us 8568 B
Export 10 1000 True 16,911.2 us 218.06 us 193.31 us 10674 B
Export 10 10000 False 7,151.9 us 72.75 us 68.05 us 8574 B
Export 10 10000 True 25,736.1 us 420.56 us 393.39 us 10674 B
Export 100 1000 False 48,253.9 us 672.36 us 628.93 us 85698 B
Export 100 1000 True 165,695.3 us 2,522.35 us 2,235.99 us 106760 B
Export 100 10000 False 70,449.0 us 1,261.47 us 1,179.98 us 85754 B
Export 100 10000 True 251,097.2 us 2,099.32 us 1,860.99 us 106760 B

Analysis

The chart above looks intimidating so I'll try to break it down 😄

TL;DR: The perf is where I would expect it to be and it is really good.

Here is the key bit:

NumberOfBatches NumberOfLogRecordsPerBatch EnableCompression Mean Allocated
1 1000 False 474.8 us 857 B
1 10000 False 748.2 us 857 B

The number of records in a batch does NOT increase the amount of memory allocated. That means converting a LogRecord to JSON we are not allocating any strings, boxing any value types, or anything. That is great! The 857 bytes are mostly HttpClient artifacts. A given export needs to allocate a bunch of stuff for the HttpClient API. HttpRequestMessage, HttpResponseMessage, stuff for headers. We also have some allocations for SuppressInstrumentationScope (should improve with open-telemetry/opentelemetry-dotnet#4304). These allocations don't concern me much. Exports trigger when either a batch is full or a time limit is reached. It is an occasional thing and a few allocations shouldn't add much pressure to the GC for the process. What we don't want to see is allocations growing based on the number of items.

NumberOfBatches NumberOfLogRecordsPerBatch EnableCompression Mean Allocated
1 1000 True 1,683.1 us 1066 B
1 10000 True 2,558.6 us 1068 B

When compression is used (enabled by default) it costs more CPU time and more memory is allocated but we have the same flat amount regardless of the number of records.

@CodeBlanch CodeBlanch added the comp:exporter.onecollector Things related to OpenTelemetry.Exporter.OneCollector label Mar 16, 2023
@CodeBlanch CodeBlanch requested a review from a team March 16, 2023 18:04
@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #1088 (d5820e6) into main (5f587d4) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1088   +/-   ##
=======================================
  Coverage   69.29%   69.29%           
=======================================
  Files         214      214           
  Lines        8358     8358           
=======================================
  Hits         5792     5792           
  Misses       2566     2566           

Copy link
Contributor

@Kielek Kielek left a comment

Choose a reason for hiding this comment

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

Project scaffolding LGTM

@CodeBlanch CodeBlanch merged commit 8f8d153 into open-telemetry:main Mar 16, 2023
@CodeBlanch CodeBlanch deleted the onecollector-benchmarks branch March 16, 2023 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:exporter.onecollector Things related to OpenTelemetry.Exporter.OneCollector
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants