Skip to content

Commit

Permalink
Merge pull request #3774 from DataDog/ivoanjo/prof-10112-tweak-for-ne…
Browse files Browse the repository at this point in the history
…gative-hashes

[PROF-10112] Also simplify actionview templates with three underscores
  • Loading branch information
ivoanjo authored Jul 10, 2024
2 parents ba23d76 + 52f6cbd commit a152340
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
12 changes: 10 additions & 2 deletions ext/datadog_profiling_native_extension/collectors_stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,14 @@ void sample_thread(
}

// Rails's ActionView likes to dynamically generate method names with suffixed hashes/ids, resulting in methods with
// names such as "_app_views_layouts_explore_html_haml__2304485752546535910_211320".
// names such as:
// * "_app_views_layouts_explore_html_haml__2304485752546535910_211320" (__number_number suffix -- two underscores)
// * "_app_views_articles_index_html_erb___2022809201779434309_12900" (___number_number suffix -- three underscores)
// This makes these stacks not aggregate well, as well as being not-very-useful data.
// (Reference: https://github.com/rails/rails/blob/4fa56814f18fd3da49c83931fa773caa727d8096/actionview/lib/action_view/template.rb#L389 )
// (Reference:
// https://github.com/rails/rails/blob/4fa56814f18fd3da49c83931fa773caa727d8096/actionview/lib/action_view/template.rb#L389
// The two vs three underscores happen when @identifier.hash is negative in that method: the "-" gets replaced with
// the extra "_".)
//
// This method trims these suffixes, so that we keep less data + the names correctly aggregate together.
static void maybe_trim_template_random_ids(ddog_CharSlice *name_slice, ddog_CharSlice *filename_slice) {
Expand All @@ -298,6 +303,9 @@ static void maybe_trim_template_random_ids(ddog_CharSlice *name_slice, ddog_Char
// Make sure there's something left before the underscores (hence the <= instead of <) + match the last underscore
if (pos <= 0 || name_slice->ptr[pos] != '_') return;

// Does it have the optional third underscore? If so, remove it as well
if (pos > 1 && name_slice->ptr[pos-1] == '_') pos--;

// If we got here, we matched on our pattern. Let's slice the length of the string to exclude it.
name_slice->len = pos;
}
Expand Down
15 changes: 14 additions & 1 deletion spec/datadog/profiling/collectors/stack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ def dummy_template.#{method_name}(ready_queue)
# rubocop:enable Style/DocumentDynamicEvalDefinition
end

it 'has a frame with a simplified method name' do
it 'samples the frame with a simplified method name' do
expect(gathered_stack).to include(
have_attributes(
path: '/myapp/app/views/layouts/explore.html.haml',
Expand All @@ -474,6 +474,19 @@ def dummy_template.#{method_name}(ready_queue)
)
end

context 'when method name ends with three ___ instead of two' do
let(:method_name) { super().gsub('__', '___') }

it 'samples the frame with a simplified method name' do
expect(gathered_stack).to include(
have_attributes(
path: '/myapp/app/views/layouts/explore.html.haml',
base_label: '_app_views_layouts_explore_html_haml',
)
)
end
end

context 'when filename ends with .rb' do
let(:filename) { 'example.rb' }

Expand Down

0 comments on commit a152340

Please sign in to comment.