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] [internal/splunk] Don't use pointer for Event.Time field value #22030

Merged
merged 1 commit into from
May 17, 2023

Conversation

dmitryax
Copy link
Member

The pointer was added in #1157 to avoid sending zero timestamps by Splunk HEC exporter. But using a pointer is not required for that, json:omitempty tag does the same for float64 zero values.

After this change, we cannot differentiate nil/0 values of Time field in the internal splunk.Event struct, but it doesn't matter because both versions produce the same JSON string without time field and translates to/from the same OTLP 0 values.

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    	   36219	     32740 ns/op	   25431 B/op	     416 allocs/op
Benchmark_pushLogData_10_10_8K
Benchmark_pushLogData_10_10_8K-10      	   13060	     94597 ns/op	   71809 B/op	    1167 allocs/op
Benchmark_pushLogData_10_10_2M
Benchmark_pushLogData_10_10_2M-10      	   12934	     91213 ns/op	   74024 B/op	    1199 allocs/op
Benchmark_pushLogData_10_200_2M
Benchmark_pushLogData_10_200_2M-10     	     634	   1907038 ns/op	 1476323 B/op	   24000 allocs/op
Benchmark_pushLogData_100_200_2M
Benchmark_pushLogData_100_200_2M-10    	      76	  13599245 ns/op	11354600 B/op	  183258 allocs/op
Benchmark_pushLogData_100_200_5M
Benchmark_pushLogData_100_200_5M-10    	      57	  18378855 ns/op	14868081 B/op	  239911 allocs/op
BenchmarkConsumeLogsRejected
BenchmarkConsumeLogsRejected-10        	    1245	    972941 ns/op	  739034 B/op	   12002 allocs/op
PASS

After:

pkg: github.com/open-telemetry/opentelemetry-collector-contrib/exporter/splunkhecexporter
Benchmark_pushLogData_10_10_1024
Benchmark_pushLogData_10_10_1024-10    	   38847	     31386 ns/op	   25167 B/op	     383 allocs/op
Benchmark_pushLogData_10_10_8K
Benchmark_pushLogData_10_10_8K-10      	   13942	     87630 ns/op	   71106 B/op	    1076 allocs/op
Benchmark_pushLogData_10_10_2M
Benchmark_pushLogData_10_10_2M-10      	   13096	     90047 ns/op	   73338 B/op	    1109 allocs/op
Benchmark_pushLogData_10_200_2M
Benchmark_pushLogData_10_200_2M-10     	     631	   1917114 ns/op	 1460391 B/op	   22010 allocs/op
Benchmark_pushLogData_100_200_2M
Benchmark_pushLogData_100_200_2M-10    	      79	  12951741 ns/op	11175612 B/op	  168047 allocs/op
Benchmark_pushLogData_100_200_5M
Benchmark_pushLogData_100_200_5M-10    	      61	  16757523 ns/op	14699264 B/op	  220011 allocs/op
BenchmarkConsumeLogsRejected
BenchmarkConsumeLogsRejected-10        	    1274	    935309 ns/op	  731880 B/op	   11012 allocs/op
PASS

The pointer was added to avoid sending zero timestamp by Splunk HEC exporter. But using pointer is not required for that `json:omitempty` tag does the same for float64 zero values.

After this change, we cannot differentiate `nil`/`0` values of `Time` field in the internal `splunk.Event` struct, but it doesn't matter because both versions produce the same JSON string without `time` field and translates to/from the same OTLP `0` values.
@dmitryax dmitryax force-pushed the splunkhec-dont-use-pointer branch from 0d32634 to 9b0f71b Compare May 17, 2023 03:31
@dmitryax dmitryax merged commit 721ca5e into open-telemetry:main May 17, 2023
@github-actions github-actions bot added this to the next release milestone May 17, 2023
@dmitryax dmitryax deleted the splunkhec-dont-use-pointer branch May 17, 2023 15:52
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.

3 participants