Outdated docs: SpanKind doesn't implement Display anymore

[survived]

Bug Report

The crate docs suggest setting span kind as "otel.kind" = %SpanKind::Client, however, it doesn't compile as SpanKind doesn't implement Display since opentelemetry v0.18 (you can see that display is implemented in v0.17, but in v0.18 it's gone).

tracing-opentelemetry/src/lib.rs

Lines 34 to 37 in 9462de6

	//! OpenTelemetry defines conventional names for attributes of common
	//! operations. These names can be assigned directly as fields, e.g.
	//! `trace_span!("request", "otel.kind" = %SpanKind::Client, "url.full" = ..)`, and they
	//! will be passed through to your configured OpenTelemetry exporter. You can

Version

v0.24.0, rendered on docs.rs

[djc]

Thanks for the report. Want to send a PR to change the % to ??

[survived]

I don't mind sending PR, but I'm not sure that changing % with ? will actually work. Technically, we shouldn't make any assumptions on what std::fmt::Debug implementation outputs, it's supposed to be used in debugging context. In this particular case, Debug implementation will output strings with the first letter capitalized. E.g. format!("{:?}, SpanKind::Client) outputs "Client", whereas Display implementation used to output "client" (all letters are lower-case), which I assume is expected string-representation of the SpanKind.

I don't know what's the best way to address that. The easiest way is to update doc so it directly uses strings: "otel.kind" = %"client" (and maybe refers to the spec where all possible values and their string-representations are listed). The other option is to expose a module like pub mod span_kind { static CLIENT: &str = "client"; /* ... */ }.

[djc]

This was removed in open-telemetry/opentelemetry-rust#758, which asserts the formatting is Jaeger-specific. Might be best to open an issue upstream to discuss further?

[survived]

I don't know what's the best way to proceed tbh since I'm not familiar with otel at all. If the formatting is indeed not a part of the spec, it makes sense to me to replace the SpanKind with the string, i.e. write "otel.kind" = %"client"

[djc]

I don't see an otel.kind attribute here:

https://opentelemetry.io/docs/specs/semconv/attributes-registry/otel/

SpanKind is documented here:

https://opentelemetry.io/docs/specs/otel/trace/api/#spankind

But without an explicit way to Display. So I guess we should maybe remove this attribute from the docs?

[survived]

So I guess we should maybe remove this attribute from the docs?

It does make sense to me!

[survived]

However, looking at the code, it might be not as trivial. Span kind is used here:

tracing-opentelemetry/src/tracer.rs

Lines 82 to 89 in 7f4409a

	builder.sampling_result = Some(provider.config().sampler.should_sample(
	Some(parent_cx),
	trace_id,
	&builder.name,
	builder.span_kind.as_ref().unwrap_or(&SpanKind::Internal),
	builder.attributes.as_deref().unwrap_or(&[]),
	builder.links.as_deref().unwrap_or(&[]),
	));

if we remove it, should_sample will always be called with SpanKind::Internal, that doesn't sound good

[djc]

I was only talking about removing it from the docs, which I don't think would affect that usage?

[mladedav]

I think the example should be changed to otel.kind = "client" so that it at least compiles.

It's a bit unfortunate that this is the example provided as span kind is not a semantic convention, it is part of the span itself. This is just the way tracing-opentelemetry lets the user set it and it should probably be documented somewhere else.

[djc]

I think the example should be changed to otel.kind = "client" so that it at least compiles.

It's a bit unfortunate that this is the example provided as span kind is not a semantic convention, it is part of the span itself. This is just the way tracing-opentelemetry lets the user set it and it should probably be documented somewhere else.

Sounds like you have informed opinions, would you be able to submit a PR for this? 👍