Skip to content

Commit

Permalink
Changed: Isolate google-protobuf to Pprof::Builder class.
Browse files Browse the repository at this point in the history
  • Loading branch information
delner committed Sep 9, 2020
1 parent 5bca986 commit 3ac6fd7
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 29 deletions.
4 changes: 1 addition & 3 deletions lib/ddtrace/profiling/encoding/profile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

require 'ddtrace/profiling/flush'
require 'ddtrace/profiling/pprof/template'
require 'ddtrace/profiling/pprof/pprof_pb'

module Datadog
module Profiling
Expand All @@ -25,8 +24,7 @@ def encode(flushes)
flushes.each { |flush| template.add_events!(flush.event_class, flush.events) }

# Build the profile and encode it
profile = template.to_profile
Perftools::Profiles::Profile.encode(profile).force_encoding(DEFAULT_ENCODING)
template.to_encoded_profile
end
end
end
Expand Down
12 changes: 12 additions & 0 deletions lib/ddtrace/profiling/pprof/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module Profiling
module Pprof
# Accumulates profile data and produces a Perftools::Profiles::Profile
class Builder
DEFAULT_ENCODING = 'UTF-8'.freeze
DESC_FRAME_OMITTED = 'frame omitted'.freeze
DESC_FRAMES_OMITTED = 'frames omitted'.freeze

Expand All @@ -29,6 +30,10 @@ def initialize
@string_table = StringTable.new
end

def encode_profile(profile)
Perftools::Profiles::Profile.encode(profile).force_encoding(DEFAULT_ENCODING)
end

def build_profile
Perftools::Profiles::Profile.new(
sample_type: @sample_types.messages,
Expand Down Expand Up @@ -105,6 +110,13 @@ def build_function(id, filename, function_name)
filename: @string_table.fetch(filename)
)
end

def build_mapping(id, filename)
Perftools::Profiles::Mapping.new(
id: id,
filename: @string_table.fetch(filename)
)
end
end
end
end
Expand Down
15 changes: 6 additions & 9 deletions lib/ddtrace/profiling/pprof/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
require 'ddtrace/profiling/pprof/builder'

require 'ddtrace/profiling/events/stack'

require 'ddtrace/profiling/pprof/pprof_pb'
require 'ddtrace/profiling/pprof/stack_sample'

module Datadog
Expand Down Expand Up @@ -37,12 +35,7 @@ def initialize(mappings)
@sample_type_mappings = Hash.new { |_h, type| raise UnknownSampleTypeMappingError, type }

# Add default mapping
builder.mappings.fetch($PROGRAM_NAME) do |id, prog_name|
Perftools::Profiles::Mapping.new(
id: id,
filename: builder.string_table.fetch(prog_name)
)
end
builder.mappings.fetch($PROGRAM_NAME, &builder.method(:build_mapping))

# Combine all sample types from each converter class
types = mappings.values.each_with_object({}) do |converter_class, t|
Expand Down Expand Up @@ -81,7 +74,11 @@ def add_events!(event_class, events)
end

def to_profile
@profile ||= builder.build_profile
builder.build_profile
end

def to_encoded_profile
builder.encode_profile(to_profile)
end

# Error when an unknown event type is given to be converted
Expand Down
17 changes: 4 additions & 13 deletions spec/ddtrace/profiling/encoding/profile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
let(:template) { instance_double(Datadog::Profiling::Pprof::Template) }
let(:profile) { instance_double(Perftools::Profiles::Profile) }
let(:encoded_profile) { instance_double(String) }
let(:encoded_data) { instance_double(String) }

before do
expect(Datadog::Profiling::Pprof::Template)
Expand All @@ -26,22 +25,14 @@
expect(template)
.to receive(:add_events!)
.with(flush.event_class, flush.events)
.ordered

expect(template)
.to receive(:to_profile)
.and_return(profile)

expect(Perftools::Profiles::Profile)
.to receive(:encode)
.with(profile)
.to receive(:to_encoded_profile)
.and_return(encoded_profile)

expect(encoded_profile)
.to receive(:force_encoding)
.with('UTF-8')
.and_return(encoded_data)
.ordered
end

it { is_expected.to be encoded_data }
it { is_expected.to be encoded_profile }
end
end
35 changes: 35 additions & 0 deletions spec/ddtrace/profiling/pprof/builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,27 @@ def string_id_for(string)
end
end

describe '#encode_profile' do
subject(:build_profile) { builder.encode_profile(profile) }
let(:profile) { instance_double(Perftools::Profiles::Profile) }
let(:encoded_profile) { instance_double(String) }
let(:encoded_string) { instance_double(String) }

before do
expect(Perftools::Profiles::Profile)
.to receive(:encode)
.with(profile)
.and_return(encoded_profile)

expect(encoded_profile)
.to receive(:force_encoding)
.with('UTF-8')
.and_return(encoded_string)
end

it { is_expected.to be(encoded_string) }
end

describe '#build_profile' do
subject(:build_profile) { builder.build_profile }

Expand Down Expand Up @@ -182,4 +203,18 @@ def string_id_for(string)
)
end
end

describe '#build_mapping' do
subject(:build_mapping) { builder.build_mapping(id, filename) }
let(:id) { id_sequence.next }
let(:filename) { double('filename') }

it do
is_expected.to be_a_kind_of(Perftools::Profiles::Mapping)
is_expected.to have_attributes(
id: id,
filename: string_id_for(filename)
)
end
end
end
21 changes: 17 additions & 4 deletions spec/ddtrace/profiling/pprof/template_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,24 @@ def string_id_for(string)
describe '#to_profile' do
subject(:to_profile) { template.to_profile }
it { is_expected.to be_kind_of(Perftools::Profiles::Profile) }
end

context 'called twice' do
it 'returns the same Profile instance' do
is_expected.to eq(template.to_profile)
end
describe '#to_encoded_profile' do
subject(:to_encoded_profile) { template.to_encoded_profile }
let(:profile) { instance_double(Perftools::Profiles::Profile) }
let(:encoded_string) { instance_double(String) }

before do
expect(template.builder)
.to receive(:build_profile)
.and_return(profile)

expect(template.builder)
.to receive(:encode_profile)
.with(profile)
.and_return(encoded_string)
end

it { is_expected.to be(encoded_string) }
end
end

0 comments on commit 3ac6fd7

Please sign in to comment.