From c39dab16a59647dd132e5fd8c5461bded63090ec Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Thu, 23 Jan 2020 17:28:49 -0500 Subject: [PATCH 1/3] Skip language tag for external spans --- lib/ddtrace/runtime/metrics.rb | 8 +++++-- .../contrib/rack/configuration_spec.rb | 3 +-- spec/ddtrace/runtime/metrics_spec.rb | 22 +++++++++++++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/ddtrace/runtime/metrics.rb b/lib/ddtrace/runtime/metrics.rb index 5710e918505..ddf02e58b2a 100644 --- a/lib/ddtrace/runtime/metrics.rb +++ b/lib/ddtrace/runtime/metrics.rb @@ -1,3 +1,4 @@ +require 'ddtrace/ext/integration' require 'ddtrace/ext/runtime' require 'ddtrace/metrics' @@ -24,8 +25,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 diff --git a/spec/ddtrace/contrib/rack/configuration_spec.rb b/spec/ddtrace/contrib/rack/configuration_spec.rb index 97d4a011138..5e653966332 100644 --- a/spec/ddtrace/contrib/rack/configuration_spec.rb +++ b/spec/ddtrace/contrib/rack/configuration_spec.rb @@ -76,8 +76,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') diff --git a/spec/ddtrace/runtime/metrics_spec.rb b/spec/ddtrace/runtime/metrics_spec.rb index 0fb6380be3a..aaf488614d9 100644 --- a/spec/ddtrace/runtime/metrics_spec.rb +++ b/spec/ddtrace/runtime/metrics_spec.rb @@ -9,18 +9,26 @@ 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' } - before do - expect(span).to receive(:set_tag) - .with(Datadog::Ext::Runtime::TAG_LANG, Datadog::Runtime::Identity.lang) + before { associate_with_span } - associate_with_span + 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 - it 'registers the span\'s service' do - expect(runtime_metrics.default_metric_options[:tags]).to include("service:#{service}") + 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 From c26dfc2093701ab9f3ca88cd335ce5435eda2a01 Mon Sep 17 00:00:00 2001 From: ericmustin Date: Tue, 8 Sep 2020 15:56:37 +0200 Subject: [PATCH 2/3] fix peer service spec fixtures --- spec/ddtrace/runtime/metrics_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ddtrace/runtime/metrics_spec.rb b/spec/ddtrace/runtime/metrics_spec.rb index 13b17d73ecb..c6df04cf1a3 100644 --- a/spec/ddtrace/runtime/metrics_spec.rb +++ b/spec/ddtrace/runtime/metrics_spec.rb @@ -43,7 +43,7 @@ context 'with external resource span' do let(:span) do - super().tap { |s| s.set_tag(Datadog::Ext::Integration::TAG_PEER_SERVICE, 'peer-service-name') } + Datadog::Span.new(nil, 'dummy', service: service).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 From bead2af85eda5756802a9f681f12863b6650c160 Mon Sep 17 00:00:00 2001 From: ericmustin Date: Tue, 8 Sep 2020 16:34:02 +0200 Subject: [PATCH 3/3] fix other metric spec fixtures for peer service tests --- spec/ddtrace/runtime/metrics_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/ddtrace/runtime/metrics_spec.rb b/spec/ddtrace/runtime/metrics_spec.rb index c6df04cf1a3..9685f0a1443 100644 --- a/spec/ddtrace/runtime/metrics_spec.rb +++ b/spec/ddtrace/runtime/metrics_spec.rb @@ -24,7 +24,7 @@ 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 @@ -43,7 +43,7 @@ context 'with external resource span' do let(:span) do - Datadog::Span.new(nil, 'dummy', service: service).tap { |s| s.set_tag(Datadog::Ext::Integration::TAG_PEER_SERVICE, 'peer-service-name') } + 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