opentelemetry-rust icon indicating copy to clipboard operation
opentelemetry-rust copied to clipboard

Ability to access parent span metadata during sampling.

Open kevincox opened this issue 3 years ago • 5 comments

I am attempting to simple my traces and add sampling information to them so that they can be correctly weighted when analyzed. I have code that is mostly working:

#[derive(Clone,Debug)]
struct Sampler;

impl opentelemetry::sdk::trace::ShouldSample for Sampler {
	fn should_sample(&self,
		parent_context: Option<&opentelemetry::Context>,
		trace_id: opentelemetry::trace::TraceId,
		_name: &str,
		_span_kind: &opentelemetry::trace::SpanKind,
		attributes: &opentelemetry::trace::OrderMap<opentelemetry::Key, opentelemetry::Value>,
		_links: &[opentelemetry::trace::Link],
		_instrumentation_library: &opentelemetry::sdk::InstrumentationLibrary,
	) -> opentelemetry::trace::SamplingResult {
		let sample_rate: u128 = attributes.iter()
			.find(|(k, _)| **k == opentelemetry::Key::from_static_str("sample_rate"))
			.map(|(_, v)| match v {
				opentelemetry::Value::I64(rate) => (*rate).try_into().unwrap_or(1),
				ref value => {
					tracing::error!(message_id="zien6Uas",
						?value,
						"Unexpected value for sample rate");
					1
				}
			})
			.unwrap_or(1)
			.max(1);

		let id = u128::from_le_bytes(trace_id.to_bytes());
		let target = u128::max_value() / sample_rate;

		let decision = if id < target {
			opentelemetry::trace::SamplingDecision::RecordAndSample
		} else {
			opentelemetry::trace::SamplingDecision::Drop
		};

		opentelemetry::trace::SamplingResult {
			decision,
			attributes: vec![
				opentelemetry::KeyValue::new("sampleRate", sample_rate),
			],
			trace_state: parent_context
				.map(|ctx| opentelemetry::trace::TraceContextExt::span(ctx)
					.span_context()
					.trace_state()
					.clone())
				.unwrap_or_default(),
		}
	}
}

However this only works correctly for top-level spans or spans that are children of unsampled spans. Notably the child of a span that is sampled 1/2 of the time should also have a sampleRate: 2 attribute applied so that it can be weighted correctly in analysis. However as far as I can tell there is no way to implement this using the current sampling infrastructure as you can't access any information from the parent span other than its trace ID.

kevincox avatar Sep 27 '22 19:09 kevincox

@kevincox unfortunatley I don't believe that this is possible with the current opentelemetry sampling spec, the only data you are given access to is the parent context, you would have to have that information passed some other way if you wanted it present when sampling.

jtescher avatar Sep 30 '22 20:09 jtescher

But this library has both spans so it would be nice if there was an optimistic way to share data between them. I guess I could set up a global cache to store this information but it would be a lot more difficult to manage.

Is there a way to add information to the context when processing a span so that it can be fetched from the parent context when processing the child span?

kevincox avatar Sep 30 '22 20:09 kevincox

Is there a way to add information to the context when processing a span so that it can be fetched from the parent context when processing the child span?

Yeah you can use Context::with_value to add arbitrary data to the parent context. If you control the parent context you can set it explicitly when starting child spans (e.g. Tracer::start_with_context`)

jtescher avatar Sep 30 '22 20:09 jtescher

I don't think that works quite enough, because I want to write to the "current" context but I don't see any way to do that with the current API. (Or can I pull the current context by using tracing_opentelemetry::OpenTelemetrySpanExt::context(&tracing::Span::current()) inside the sampler?) Reading from the parent context seems easy enough, the problem is just saving the sample rate of the current span into the context to be accessed as the parent context later.

kevincox avatar Sep 30 '22 20:09 kevincox

I'm not sure if this will make it in , but we need to make a decision on this before being stable.

hdost avatar Feb 21 '24 08:02 hdost