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

[chore][exporter/elasticsearch] Add benchmark for traces consumer #33087

Merged
merged 11 commits into from
May 22, 2024

Conversation

lahsivjar
Copy link
Member

@lahsivjar lahsivjar commented May 16, 2024

Description: Similar to #33035 the PR adds benchmarks for ConsumeTraces.

The PR also updates the mock es receiver to handle traces properly in order to have better readability of the tests/benchmarks and avoid confusion with the special handling -- this is done by using separate index configuration for traces and logs to distinguish the items in the bulk indexer payload. The PR also adds an extra metric to the benchmarks (Consume{Traces, Logs}) for reporting events consumed per second.

Link to tracking Issue: 32504

Testing: cd exporter/elasticsearchexporter/integrationtest && go test -bench=BenchmarkExporter -run=^$ ./...

Documentation: N/A

- Updates the mock Elasticsearch data receiver to handle traces.
- Adds an extra metric to report events per second.
return err
return fmt.Errorf("failed to create logs receiver: %w", err)
}
es.receiver, err = factory.CreateTracesReceiver(context.Background(), set, cfg, tc)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is overwriting es.receiver which is a receiver.Logs. Is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, both will be same since we use SharedComponent

Copy link
Member Author

@lahsivjar lahsivjar May 16, 2024

Choose a reason for hiding this comment

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

It might have been a source of confusion for others so I added a comment and assertion (overkill maybe? - feedbacks welcomed) to check if they point to the same object.

@lahsivjar
Copy link
Member Author

[For reviewers] In order to reduce the impact of decoding bulk requests in the mock receiver I have removed the correctness checks from the benchmarks. The benchmarks now don't decode bulk requests and rely on the correctness to be tested in other tests. This gives us a better benchmark result. Note that if memprofiles are generated from tests they will show Generate{Logs, Traces} but the benchmark results produced by running the tests won't include these values as they are removed by using timers.

@andrzej-stencel
Copy link
Member

Looking good! Here are the results from my machine:

$ cd exporter/elasticsearchexporter/integrationtest
$ go test -bench=BenchmarkExporter -run=^$ ./... 
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/exporter/elasticsearchexporter/integrationtest
cpu: 13th Gen Intel(R) Core(TM) i7-13700H
BenchmarkExporter/logs/small_batch-20              10000            219200 ns/op             45620 events/s        65539 B/op        355 allocs/op
BenchmarkExporter/logs/medium_batch-20               554           2009038 ns/op             49775 events/s       643463 B/op       3450 allocs/op
BenchmarkExporter/logs/large_batch-20                 56          21210056 ns/op             47147 events/s      6430709 B/op      34416 allocs/op
BenchmarkExporter/logs/xlarge_batch-20                 6         191364099 ns/op             52256 events/s     63892213 B/op     343340 allocs/op
BenchmarkExporter/traces/small_batch-20             5449            247561 ns/op             40394 events/s        67422 B/op        396 allocs/op
BenchmarkExporter/traces/medium_batch-20             506           2096840 ns/op             47691 events/s       661824 B/op       3853 allocs/op
BenchmarkExporter/traces/large_batch-20               54          19818097 ns/op             50459 events/s      6585914 B/op      38418 allocs/op
BenchmarkExporter/traces/xlarge_batch-20               5         235706998 ns/op             42426 events/s     65868492 B/op     384351 allocs/op
PASS
ok      github.com/open-telemetry/opentelemetry-collector-contrib/exporter/elasticsearchexporter/integrationtest        21.859s

@andrzej-stencel andrzej-stencel merged commit c2b0082 into open-telemetry:main May 22, 2024
162 checks passed
@github-actions github-actions bot added this to the next release milestone May 22, 2024
@lahsivjar lahsivjar deleted the benchmarks_32504_traces branch May 22, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants