feat(otel): add support for span links

[mabdinur]

What does this PR do?

This PR ensures that when span links are generative via the OpenTelemetry API the values stored on the link are set on the underlying datadog span.

Motivation:

Support OTEL Span Links and provide better support for otel instrumentations

[marcotc]

Suggested change

	trace_id: link.span_context.trace_id.unpack1('Q'),
	trace_id: to_datadog_id(link.span_context.trace_id),

Can you wrap unpack1('Q') into a method, so that we know what it does in the future?

For example, this is how we do the inverse operation:

dd-trace-rb/lib/datadog/opentelemetry/sdk/propagator.rb

Lines 84 to 88 in 53953ac

	# Converts the {Numeric} Datadog id object to OpenTelemetry's byte array format.
	# This method currently converts an unsigned 64-bit Integer to a binary String.
	def to_otel_id(dd_id)
	Array(dd_id).pack('Q')
	end

[mabdinur]

I updated this code to use hex_trace_id and hex_span_id field on the otel SpanContext. This way we ensure the otel library encodes/decodes bytes in a consistent manner (I believe they use Array.pack('H*') for trace ids and Array.pack('Q') for span ids).

[marcotc]

You'll want to add a test to the given an option group for links:

dd-trace-rb/spec/datadog/tracing/span_operation_spec.rb

Lines 103 to 289 in 7e9fe1e

	context 'given an option' do
	shared_examples 'a string property' do |nillable: true|
	let(:options) { { property => value } }
	context 'set to a String' do
	let(:value) { 'test string' }
	it { is_expected.to have_attributes(property => value) }
	end
	context 'set to a non-UTF-8 String' do
	let(:value) { 'ascii'.encode(Encoding::ASCII) }
	it { is_expected.to have_attributes(property => value) }
	it { expect(span_op.public_send(property).encoding).to eq(Encoding::UTF_8) }
	end
	context 'invoking the public setter' do
	subject! { span_op.public_send("#{property}=", value) }
	context 'with a string' do
	let(:value) { 'test string' }
	it { expect(span_op).to have_attributes(property => value) }
	end
	context 'with a string that is not in UTF-8' do
	let(:value) { 'ascii'.encode(Encoding::ASCII) }
	it { expect(span_op).to have_attributes(property => value) }
	it { expect(span_op.public_send(property).encoding).to eq(Encoding::UTF_8) }
	end
	end
	if nillable
	context 'set to nil' do
	let(:value) { nil }
	# Allow property to be explicitly set to nil
	it { is_expected.to have_attributes(property => nil) }
	end
	else
	context 'set to nil' do
	let(:value) { nil }
	it { expect { subject }.to raise_error(ArgumentError) }
	end
	end
	end
	context ':on_error' do
	let(:options) { { on_error: block } }
	let(:block) { proc { raise error } }
	let(:error) { error_class.new('error message') }
	let(:error_class) { stub_const('TestError', Class.new(StandardError)) }
	context 'that is nil' do
	let(:on_error) { nil }
	context 'and #measure raises an error' do
	subject(:measure) { span_op.measure { raise error } }
	before { allow(span_op).to receive(:set_error) }
	it 'propagates the error' do
	expect { measure }.to raise_error(error)
	expect(span_op).to have_received(:set_error).with(error)
	end
	end
	end
	context 'that is a block' do
	let(:on_error) { block }
	it 'yields to the error block and raises the error' do
	expect do
	expect do |b|
	options[:on_error] = b.to_proc
	span_op.measure(&block)
	end.to yield_with_args(
	a_kind_of(described_class),
	error
	)
	end.to raise_error(error)
	# It should not set an error, as this overrides behavior.
	expect(span_op).to_not have_error
	end
	end
	context 'that is not a Proc' do
	let(:on_error) { 'not a proc' }
	it 'fallbacks to default error handler and log a debug message' do
	expect(Datadog.logger).to receive(:debug).at_least(:once)
	expect do
	span_op.measure(&block)
	end.to raise_error(error)
	end
	end
	end
	describe ':name' do
	it_behaves_like 'a string property', nillable: false do
	let(:property) { :name }
	# :name is not a keyword argument, but positional.
	# We swap those two here.
	let(:options) { {} }
	let(:name) { value }
	end
	end
	describe ':parent_id' do
	let(:options) { { parent_id: parent_id } }
	context 'that is nil' do
	let(:parent_id) { nil }
	it { is_expected.to have_attributes(parent_id: 0) }
	end
	context 'that is an Integer' do
	let(:parent_id) { instance_double(Integer) }
	it { is_expected.to have_attributes(parent_id: parent_id) }
	end
	end
	describe ':resource' do
	it_behaves_like 'a string property' do
	let(:property) { :resource }
	end
	end
	describe ':service' do
	it_behaves_like 'a string property' do
	let(:property) { :service }
	end
	end
	describe ':start_time' do
	let(:options) { { start_time: start_time } }
	let(:start_time) { instance_double(Time) }
	context 'that is nil' do
	let(:start_time) { nil }
	it { is_expected.to have_attributes(start_time: nil) }
	end
	context 'that is a Time' do
	let(:start_time) { instance_double(Time) }
	it { is_expected.to have_attributes(start_time: start_time) }
	end
	end
	describe ':tags' do
	let(:options) { { tags: tags } }
	context 'that is nil' do
	let(:tags) { nil }
	it_behaves_like 'a root span operation'
	end
	context 'that is a Hash' do
	let(:tags) { { 'custom_tag' => 'custom_value' } }
	it_behaves_like 'a root span operation'
	it { expect(span_op.get_tag('custom_tag')).to eq(tags['custom_tag']) }
	end
	end
	describe ':trace_id' do
	let(:options) { { trace_id: trace_id } }
	context 'that is nil' do
	let(:trace_id) { nil }
	it { is_expected.to have_attributes(trace_id: kind_of(Integer)) }
	end
	context 'that is an Integer' do
	let(:trace_id) { Datadog::Tracing::Utils::TraceId.next_id }
	it { is_expected.to have_attributes(trace_id: trace_id) }
	end
	end
	describe ':type' do
	it_behaves_like 'a string property' do
	let(:property) { :type }
	end
	end
	end
	end

