Spec compliance for span and trace IDs #280
Conversation
| @@ -99,7 +99,7 @@ def stub_response(options) | |||
| _(easy.instance_eval { @otel_span }).must_be_nil | |||
| _( | |||
| easy.instance_eval { @otel_original_headers['traceparent'] } | |||
| ).must_equal "00-#{span.trace_id}-#{span.span_id}-01" | |||
| ).must_equal "00-#{span.trace_id.unpack1('H*')}-#{span.span_id.unpack1('H*')}-01" | |||
There was a problem hiding this comment.
This is a very common pattern in the adapter tests. It’s worth thinking about extracting this to a helper in the API or SDK. It can be accomplished with something like:
OpenTelemetry::Trace::Propagation::TraceContext::TraceParent.from_context(span).to_sbut that’s pretty wordy, may not work exactly like this, and doesn’t really reveal the intent terribly well. Something like the following would be better:
OpenTelemetry::Trace.to_traceparent(trace_id: span.trace_id, span_id: span.span_id, sampled: true)I guess that’s even longer, but I think it is clearer.
There was a problem hiding this comment.
+1 on wanting a helper. TBH, I like the first option better, because the second implicitly couples the Trace module to W3C trace context. Both are wordy, and I could see sticking it somewhere else altogether.
A related issue is that we don't have a good way to share a) test helpers, b) shared logic between adapters. We should think about how we want to solve that moving forward.
fbogsany
requested review from
bai,
dazuma,
elskwid,
luvtechno and
mwear
as code owners
| @@ -99,7 +99,7 @@ def stub_response(options) | |||
| _(easy.instance_eval { @otel_span }).must_be_nil | |||
| _( | |||
| easy.instance_eval { @otel_original_headers['traceparent'] } | |||
| ).must_equal "00-#{span.trace_id}-#{span.span_id}-01" | |||
| ).must_equal "00-#{span.trace_id.unpack1('H*')}-#{span.span_id.unpack1('H*')}-01" | |||
There was a problem hiding this comment.
+1 on wanting a helper. TBH, I like the first option better, because the second implicitly couples the Trace module to W3C trace context. Both are wordy, and I could see sticking it somewhere else altogether.
A related issue is that we don't have a good way to share a) test helpers, b) shared logic between adapters. We should think about how we want to solve that moving forward.
|
👍 I created open-telemetry/opentelemetry-ruby-contrib#18 to track this. |
Fixes #232