Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test:Migrate span tests to RSpec #1149

Merged
merged 2 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 199 additions & 1 deletion spec/ddtrace/span_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,127 @@
require 'ddtrace/span'

RSpec.describe Datadog::Span do
subject(:span) { described_class.new(tracer, name, context: context) }
subject(:span) { described_class.new(tracer, name, context: context, **span_options) }
let(:tracer) { get_test_tracer }
let(:context) { Datadog::Context.new }
let(:name) { 'my.span' }
let(:span_options) { {} }

context 'ids' do
it do
expect(span.span_id).to be_nonzero
expect(span.parent_id).to be_zero
expect(span.trace_id).to be_nonzero

expect(span.trace_id).to_not eq(span.span_id)
end

context 'with parent id' do
let(:span_options) { { parent_id: 2 } }
it { expect(span.parent_id).to eq(2) }
end

context 'with trace id' do
let(:span_options) { { trace_id: 3 } }
it { expect(span.trace_id).to eq(3) }
end

context 'set parent span' do
subject(:parent=) { span.parent = parent }

context 'to a span' do
let(:parent) { described_class.new(tracer, 'parent', **parent_span_options) }
let(:parent_span_options) { {} }

before do
parent.sampled = false
subject
end

it do
expect(span.parent).to eq(parent)
expect(span.parent_id).to eq(parent.span_id)
expect(span.trace_id).to eq(parent.trace_id)
expect(span.sampled).to eq(false)
end

context 'with service' do
let(:parent_span_options) { { service: 'parent' } }

it 'copies parent service to child' do
expect(span.service).to eq('parent')
end

context 'with existing child service' do
let(:span_options) { { service: 'child' } }

it 'does not override child service' do
expect(span.service).to eq('child')
end
end
end
end

context 'to nil' do
let(:parent) { nil }

it 'removes the parent' do
subject
expect(span.parent).to be_nil
expect(span.parent_id).to be_zero
expect(span.trace_id).to eq(span.span_id)
end
end
end
end

describe '#finish' do
subject(:finish) { span.finish }

it 'calculates duration' do
expect(span.start_time).to be_nil
expect(span.end_time).to be_nil

subject

expect(span.end_time).to be <= Time.now
expect(span.start_time).to be <= span.end_time
expect(span.to_hash[:duration]).to be >= 0
end

context 'with multiple calls to finish' do
it 'does not flush the span more than once' do
allow(context).to receive(:close_span).once
allow(tracer).to receive(:record).once

subject
expect(span.finish).to be_falsey
end

it 'does not modify the span' do
end_time = subject.end_time

expect(span.finish).to be_falsey
expect(span.end_time).to eq(end_time)
end
end

context 'with finish time provided' do
subject(:finish) { span.finish(time) }
let(:time) { Time.now }

it 'does not use wall time' do
sleep(0.0001)
subject

expect(span.end_time).to eq(time)
end
end

context '#finished?' do
it { expect { subject }.to change { span.finished? }.from(false).to(true) }
end

context 'when an error occurs while closing the span on the context' do
include_context 'health metrics'

Expand Down Expand Up @@ -94,6 +207,74 @@
end
end

describe '#get_metric' do
subject(:get_metric) { span.get_metric(key) }
let(:key) { 'key' }

context 'with no metrics' do
it { is_expected.to be_nil }
end

context 'with a metric' do
let(:value) { 1.0 }
before { span.set_metric(key, value) }

it { is_expected.to eq(1.0) }
end

context 'with a tag' do
let(:value) { 'tag' }
before { span.set_tag(key, value) }

it { is_expected.to eq('tag') }
end
end

describe '#set_metric' do
subject(:set_metric) { span.set_metric(key, value) }
let(:key) { 'key' }

let(:metrics) { span.to_hash[:metrics] }
let(:metric) { metrics[key] }

shared_examples 'a metric' do |value, expected|
let(:value) { value }

it do
subject
expect(metric).to eq(expected)
end
end

context 'with a valid value' do
context 'with an integer' do
it_behaves_like 'a metric', 0, 0.0
end

context 'with a float' do
it_behaves_like 'a metric', 12.34, 12.34
end

context 'with a number as string' do
it_behaves_like 'a metric', '12.34', 12.34
end
end

context 'with an invalid value' do
context 'with nil' do
it_behaves_like 'a metric', nil, nil
end

context 'with a string' do
it_behaves_like 'a metric', 'foo', nil
end

context 'with a complex object' do
it_behaves_like 'a metric', [], nil
end
end
end

describe '#set_tag' do
subject(:set_tag) { span.set_tag(key, value) }

Expand Down Expand Up @@ -338,4 +519,21 @@
end
end
end

describe '#set_error' do
subject(:set_error) { span.set_error(error) }
let(:error) { RuntimeError.new('oops') }
let(:backtrace) { %w[method1 method2 method3] }

before { error.set_backtrace(backtrace) }

it do
subject

expect(span).to have_error
expect(span).to have_error_message('oops')
expect(span).to have_error_type('RuntimeError')
expect(span).to have_error_stack(backtrace.join($RS))
end
end
end
4 changes: 3 additions & 1 deletion spec/ddtrace/tracer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@

context 'is a block' do
it 'yields to the error block and raises the error' do
expect_any_instance_of(Datadog::Span).to_not receive(:set_error)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out a specific combination of any_instance_of and JRuby causes issues like this: https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/1694/workflows/b0a5b0af-711b-4aca-81db-3c99b1d504e5/jobs/81614

I've opened a rspec-mocks issue with this report: rspec/rspec-mocks#1338
I wasn't able to confidently fix it, unfortunately 😕

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate if you could respond in your ticket. Unfortunately, most probably nobody is going to fix it for you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pirj, I'll follow up on the ticket, but long story short: I dedicated around 2 days trying to fix it in rspec-mocks but I wasn't able to.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot. I understand, if it's tolerable on your side, it might not worth the effort. However, if you decide to proceed, I'll be happy to provide any support.

expect do
expect do |b|
tracer.trace(name, on_error: b.to_proc, &block)
Expand All @@ -196,6 +195,9 @@
error
)
end.to raise_error(error)

expect(spans).to have(1).item
expect(spans[0]).to_not have_error
end
end
end
Expand Down
Loading