[mabdinur]

I added test coverage for the links field in span_operation

[marcotc]

@mabdinur I think we only have one test failure left here:

  1) Datadog::OpenTelemetry with Datadog TraceProvider #start_span with span_links sets span links
     Failure/Error: expect(span.links[1].span_id).to eq(67890)
     
       expected: 67890
            got: 10
     
       (compared using ==)
     # ./spec/datadog/opentelemetry_spec.rb:355:in `block (5 levels) in <top (required)>'
     # ./spec/spec_helper.rb:230:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:115:in `block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/webmock-3.13.0/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block (2 levels) in <top (required)>'

Finished in 0.59189 seconds (files took 0.88941 seconds to load)
140 examples, 1 failure

Failed examples:

rspec ./spec/datadog/opentelemetry_spec.rb:344 # Datadog::OpenTelemetry with Datadog TraceProvider #start_span with span_links sets span links

[marcotc]

@mabdinur, do we have span links system tests in place already?

[marcotc]

Different failure now, possibly because of the 2.0 branch being merged into master:

  1) Datadog::Tracing::SpanOperation::new given an option :links that is an Array 
     Failure/Error:
       def initialize(
         digest,
         attributes: nil
       )
         @span_id = digest.span_id
         @trace_id = digest.trace_id
         @trace_flags = if digest.trace_sampling_priority.nil?
                          nil
                        elsif digest.trace_sampling_priority > 0
                          1
     
     ArgumentError:
       wrong number of arguments (given 0, expected 1)
     # ./lib/datadog/tracing/span_link.rb:43:in `initialize'
     # ./spec/datadog/tracing/span_operation_spec.rb:244:in `new'
     # ./spec/datadog/tracing/span_operation_spec.rb:244:in `block (6 levels) in <top (required)>'
     # ./spec/datadog/tracing/span_operation_spec.rb:240:in `block (5 levels) in <top (required)>'
     # ./spec/datadog/tracing/span_operation_spec.rb:17:in `block (2 levels) in <top (required)>'
     # ./spec/datadog/tracing/span_operation_spec.rb:249:in `block (6 levels) in <top (required)>'
     # ./spec/spec_helper.rb:230:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:115:in `block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/rspec-wait-0.0.10/lib/rspec/wait.rb:47:in `block (2 levels) in <top (required)>'

Finished in 1 minute 5.14 seconds (files took 2.74 seconds to load)
4289 examples, 1 failure, 46 pending

Failed examples:

rspec ./spec/datadog/tracing/span_operation_spec.rb:249 # Datadog::Tracing::SpanOperation::new given an option :links that is an Array

[mabdinur]

@mabdinur, do we have span links system tests in place already?

Yes I plan to enable them in this PR: https://github.com/DataDog/system-tests/pull/2351/files

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.13%. Comparing base (e581d87) to head (683dd7b).
Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3572   +/-   ##
=======================================
  Coverage   98.13%   98.13%           
=======================================
  Files        1223     1223           
  Lines       72146    72185   +39     
  Branches     3425     3428    +3     
=======================================
+ Hits        70797    70836   +39     
  Misses       1349     1349           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.