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

PoC: Update Jaeger exporter to rely on the SDK for batching and NoOp performance #1803

Closed
wants to merge 2 commits into from

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 12, 2021

Changes

  • Remove Jaeger exporter dependency on cloud.google.com/go by removing the use of the provided Bundler.
  • Stop batching spans that have been sent to the Jaeger exporter for export. Instead transform and export the span during the call to ExportSpans and rely on the BatchSpanProcessor for batching.
  • Remove the flush function from the Jaeger exporter. The exporter no longer maintains spans between calls to ExportSpans so there is no use for a flush function.
  • Update the return value for all the convenience creation functions (InstallNewPipeline and NewExportPiepeline) to not return the now unneeded flush function.
  • Remove the WithBufferMaxCount option. The Jaeger exporter no longer bundles.
  • Remove the WithBatchMaxCount option. The Jaeger exporter no longer bundles.
  • Remove the WithDisabled option. Unregistered the wrapping span processor from the TracerProvider or do not set a global TracerProvider for instrumentation to use.
  • Remove the WithSDKOptions option and instead pass these options directly to the convenience creation functions.
  • Remove the Option type. There is not configuration needed to be set for the Jaeger exporter other than endpoint configuration (which is handled by the respective uploader configuration).
  • Remove the locking for the Jaeger exporter, instead unify on using context and channels to signal when an export needs to be canceled.
  • Remove the Process type. This is leftover from Remove process config for Jaeger exporter #1776.
  • Add benchmark test to track resources used in ExportSpans

Benchmarks

Before

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/exporters/trace/jaeger
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkExportSpans1-8       	  264658	      4094 ns/op	     804 B/op	      18 allocs/op
BenchmarkExportSpans10-8      	   97534	     12446 ns/op	    4743 B/op	      98 allocs/op
BenchmarkExportSpans100-8     	   18016	     72945 ns/op	   42876 B/op	     824 allocs/op
BenchmarkExportSpans1000-8    	    1987	    610675 ns/op	  417141 B/op	    8030 allocs/op
BenchmarkExportSpans10000-8   	     198	   6511413 ns/op	 4699629 B/op	   80050 allocs/op

After

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/exporters/trace/jaeger
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkExportSpans1-8       	  843811	      1309 ns/op	     594 B/op	      13 allocs/op
BenchmarkExportSpans10-8      	  186817	      6862 ns/op	    4071 B/op	      80 allocs/op
BenchmarkExportSpans100-8     	   21878	     50945 ns/op	   38266 B/op	     713 allocs/op
BenchmarkExportSpans1000-8    	    2388	    457674 ns/op	  376684 B/op	    7016 allocs/op
BenchmarkExportSpans10000-8   	     254	   5324264 ns/op	 3986591 B/op	   70026 allocs/op

Related Issues

Resolve #1799

@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #1803 (b011f31) into main (d0cea04) will decrease coverage by 0.2%.
The diff coverage is 53.8%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #1803     +/-   ##
=======================================
- Coverage   78.6%   78.4%   -0.3%     
=======================================
  Files        134     134             
  Lines       7159    7137     -22     
=======================================
- Hits        5633    5596     -37     
- Misses      1276    1297     +21     
+ Partials     250     244      -6     
Impacted Files Coverage Δ
exporters/trace/jaeger/agent.go 60.0% <0.0%> (ø)
exporters/trace/jaeger/uploader.go 31.5% <0.0%> (-17.7%) ⬇️
exporters/trace/jaeger/jaeger.go 92.3% <84.0%> (-1.2%) ⬇️
sdk/metric/sdk.go 81.3% <0.0%> (+1.1%) ⬆️
sdk/metric/refcount_mapped.go 100.0% <0.0%> (+22.2%) ⬆️

@MrAlias MrAlias changed the title Update Jaeger exporter to rely on the SDK for batching and NoOp performance PoC: Update Jaeger exporter to rely on the SDK for batching and NoOp performance Apr 12, 2021
@MrAlias
Copy link
Contributor Author

MrAlias commented Apr 13, 2021

Going to try and break this into smaller PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the bundler from the Jaeger exporter
1 participant