fix: jaeger collector only works in batch mode

[skyzh]

The docs is misleading for jaeger HTTP collector mode -- install_simple will always initialize a UDP client even if users have configured to use HTTP collector.

In install_simple, we called build_simple.

opentelemetry-rust/opentelemetry-jaeger/src/exporter/mod.rs

Lines 324 to 333 in 605d6c0

	pub fn install_simple(self) -> Result<sdk::trace::Tracer, TraceError> {
	let tracer_provider = self.build_simple()?;
	let tracer = tracer_provider.versioned_tracer(
	"opentelemetry-jaeger",
	Some(env!("CARGO_PKG_VERSION")),
	None,
	);
	let _ = global::set_tracer_provider(tracer_provider);
	Ok(tracer)
	}

In build_simple, we called init_sync_exporter_with_process.

opentelemetry-rust/opentelemetry-jaeger/src/exporter/mod.rs

Lines 405 to 413 in 605d6c0

	pub fn build_simple(mut self) -> Result<sdk::trace::TracerProvider, TraceError> {
	let mut builder = sdk::trace::TracerProvider::builder();
	let (config, process) = self.build_config_and_process(builder.sdk_provided_resource());
	let exporter = self.init_sync_exporter_with_process(process)?;
	builder = builder.with_simple_exporter(exporter);
	builder = builder.with_config(config);
	Ok(builder.build())
	}

In that, we calls init_sync_uploader.

opentelemetry-rust/opentelemetry-jaeger/src/exporter/mod.rs

Lines 439 to 448 in 605d6c0

	fn init_sync_exporter_with_process(self, process: Process) -> Result<Exporter, TraceError> {
	let export_instrumentation_lib = self.export_instrument_library;
	let uploader = self.init_sync_uploader()?;
	Ok(Exporter {
	process: process.into(),
	export_instrumentation_lib,
	uploader,
	})
	}

... which ignores the HTTP collector settings and will only create a UDP socket.

opentelemetry-rust/opentelemetry-jaeger/src/exporter/mod.rs

Lines 477 to 485 in 605d6c0

	fn init_sync_uploader(self) -> Result<Box<dyn Uploader>, TraceError> {
	let agent = agent::AgentSyncClientUdp::new(
	self.agent_endpoint.as_slice(),
	self.max_packet_size,
	self.auto_split,
	)
	.map_err::<Error, _>(Into::into)?;
	Ok(Box::new(SyncUploader::Agent(agent)))
	}

In the future, we should warn users or force use reqwest / isahc sync mode here. But for now, I think it's a quick fix to simply change the docs and prevent readers to mis-configure their clients.

Thanks for review!

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Alex Chi (a8cfbd2)

[skyzh]

cc @djc @TommyCpp would you please take a look? Thanks!

[codecov]

Codecov Report

Merging #706 (5972c5d) into main (d03926a) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #706      +/-   ##
==========================================
+ Coverage   71.36%   71.38%   +0.01%     
==========================================
  Files         101      101              
  Lines        8557     8557              
==========================================
+ Hits         6107     6108       +1     
+ Misses       2450     2449       -1     

Impacted Files	Coverage Δ
opentelemetry-jaeger/src/lib.rs	93.04% <ø> (ø)
opentelemetry/src/sdk/metrics/controllers/push.rs	83.33% <0.00%> (+3.33%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d03926a...5972c5d. Read the comment docs.

[TommyCpp]

Good catch! And yes we probably need to use sync mode in HTTP clients if that's available.

I believe we also need to add #[tokio::main] on top of the main function and make it async for it to works.

[skyzh]

Good catch! And yes we probably need to use sync mode in HTTP clients if that's available.

I believe we also need to add #[tokio::main] on top of the main function and make it async for it to works.

Thanks! However, it seems other parts of this docs don't add #[tokio::main] for those examples. For example,

opentelemetry-rust/opentelemetry-jaeger/src/lib.rs

Lines 136 to 151 in 5972c5d

	//! ## Kitchen Sink Full Configuration
	//!
	//! Example showing how to override all configuration options. See the
	//! [`PipelineBuilder`] docs for details of each option.
	//!
	//!
	//! ```no_run
	//! use opentelemetry::{KeyValue, trace::{Tracer, TraceError}};
	//! use opentelemetry::sdk::{trace::{self, IdGenerator, Sampler}, Resource};
	//! use opentelemetry::global;
	//!
	//! fn main() -> Result<(), TraceError> {
	//! global::set_text_map_propagator(opentelemetry_jaeger::Propagator::new());
	//! let tracer = opentelemetry_jaeger::new_pipeline()
	//! .with_agent_endpoint("localhost:6831")
	//! .with_service_name("my_app")

As those examples won't run in doctests, I think it's reasonable to ignore things like #[tokio::main]?

[TommyCpp]

LGTM. Thanks