tracing-opentelemetry: Docs for otel.kind are unclear about capitalisation

[nikclayton-dfinity]

Bug Report

Version

Git version https://github.com/tokio-rs/tracing/blob/705613c3ef04e334b90a49a60676ff7de8e495bd/tracing-opentelemetry/src/lib.rs

Description

The documentation is unclear / inconsistent about the capitalisation of values for the otel.kind field.

tracing/tracing-opentelemetry/src/lib.rs

Lines 25 to 28 in 705613c

	//! * `otel.kind`: Set the span kind to one of the supported OpenTelemetry
	//! [span kinds]. The value should be a string of any of the supported values:
	//! `SERVER`, `CLIENT`, `PRODUCER`, `CONSUMER` or `INTERNAL`. Other values are
	//! silently ignored.

lists the values in ALL-CAPS.

The example code, a few lines below, at

tracing/tracing-opentelemetry/src/lib.rs

Line 36 in 705613c

//! `trace_span!("request", "otel.kind" = "client", "http.url" = ..)`, and they

is all lower-case.

The test code in

tracing/tracing-opentelemetry/src/layer.rs

Lines 660 to 682 in 705613c

	tracing::debug_span!("request", otel.kind = "Server");
	});
	let recorded_kind = tracer.0.lock().unwrap().as_ref().unwrap().span_kind.clone();
	assert_eq!(recorded_kind, Some(otel::SpanKind::Server))
	}
	#[test]
	fn trace_id_from_existing_context() {
	let tracer = TestTracer(Arc::new(Mutex::new(None)));
	let subscriber = tracing_subscriber::registry().with(layer().with_tracer(tracer.clone()));
	let trace_id = otel::TraceId::from_u128(42);
	let existing_cx = OtelContext::current_with_span(TestSpan(otel::SpanContext::new(
	trace_id,
	otel::SpanId::from_u64(1),
	0,
	false,
	Default::default(),
	)));
	let _g = existing_cx.attach();
	tracing::collect::with_default(subscriber, || {
	tracing::debug_span!("request", otel.kind = "Server");

uses Initial case / Sentence case.

The actual implementation appears to be case insensitive,

tracing/tracing-opentelemetry/src/layer.rs

Lines 87 to 101 in 705613c

	fn str_to_span_kind(s: &str) -> Option<otel::SpanKind> {
	if s.eq_ignore_ascii_case("SERVER") {
	Some(otel::SpanKind::Server)
	} else if s.eq_ignore_ascii_case("CLIENT") {
	Some(otel::SpanKind::Client)
	} else if s.eq_ignore_ascii_case("PRODUCER") {
	Some(otel::SpanKind::Producer)
	} else if s.eq_ignore_ascii_case("CONSUMER") {
	Some(otel::SpanKind::Consumer)
	} else if s.eq_ignore_ascii_case("INTERNAL") {
	Some(otel::SpanKind::Internal)
	} else {
	None
	}
	}

.

[morigs]

I believe the best option is to replace string with enum. According to OTel spec spankind has a fixed number of variants. And the most actual implementation uses enums.

Type system is the best documentation 😄

[nikclayton-dfinity]

Completely agree -- the equivalent Rust enum is https://github.com/open-telemetry/opentelemetry-rust/blob/45da275bd4d66cf8816eb9f3c7c15961ae2595f0/opentelemetry/src/trace/span.rs#L168-L238. Maybe the documentation could be updated to reflect this as best practice?

[hawkw]

cc @jtescher

[jtescher]

Yeah these docs should be updated to use the enum. E.g. trace_span!("request", "otel.kind" = %SpanKind::Client, "http.url" = ..) is a better choice.