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

[PROF-4065] Rename extract_trace_resource to endpoint.collection.enabled #1702

Merged
merged 3 commits into from
Sep 29, 2021
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
2 changes: 1 addition & 1 deletion lib/ddtrace/configuration/components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def build_profiler(settings, agent_settings, tracer)

trace_identifiers_helper = Datadog::Profiling::TraceIdentifiers::Helper.new(
tracer: tracer,
extract_trace_resource: settings.profiling.advanced.extract_trace_resource
endpoint_collection_enabled: settings.profiling.advanced.endpoint.collection.enabled
)

recorder = build_profiler_recorder(settings)
Expand Down
13 changes: 10 additions & 3 deletions lib/ddtrace/configuration/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,16 @@ def logger=(logger)
o.lazy
end

# When using profiling together with tracing, this controls if trace resources (usually the endpoint names)
# are gathered and reported together with profiles.
option :extract_trace_resource, default: true
settings :endpoint do
settings :collection do
# When using profiling together with tracing, this controls if endpoint names
# are gathered and reported together with profiles.
option :enabled do |o|
o.default { env_to_bool(Ext::Profiling::ENV_ENDPOINT_COLLECTION_ENABLED, true) }
o.lazy
end
end
end
end

settings :upload do
Expand Down
1 change: 1 addition & 0 deletions lib/ddtrace/ext/profiling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module Profiling
ENV_UPLOAD_TIMEOUT = 'DD_PROFILING_UPLOAD_TIMEOUT'.freeze
ENV_MAX_FRAMES = 'DD_PROFILING_MAX_FRAMES'.freeze
ENV_AGENTLESS = 'DD_PROFILING_AGENTLESS'.freeze
ENV_ENDPOINT_COLLECTION_ENABLED = 'DD_PROFILING_ENDPOINT_COLLECTION_ENABLED'.freeze

module Pprof
LABEL_KEY_SPAN_ID = 'span id'.freeze
Expand Down
6 changes: 3 additions & 3 deletions lib/ddtrace/profiling/trace_identifiers/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ class Helper
def initialize(
tracer:,
# If this is disabled, the helper will strip the optional trace_resource_container even if provided by the api
extract_trace_resource:,
endpoint_collection_enabled:,
supported_apis: DEFAULT_SUPPORTED_APIS.map { |api| api.new(tracer: tracer) }
)
@extract_trace_resource = extract_trace_resource
@endpoint_collection_enabled = endpoint_collection_enabled
@supported_apis = supported_apis
end

Expand All @@ -34,7 +34,7 @@ def trace_identifiers_for(thread)
trace_identifiers = api.trace_identifiers_for(thread)

if trace_identifiers
return @extract_trace_resource ? trace_identifiers : trace_identifiers[0..1]
return @endpoint_collection_enabled ? trace_identifiers : trace_identifiers[0..1]
end
end

Expand Down
8 changes: 4 additions & 4 deletions spec/ddtrace/configuration/components_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -792,12 +792,12 @@
end

[true, false].each do |value|
context "when extract_trace_resource is #{value}" do
before { settings.profiling.advanced.extract_trace_resource = value }
context "when endpoint_collection_enabled is #{value}" do
before { settings.profiling.advanced.endpoint.collection.enabled = value }

it "initializes the TraceIdentifiers::Helper with extract_trace_resource: #{value}" do
it "initializes the TraceIdentifiers::Helper with endpoint_collection_enabled: #{value}" do
expect(Datadog::Profiling::TraceIdentifiers::Helper)
.to receive(:new).with(tracer: tracer, extract_trace_resource: value)
.to receive(:new).with(tracer: tracer, endpoint_collection_enabled: value)

build_profiler
end
Expand Down
48 changes: 35 additions & 13 deletions spec/ddtrace/configuration/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -589,20 +589,42 @@
end
end

describe '#extract_trace_resource' do
subject(:extract_trace_resource) { settings.profiling.advanced.extract_trace_resource }

it 'defaults to true' do
is_expected.to be true
end
end
describe '#endpoint' do
describe '#collection' do
describe '#enabled' do
subject(:enabled) { settings.profiling.advanced.endpoint.collection.enabled }

context "when #{Datadog::Ext::Profiling::ENV_ENDPOINT_COLLECTION_ENABLED}" do
around do |example|
ClimateControl.modify(Datadog::Ext::Profiling::ENV_ENDPOINT_COLLECTION_ENABLED => environment) do
example.run
end
end

context 'is not defined' do
let(:environment) { nil }

it { is_expected.to be true }
end

{ 'true' => true, 'false' => false }.each do |string, value|
context "is defined as #{string}" do
let(:environment) { string }

it { is_expected.to be value }
end
end
end
end

describe '#extract_trace_resource=' do
it 'updates the #extract_trace_resource setting' do
expect { settings.profiling.advanced.extract_trace_resource = false }
.to change { settings.profiling.advanced.extract_trace_resource }
.from(true)
.to(false)
describe '#enabled=' do
it 'updates the #enabled setting' do
expect { settings.profiling.advanced.endpoint.collection.enabled = false }
.to change { settings.profiling.advanced.endpoint.collection.enabled }
.from(true)
.to(false)
end
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/ddtrace/profiling/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def stack_two
Datadog::Profiling::Collectors::Stack.new(
recorder,
trace_identifiers_helper:
Datadog::Profiling::TraceIdentifiers::Helper.new(tracer: tracer, extract_trace_resource: true),
Datadog::Profiling::TraceIdentifiers::Helper.new(tracer: tracer, endpoint_collection_enabled: true),
max_frames: 400
)
end
Expand Down
8 changes: 4 additions & 4 deletions spec/ddtrace/profiling/trace_identifiers/helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
let(:thread) { instance_double(Thread) }
let(:api1) { instance_double(Datadog::Profiling::TraceIdentifiers::Ddtrace, 'api1') }
let(:api2) { instance_double(Datadog::Profiling::TraceIdentifiers::Ddtrace, 'api2') }
let(:extract_trace_resource) { true }
let(:endpoint_collection_enabled) { true }

subject(:trace_identifiers_helper) do
described_class.new(tracer: nil, extract_trace_resource: extract_trace_resource, supported_apis: [api1, api2])
described_class.new(tracer: nil, endpoint_collection_enabled: endpoint_collection_enabled, supported_apis: [api1, api2])
end

describe '::DEFAULT_SUPPORTED_APIS' do
Expand Down Expand Up @@ -71,8 +71,8 @@
expect(trace_identifiers_for).to eq [:api1_trace_id, :api1_span_id, :api1_trace_resource_container]
end

context 'and extract_trace_resource is set to false' do
let(:extract_trace_resource) { false }
context 'and endpoint_collection_enabled is set to false' do
let(:endpoint_collection_enabled) { false }

it 'returns the trace identifiers but removes the trace resource container' do
expect(trace_identifiers_for).to eq [:api1_trace_id, :api1_span_id]
Expand Down