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

Skip language tag for external spans #935

Merged
merged 4 commits into from Sep 10, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/ddtrace/runtime/metrics.rb
@@ -1,3 +1,4 @@
require 'ddtrace/ext/integration'
require 'ddtrace/ext/runtime'

require 'ddtrace/metrics'
Expand Down Expand Up @@ -25,8 +26,11 @@ def associate_with_span(span)
# Register service as associated with metrics
register_service(span.service) unless span.service.nil?

# Tag span with language and runtime ID for association with metrics
span.set_tag(Ext::Runtime::TAG_LANG, Runtime::Identity.lang)
# Tag span with language and runtime ID for association with metrics.
# We only tag spans that performed internal application work.
unless span.get_tag(Datadog::Ext::Integration::TAG_PEER_SERVICE)
span.set_tag(Ext::Runtime::TAG_LANG, Runtime::Identity.lang)
end
end

# Associate service with runtime metrics
Expand Down
3 changes: 1 addition & 2 deletions spec/ddtrace/contrib/rack/configuration_spec.rb
Expand Up @@ -77,8 +77,7 @@
expect(queue_span.span_type).to eq('proxy')
expect(queue_span.service).to eq(Datadog.configuration[:rack][:web_service_name])
expect(queue_span.start_time.to_i).to eq(queue_time)
# Queue span gets tagged for runtime metrics because its a local root span.
# TODO: It probably shouldn't get tagged like this in the future; it's not part of the runtime.
expect(queue_span.get_tag(Datadog::Ext::Runtime::TAG_LANG)).to be_nil

expect(rack_span.name).to eq('rack.request')
expect(rack_span.span_type).to eq('web')
Expand Down
22 changes: 16 additions & 6 deletions spec/ddtrace/runtime/metrics_spec.rb
Expand Up @@ -24,21 +24,31 @@

describe '#associate_with_span' do
subject(:associate_with_span) { runtime_metrics.associate_with_span(span) }
let(:span) { instance_double(Datadog::Span, service: service) }
let(:span) { Datadog::Span.new(nil, 'dummy', service: service) }
let(:service) { 'parser' }

context 'when enabled' do
before do
runtime_metrics.enabled = true

expect(span).to receive(:set_tag)
.with(Datadog::Ext::Runtime::TAG_LANG, Datadog::Runtime::Identity.lang)

associate_with_span
end

it 'registers the span\'s service' do
expect(runtime_metrics.default_metric_options[:tags]).to include("service:#{service}")
context 'with internal span' do
it 'registers the span\'s service' do
expect(runtime_metrics.default_metric_options[:tags]).to include("service:#{service}")
expect(span.get_tag(Datadog::Ext::Runtime::TAG_LANG)).to eq(Datadog::Runtime::Identity.lang)
end
end

context 'with external resource span' do
let(:span) do
super().tap { |s| s.set_tag(Datadog::Ext::Integration::TAG_PEER_SERVICE, 'peer-service-name') }
end

it "doesn't tag as an internal language span" do
expect(span.get_tag(Datadog::Ext::Runtime::TAG_LANG)).to be nil
end
end
end

Expand Down