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/splunkhec] Simplify the batching process #21969

Merged
merged 1 commit into from
May 15, 2023

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented May 15, 2023

Rework how the batches are created and sent to the backend to make it easier to read the code.

Also, this change fixes an issue introduced by removing the additional index field of the bufferState struct in #21765: if a hec batch combining several OTel resources gets rejected with a retriable error, only the latest read resource/scope records were sent for retry. The fix is confirmed by an additional TestPushLogsRetryableFailureMultipleResources test.

This also significantly improves performance by removing a bunch of memory allocations in cases where the incoming data ends up being sent in several HEC batches.

Before:

goos: darwin
goarch: arm64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/exporter/splunkhecexporter
Benchmark_pushLogData_10_10_1024
Benchmark_pushLogData_10_10_1024-10    	   12033	     99060 ns/op	   90027 B/op	    1299 allocs/op
Benchmark_pushLogData_10_10_8K
Benchmark_pushLogData_10_10_8K-10      	   12072	     96328 ns/op	   90065 B/op	    1299 allocs/op
Benchmark_pushLogData_10_10_2M
Benchmark_pushLogData_10_10_2M-10      	   12306	     97606 ns/op	   90114 B/op	    1299 allocs/op
Benchmark_pushLogData_10_200_2M
Benchmark_pushLogData_10_200_2M-10     	     572	   2104811 ns/op	 1828672 B/op	   26003 allocs/op
Benchmark_pushLogData_100_200_2M
Benchmark_pushLogData_100_200_2M-10    	      56	  19257562 ns/op	18350930 B/op	  259916 allocs/op
Benchmark_pushLogData_100_200_5M
Benchmark_pushLogData_100_200_5M-10    	      55	  19130223 ns/op	18467195 B/op	  259916 allocs/op
PASS

After:

Benchmark_pushLogData_10_10_1024
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/exporter/splunkhecexporter
Benchmark_pushLogData_10_10_1024
Benchmark_pushLogData_10_10_1024-10    	   34998	     34225 ns/op	   30894 B/op	     450 allocs/op
Benchmark_pushLogData_10_10_8K
Benchmark_pushLogData_10_10_8K-10      	   12380	     94922 ns/op	   87420 B/op	    1264 allocs/op
Benchmark_pushLogData_10_10_2M
Benchmark_pushLogData_10_10_2M-10      	   12411	     96700 ns/op	   90155 B/op	    1299 allocs/op
Benchmark_pushLogData_10_200_2M
Benchmark_pushLogData_10_200_2M-10     	     560	   2088985 ns/op	 1827450 B/op	   26003 allocs/op
Benchmark_pushLogData_100_200_2M
Benchmark_pushLogData_100_200_2M-10    	      70	  15065196 ns/op	14020531 B/op	  198538 allocs/op
Benchmark_pushLogData_100_200_5M
Benchmark_pushLogData_100_200_5M-10    	      55	  18660002 ns/op	18467047 B/op	  259916 allocs/op
PASS

Rework how the batches are created and sent to the backend to make it easier to read the code.

Also this change fixes an issue introduced with removing of the additional `index` field of the `bufferState` struct: if a hec batch combining several OTel resources gets rejected with a retryable error, only the latests read resource/scope records were sent for retry. The fix is confirmed by additional TestPushLogsRetryableFailureMultipleResources test.

This also improves performance by removing a bunch of memory allocations.
@dmitryax dmitryax requested a review from a team May 15, 2023 19:25
@dmitryax dmitryax requested a review from atoulme as a code owner May 15, 2023 19:25
@dmitryax dmitryax merged commit 42fb6b6 into open-telemetry:main May 15, 2023
@github-actions github-actions bot added this to the next release milestone May 15, 2023
@dmitryax dmitryax deleted the hec-exp-simplify-the-logic branch May 15, 2023 21:43
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.

2 participants