From 1446109ea58be649fdc0740f59a1c36a2a38c9f1 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Mon, 8 Jul 2024 09:30:11 +0100 Subject: [PATCH] Replace "Missing Allocations" with "Skipped Samples" for placeholder name This was a great suggestion from the PR review. --- .../collectors_cpu_and_wall_time_worker.c | 4 ++-- .../collectors_thread_context.c | 18 +++++++-------- .../collectors_thread_context.h | 2 +- .../cpu_and_wall_time_worker_spec.rb | 6 ++--- .../collectors/thread_context_spec.rb | 22 +++++++++---------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c b/ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c index c132fd2eb40..e27f8b53894 100644 --- a/ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c +++ b/ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c @@ -1145,9 +1145,9 @@ static VALUE rescued_sample_allocation(VALUE tracepoint_data) { // assigning a very large number to a sample, if for instance the dynamic sampling mechanism chose a really big interval. unsigned int weight = allocations_since_last_sample > MAX_ALLOC_WEIGHT ? MAX_ALLOC_WEIGHT : (unsigned int) allocations_since_last_sample; thread_context_collector_sample_allocation(state->thread_context_collector_instance, weight, new_object); - // ...but we still represent the missing allocations in the profile, thus the data will account for all allocations. + // ...but we still represent the skipped samples in the profile, thus the data will account for all allocations. if (weight < allocations_since_last_sample) { - thread_context_collector_sample_missed_allocations(state->thread_context_collector_instance, allocations_since_last_sample - weight); + thread_context_collector_sample_skipped_allocation_samples(state->thread_context_collector_instance, allocations_since_last_sample - weight); } // Return a dummy VALUE because we're called from rb_rescue2 which requires it diff --git a/ext/datadog_profiling_native_extension/collectors_thread_context.c b/ext/datadog_profiling_native_extension/collectors_thread_context.c index dad402523a6..6f12c7e64a0 100644 --- a/ext/datadog_profiling_native_extension/collectors_thread_context.c +++ b/ext/datadog_profiling_native_extension/collectors_thread_context.c @@ -231,7 +231,7 @@ static void ddtrace_otel_trace_identifiers_for( VALUE active_span, VALUE otel_values ); -static VALUE _native_sample_missed_allocations(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE missed_allocations); +static VALUE _native_sample_skipped_allocation_samples(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE missed_allocations); void collectors_thread_context_init(VALUE profiling_module) { VALUE collectors_module = rb_define_module_under(profiling_module, "Collectors"); @@ -262,7 +262,7 @@ void collectors_thread_context_init(VALUE profiling_module) { rb_define_singleton_method(testing_module, "_native_stats", _native_stats, 1); rb_define_singleton_method(testing_module, "_native_gc_tracking", _native_gc_tracking, 1); rb_define_singleton_method(testing_module, "_native_new_empty_thread", _native_new_empty_thread, 0); - rb_define_singleton_method(testing_module, "_native_sample_missed_allocations", _native_sample_missed_allocations, 2); + rb_define_singleton_method(testing_module, "_native_sample_skipped_allocation_samples", _native_sample_skipped_allocation_samples, 2); at_active_span_id = rb_intern_const("@active_span"); at_active_trace_id = rb_intern_const("@active_trace"); @@ -1403,14 +1403,14 @@ static void ddtrace_otel_trace_identifiers_for( *numeric_span_id = resolved_numeric_span_id; } -void thread_context_collector_sample_missed_allocations(VALUE self_instance, unsigned int missed_allocations) { +void thread_context_collector_sample_skipped_allocation_samples(VALUE self_instance, unsigned int missed_allocations) { struct thread_context_collector_state *state; TypedData_Get_Struct(self_instance, struct thread_context_collector_state, &thread_context_collector_typed_data, state); ddog_prof_Label labels[] = { - {.key = DDOG_CHARSLICE_C("thread id"), .str = DDOG_CHARSLICE_C("MA")}, - {.key = DDOG_CHARSLICE_C("thread name"), .str = DDOG_CHARSLICE_C("Missing Allocations")}, - {.key = DDOG_CHARSLICE_C("allocation class"), .str = DDOG_CHARSLICE_C("(Missing Allocations)")}, + {.key = DDOG_CHARSLICE_C("thread id"), .str = DDOG_CHARSLICE_C("SA")}, + {.key = DDOG_CHARSLICE_C("thread name"), .str = DDOG_CHARSLICE_C("Skipped Samples")}, + {.key = DDOG_CHARSLICE_C("allocation class"), .str = DDOG_CHARSLICE_C("(Skipped Samples)")}, }; ddog_prof_Slice_Label slice_labels = {.ptr = labels, .len = sizeof(labels) / sizeof(labels[0])}; @@ -1423,11 +1423,11 @@ void thread_context_collector_sample_missed_allocations(VALUE self_instance, uns .state_label = NULL, .end_timestamp_ns = 0, // For now we're not collecting timestamps for allocation events }, - DDOG_CHARSLICE_C("Missing Allocations") + DDOG_CHARSLICE_C("Skipped Samples") ); } -static VALUE _native_sample_missed_allocations(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE missed_allocations) { - thread_context_collector_sample_missed_allocations(collector_instance, NUM2UINT(missed_allocations)); +static VALUE _native_sample_skipped_allocation_samples(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE missed_allocations) { + thread_context_collector_sample_skipped_allocation_samples(collector_instance, NUM2UINT(missed_allocations)); return Qtrue; } diff --git a/ext/datadog_profiling_native_extension/collectors_thread_context.h b/ext/datadog_profiling_native_extension/collectors_thread_context.h index 8566b16398d..16e9405ca3b 100644 --- a/ext/datadog_profiling_native_extension/collectors_thread_context.h +++ b/ext/datadog_profiling_native_extension/collectors_thread_context.h @@ -9,7 +9,7 @@ void thread_context_collector_sample( VALUE profiler_overhead_stack_thread ); void thread_context_collector_sample_allocation(VALUE self_instance, unsigned int sample_weight, VALUE new_object); -void thread_context_collector_sample_missed_allocations(VALUE self_instance, unsigned int missed_allocations); +void thread_context_collector_sample_skipped_allocation_samples(VALUE self_instance, unsigned int missed_allocations); VALUE thread_context_collector_sample_after_gc(VALUE self_instance); void thread_context_collector_on_gc_start(VALUE self_instance); bool thread_context_collector_on_gc_finish(VALUE self_instance); diff --git a/spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb b/spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb index 0bfc3c5ff9b..954749500b9 100644 --- a/spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb +++ b/spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb @@ -543,15 +543,15 @@ # We then assign a weight to every sample to compensate for this; to avoid bias, we have a limit on this weight, # and we clamp it if it goes over the limit. # But the total amount of allocations recorded should match the number we observed, and thus we record the - # remainder above the clamped value as a separate "Missing Allocations" step. - it 'records missed allocations when weights are clamped' do + # remainder above the clamped value as a separate "Skipped Samples" step. + it 'records skipped allocation samples when weights are clamped' do start thread_that_allocates_as_fast_as_possible = Thread.new { loop { BasicObject.new } } allocation_samples = try_wait_until do samples = samples_from_pprof(recorder.serialize!).select { |it| it.values[:'alloc-samples'] > 0 } - samples if samples.any? { |it| it.labels[:'thread name'] == 'Missing Allocations' } + samples if samples.any? { |it| it.labels[:'thread name'] == 'Skipped Samples' } end thread_that_allocates_as_fast_as_possible.kill diff --git a/spec/datadog/profiling/collectors/thread_context_spec.rb b/spec/datadog/profiling/collectors/thread_context_spec.rb index 3f76d6f1a4b..09fc04e19b3 100644 --- a/spec/datadog/profiling/collectors/thread_context_spec.rb +++ b/spec/datadog/profiling/collectors/thread_context_spec.rb @@ -79,8 +79,8 @@ def sample_allocation(weight:, new_object: Object.new) described_class::Testing._native_sample_allocation(cpu_and_wall_time_collector, weight, new_object) end - def sample_missed_allocations(missed_allocations) - described_class::Testing._native_sample_missed_allocations(cpu_and_wall_time_collector, missed_allocations) + def sample_skipped_allocation_samples(missed_allocations) + described_class::Testing._native_sample_skipped_allocation_samples(cpu_and_wall_time_collector, missed_allocations) end def thread_list @@ -1214,28 +1214,28 @@ def self.otel_sdk_available? end end - describe '#sample_missed_allocations' do + describe '#sample_skipped_allocation_samples' do let(:single_sample) do expect(samples.size).to be 1 samples.first end - before { sample_missed_allocations(123) } + before { sample_skipped_allocation_samples(123) } - it 'records the number of missed allocations' do + it 'records the number of skipped allocations' do expect(single_sample.values).to include('alloc-samples': 123) end - it 'attributes the missed allocations to a "Missing Allocations" thread' do - expect(single_sample.labels).to include('thread id': 'MA', 'thread name': 'Missing Allocations') + it 'attributes the missed allocations to a "Skipped Samples" thread' do + expect(single_sample.labels).to include('thread id': 'SA', 'thread name': 'Skipped Samples') end - it 'attributes the missed allocations to a "(Missing Allocations)" allocation class' do - expect(single_sample.labels).to include('allocation class': '(Missing Allocations)') + it 'attributes the missed allocations to a "(Skipped Samples)" allocation class' do + expect(single_sample.labels).to include('allocation class': '(Skipped Samples)') end - it 'includes a placeholder stack attributed to "Missing Allocations"' do + it 'includes a placeholder stack attributed to "Skipped Samples"' do expect(single_sample.locations.size).to be 1 - expect(single_sample.locations.first.path).to eq 'Missing Allocations' + expect(single_sample.locations.first.path).to eq 'Skipped Samples' end end