Skip to content

Commit 43bb3d3

Browse files
committed
[PROF-12841] Fix profiler not identifying executables with gems they belong to
**What does this PR do?** This PR fixes the profiler's "code provenance" metadata to include the paths to gem executables (if any). This ensures that, when e.g. starting `puma` via `bin/puma` or `bundle exec puma`, we still identify the puma executable as belonging to the `puma` gem. **Motivation:** The "code provenance" is used to power the "Only My Code" feature that shows up in multiple places in the profiler UX, and so we want it to be as accurate as possible. **Additional Notes:** N/A **How to test the change?** This change includes test coverage. It can also be tested easily by running ```bash DD_PROFILING_ENABLED=true DD_SERVICE=test-provenance bundle exec ddprofrb exec pry -e "sleep 1; exit" ``` and checking that `bin/pry` is correctly categorized as belonging to the `pry` gem in the Datadog UX.
1 parent 12d5226 commit 43bb3d3

File tree

6 files changed

+43
-5
lines changed

6 files changed

+43
-5
lines changed

Steepfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ target :datadog do
224224
# References `RubyVM::YJIT`, which does not have type information.
225225
ignore 'lib/datadog/core/environment/yjit.rb'
226226

227+
library 'bundler'
227228
library 'pathname'
228229
library 'cgi'
229230
library 'logger', 'monitor'

lib/datadog/profiling/collectors/code_provenance.rb

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ def initialize(
2222
@libraries_by_path = {}
2323
@seen_files = Set.new
2424
@seen_libraries = Set.new
25+
@executable_paths = [Gem.bindir, (Bundler.bin_path.to_s if defined?(Bundler))].uniq.compact.freeze
2526

2627
record_library(
2728
Library.new(
@@ -51,7 +52,8 @@ def generate_json
5152
:libraries_by_name,
5253
:libraries_by_path,
5354
:seen_files,
54-
:seen_libraries
55+
:seen_libraries,
56+
:executable_paths
5557

5658
def record_library(library)
5759
libraries_by_name[library.name] = library
@@ -79,13 +81,20 @@ def record_loaded_specs(loaded_specs)
7981
loaded_specs.each do |spec|
8082
next if libraries_by_name.key?(spec.name)
8183

84+
extra_paths = [(spec.extension_dir if spec.extensions.any?)]
85+
spec.executables&.each do |executable|
86+
executable_paths.each do |path|
87+
extra_paths << File.join(path, executable)
88+
end
89+
end
90+
8291
record_library(
8392
Library.new(
8493
kind: "library",
8594
name: spec.name,
8695
version: spec.version,
8796
path: spec.gem_dir,
88-
extra_paths: [(spec.extension_dir if spec.extensions.any?)],
97+
extra_paths: extra_paths,
8998
)
9099
)
91100
recorded_library = true
@@ -119,7 +128,7 @@ class Library
119128
attr_reader :kind, :name, :version
120129

121130
def initialize(kind:, name:, version:, path:, extra_paths:)
122-
extra_paths = Array(extra_paths).reject { |p| p.nil? || p.empty? }.map { |p| p.dup.freeze }
131+
extra_paths = Array(extra_paths).compact.reject(&:empty?).map { |p| p.dup.freeze }
123132
@kind = kind.freeze
124133
@name = name.dup.freeze
125134
@version = version.to_s.dup.freeze

sig/datadog/profiling/collectors/code_provenance.rbs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ module Datadog
1212
attr_reader libraries_by_path: Hash[String, Library]
1313
attr_reader seen_files: Set[String]
1414
attr_reader seen_libraries: Set[Library]
15+
attr_reader executable_paths: Array[String]
1516

1617
def record_library: (Library) -> void
1718
def sort_libraries_by_longest_path_first: () -> void

spec/datadog/profiling/collectors/code_provenance_spec.rb

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,16 @@
3232
kind: "library",
3333
name: "datadog",
3434
version: Datadog::VERSION::STRING,
35-
paths: contain_exactly(start_with("/"), include("extensions").and(include(RUBY_PLATFORM))),
35+
paths: contain_exactly(
36+
start_with("/"),
37+
include("extensions").and(include(RUBY_PLATFORM)),
38+
"#{Gem.bindir}/ddprofrb",
39+
"#{Bundler.bin_path}/ddprofrb",
40+
),
3641
},
3742
{
3843
kind: "library",
39-
name: "rspec-core",
44+
name: "rspec",
4045
version: start_with("3."), # This will one day need to be bumped for RSpec 4
4146
paths: contain_exactly(start_with("/")),
4247
},
@@ -59,6 +64,20 @@
5964
)
6065
end
6166

67+
it "includes the executables for gems with executables" do
68+
refresh
69+
70+
expect(generate_result.find { |it| it[:name] == "rspec-core" }.fetch(:paths)).to contain_exactly(
71+
Gem.loaded_specs.fetch("rspec-core").gem_dir,
72+
"#{Gem.bindir}/rspec",
73+
"#{Bundler.bin_path}/rspec",
74+
)
75+
76+
# Sanity checks
77+
expect(Gem.bindir).to start_with("/")
78+
expect(Bundler.bin_path.to_s).to start_with("/")
79+
end
80+
6281
it "records the correct path for datadog" do
6382
refresh
6483

@@ -78,13 +97,15 @@
7897
version: "not_loaded_version",
7998
gem_dir: "/not_loaded/",
8099
extensions: [],
100+
executables: [],
81101
),
82102
instance_double(
83103
Gem::Specification,
84104
name: "is_loaded",
85105
version: "is_loaded_version",
86106
gem_dir: "/is_loaded/",
87107
extensions: [],
108+
executables: [],
88109
)
89110
],
90111
)
@@ -131,13 +152,15 @@
131152
version: "1.2.3",
132153
gem_dir: "/dd-trace-rb",
133154
extensions: [],
155+
executables: [],
134156
),
135157
instance_double(
136158
Gem::Specification,
137159
name: "byebug",
138160
version: "4.5.6",
139161
gem_dir: "/dd-trace-rb/vendor/bundle/ruby/2.7.0/gems/byebug-11.1.3",
140162
extensions: [],
163+
executables: [],
141164
)
142165
],
143166
)

vendor/rbs/bundler/0/bundler.rbs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module Bundler
2+
def self.bin_path: () -> Pathname
3+
end

vendor/rbs/gem/0/gem.rbs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,5 @@ class Gem::BasicSpecification
1616
def gem_dir: () -> String
1717
def extension_dir: () -> String
1818
def extensions: () -> Array[String]
19+
def executables: () -> Array[String]?
1920
end

0 commit comments

Comments
 (0)