Merge remote-tracking branch 'origin/master' into landerson/one-pipeline

.vscode/extensions.json

@@ -0,0 +1,8 @@
+{
+    "recommendations": [
+        "shopify.ruby-lsp",
+        "editorconfig.editorconfig",
+        "soutaro.rbs-syntax",
+        "soutaro.steep-vscode"
+    ]
+}

Appraisals

@@ -17,6 +17,9 @@ REMOVED_GEMS = {
     'rbs',
     'steep',
   ],
+  :dev => [
+    'ruby-lsp',
+  ],
 }
 
 def appraise(group, &block)

CHANGELOG.md

@@ -2,6 +2,36 @@
 
 ## [Unreleased]
 
+## [2.2.0] - 2024-07-11
+
+### Added
+
+* Tracing: Add `Rails` Runner instrumentation ([#2509][])
+* Tracing: Introduce a new, reworked `GraphQL` tracer to comply with span attributes specification ([#3672][])
+* Tracing: Enhance `ActiveSupport::Cache` instrumentation with `ActiveSupport::Notifications` subscription ([#3772][])
+* Profiling: Track unscaled allocation counts in allocation profiler ([#3770][])
+
+### Changed
+
+* Core: Send Telemetry events in batches ([#3749][])
+* Tracing: Populate spans from `ActiveSupport::Notifications` as early as possible ([#3725][])
+* Profiling: Upgrade to `libdatadog` 10 ([#3753][])
+* Profiling: Optimize `CodeProvenance#record_loaded_files` to avoid allocations ([#3757][])
+
+### Fixed
+
+* Core: Fix Telemetry events blocking main thread ([#3718][])
+* Core: Fix deadlock from Telemetry threads ([#3743][])
+* Tracing: Fix empty log correlation when tracing is disabled ([#3731][])
+* Tracing: Fix HTTP propagation when missing parent span id ([#3730][])
+* Tracing: Ensure `_dd.p.tid` tag with fixed size ([#3729][])
+* OTel: Fix ids encoding/decoding for propagation ([#3709][])
+* Profiling: Workaround Ruby `Dir` returning incorrect results ([#3720][])
+* Profiling: Fix `Phusion Passenger` detection when missing from `Gemfile`/`gems.rb` ([#3750][])
+* Profiling: Fix `rpath` for linking to libdatadog when loading extension ([#3706][])
+* Profiling: Fix incorrect code provenance due to broken JSON monkey patch ([#3695][])
+* Profiling: Fix aggregation of actionview template classes ([#3759][], [#3774][])
+
 ## [2.1.0] - 2024-06-10
 
 ### Added
@@ -2904,7 +2934,8 @@ Release notes: https://github.com/DataDog/dd-trace-rb/releases/tag/v0.3.1
 Git diff: https://github.com/DataDog/dd-trace-rb/compare/v0.3.0...v0.3.1
 
 
-[Unreleased]: https://github.com/DataDog/dd-trace-rb/compare/v2.1.0...master
+[Unreleased]: https://github.com/DataDog/dd-trace-rb/compare/v2.2.0...master
+[2.2.0]: https://github.com/DataDog/dd-trace-rb/compare/v2.1.0...v2.2.0
 [2.1.0]: https://github.com/DataDog/dd-trace-rb/compare/v2.0.0...v2.1.0
 [2.0.0]: https://github.com/DataDog/dd-trace-rb/compare/v2.0.0.rc1...v2.0.0
 [2.0.0.rc1]: https://github.com/DataDog/dd-trace-rb/compare/v2.0.0.beta2...v2.0.0.rc1
@@ -3901,6 +3932,7 @@ Git diff: https://github.com/DataDog/dd-trace-rb/compare/v0.3.0...v0.3.1
 [#2497]: https://github.com/DataDog/dd-trace-rb/issues/2497
 [#2501]: https://github.com/DataDog/dd-trace-rb/issues/2501
 [#2504]: https://github.com/DataDog/dd-trace-rb/issues/2504
+[#2509]: https://github.com/DataDog/dd-trace-rb/issues/2509
 [#2512]: https://github.com/DataDog/dd-trace-rb/issues/2512
 [#2513]: https://github.com/DataDog/dd-trace-rb/issues/2513
 [#2522]: https://github.com/DataDog/dd-trace-rb/issues/2522
@@ -4283,6 +4315,25 @@ Git diff: https://github.com/DataDog/dd-trace-rb/compare/v0.3.0...v0.3.1
 [#3651]: https://github.com/DataDog/dd-trace-rb/issues/3651
 [#3657]: https://github.com/DataDog/dd-trace-rb/issues/3657
 [#3664]: https://github.com/DataDog/dd-trace-rb/issues/3664
+[#3672]: https://github.com/DataDog/dd-trace-rb/issues/3672
+[#3695]: https://github.com/DataDog/dd-trace-rb/issues/3695
+[#3706]: https://github.com/DataDog/dd-trace-rb/issues/3706
+[#3709]: https://github.com/DataDog/dd-trace-rb/issues/3709
+[#3718]: https://github.com/DataDog/dd-trace-rb/issues/3718
+[#3720]: https://github.com/DataDog/dd-trace-rb/issues/3720
+[#3725]: https://github.com/DataDog/dd-trace-rb/issues/3725
+[#3729]: https://github.com/DataDog/dd-trace-rb/issues/3729
+[#3730]: https://github.com/DataDog/dd-trace-rb/issues/3730
+[#3731]: https://github.com/DataDog/dd-trace-rb/issues/3731
+[#3743]: https://github.com/DataDog/dd-trace-rb/issues/3743
+[#3749]: https://github.com/DataDog/dd-trace-rb/issues/3749
+[#3750]: https://github.com/DataDog/dd-trace-rb/issues/3750
+[#3753]: https://github.com/DataDog/dd-trace-rb/issues/3753
+[#3757]: https://github.com/DataDog/dd-trace-rb/issues/3757
+[#3759]: https://github.com/DataDog/dd-trace-rb/issues/3759
+[#3770]: https://github.com/DataDog/dd-trace-rb/issues/3770
+[#3772]: https://github.com/DataDog/dd-trace-rb/issues/3772
+[#3774]: https://github.com/DataDog/dd-trace-rb/issues/3774
 [@AdrianLC]: https://github.com/AdrianLC
 [@Azure7111]: https://github.com/Azure7111
 [@BabyGroot]: https://github.com/BabyGroot
@@ -4434,4 +4485,4 @@ Git diff: https://github.com/DataDog/dd-trace-rb/compare/v0.3.0...v0.3.1
 [@y-yagi]: https://github.com/y-yagi
 [@yujideveloper]: https://github.com/yujideveloper
 [@yukimurasawa]: https://github.com/yukimurasawa
-[@zachmccormick]: https://github.com/zachmccormick
+[@zachmccormick]: https://github.com/zachmccormick

Gemfile

@@ -84,6 +84,10 @@ group :check do
   end
 end
 
+group :dev do
+  gem 'ruby-lsp', require: false if RUBY_VERSION >= '3.0.0' && RUBY_PLATFORM != 'java'
+end
+
 # `1.17.0` provides broken RBS type definitions
 # https://github.com/ffi/ffi/blob/master/CHANGELOG.md#1170rc1--2024-04-08
 #

ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c

@@ -1140,15 +1140,9 @@ static VALUE rescued_sample_allocation(VALUE tracepoint_data) {
     discrete_dynamic_sampler_events_since_last_sample(&state->allocation_sampler) :
     // if we aren't, then we're sampling every event
     1;
-
-  // To control bias from sampling, we clamp the maximum weight attributed to a single allocation sample. This avoids
-  // assigning a very large number to a sample, if for instance the dynamic sampling mechanism chose a really big interval.
+  // TODO: Signal in the profile that clamping happened?
   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 skipped samples in the profile, thus the data will account for all allocations.
-  if (weight < allocations_since_last_sample) {
-    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
   return Qnil;

ext/datadog_profiling_native_extension/collectors_stack.c

@@ -268,15 +268,19 @@ void sample_thread(
     recorder_instance,
     (ddog_prof_Slice_Location) {.ptr = buffer->locations, .len = captured_frames},
     values,
-    labels,
-    /* placeholder: */ false
+    labels
   );
 }
 
 // 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) {
@@ -298,6 +302,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;
 }
@@ -373,8 +380,7 @@ void record_placeholder_stack(
     recorder_instance,
     (ddog_prof_Slice_Location) {.ptr = buffer->locations, .len = 1},
     values,
-    labels,
-    /* placeholder: */ true
+    labels
   );
 }
 

ext/datadog_profiling_native_extension/collectors_thread_context.c

@@ -231,7 +231,6 @@ static void ddtrace_otel_trace_identifiers_for(
   VALUE active_span,
   VALUE otel_values
 );
-static VALUE _native_sample_skipped_allocation_samples(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE skipped_samples);
 
 void collectors_thread_context_init(VALUE profiling_module) {
   VALUE collectors_module = rb_define_module_under(profiling_module, "Collectors");
@@ -262,7 +261,6 @@ 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_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");
@@ -1402,33 +1400,3 @@ static void ddtrace_otel_trace_identifiers_for(
   *active_trace = current_trace;
   *numeric_span_id = resolved_numeric_span_id;
 }
-
-void thread_context_collector_sample_skipped_allocation_samples(VALUE self_instance, unsigned int skipped_samples) {
-  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[] = {
-    // Providing .num = 0 should not be needed but the tracer-2.7 docker image ships a buggy gcc that complains about this
-    {.key = DDOG_CHARSLICE_C("thread id"),        .str = DDOG_CHARSLICE_C("SA"),                .num = 0},
-    {.key = DDOG_CHARSLICE_C("thread name"),      .str = DDOG_CHARSLICE_C("Skipped Samples"),   .num = 0},
-    {.key = DDOG_CHARSLICE_C("allocation class"), .str = DDOG_CHARSLICE_C("(Skipped Samples)"), .num = 0},
-  };
-  ddog_prof_Slice_Label slice_labels = {.ptr = labels, .len = sizeof(labels) / sizeof(labels[0])};
-
-  record_placeholder_stack(
-    state->sampling_buffer,
-    state->recorder_instance,
-    (sample_values) {.alloc_samples = skipped_samples},
-    (sample_labels) {
-      .labels = slice_labels,
-      .state_label = NULL,
-      .end_timestamp_ns = 0, // For now we're not collecting timestamps for allocation events
-    },
-    DDOG_CHARSLICE_C("Skipped Samples")
-  );
-}
-
-static VALUE _native_sample_skipped_allocation_samples(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE skipped_samples) {
-  thread_context_collector_sample_skipped_allocation_samples(collector_instance, NUM2UINT(skipped_samples));
-  return Qtrue;
-}

ext/datadog_profiling_native_extension/collectors_thread_context.h

@@ -9,7 +9,6 @@ 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_skipped_allocation_samples(VALUE self_instance, unsigned int skipped_samples);
 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);

ext/datadog_profiling_native_extension/heap_recorder.c

@@ -166,12 +166,6 @@ struct heap_recorder {
     size_t objects_frozen;
   } stats_last_update;
 };
-
-struct end_heap_allocation_args {
-  struct heap_recorder *heap_recorder;
-  ddog_prof_Slice_Location locations;
-};
-
 static heap_record* get_or_create_heap_record(heap_recorder*, ddog_prof_Slice_Location);
 static void cleanup_heap_record_if_unused(heap_recorder*, heap_record*);
 static void on_committed_object_record_cleanup(heap_recorder *heap_recorder, object_record *record);
@@ -182,7 +176,6 @@ static int st_object_records_iterate(st_data_t, st_data_t, st_data_t);
 static int st_object_records_debug(st_data_t key, st_data_t value, st_data_t extra);
 static int update_object_record_entry(st_data_t*, st_data_t*, st_data_t, int);
 static void commit_recording(heap_recorder*, heap_record*, recording);
-static VALUE end_heap_allocation_recording(VALUE end_heap_allocation_args);
 
 // ==========================
 // Heap Recorder External API
@@ -347,28 +340,9 @@ void start_heap_allocation_recording(heap_recorder *heap_recorder, VALUE new_obj
   };
 }
 
-// end_heap_allocation_recording_with_rb_protect gets called while the stack_recorder is holding one of the profile
-// locks. To enable us to correctly unlock the profile on exception, we wrap the call to end_heap_allocation_recording
-// with an rb_protect.
-__attribute__((warn_unused_result))
-int end_heap_allocation_recording_with_rb_protect(struct heap_recorder *heap_recorder, ddog_prof_Slice_Location locations) {
-  int exception_state;
-  struct end_heap_allocation_args end_heap_allocation_args = {
-    .heap_recorder = heap_recorder,
-    .locations = locations,
-  };
-  rb_protect(end_heap_allocation_recording, (VALUE) &end_heap_allocation_args, &exception_state);
-  return exception_state;
-}
-
-static VALUE end_heap_allocation_recording(VALUE end_heap_allocation_args) {
-  struct end_heap_allocation_args *args = (struct end_heap_allocation_args *) end_heap_allocation_args;
-
-  struct heap_recorder *heap_recorder = args->heap_recorder;
-  ddog_prof_Slice_Location locations = args->locations;
-
+void end_heap_allocation_recording(struct heap_recorder *heap_recorder, ddog_prof_Slice_Location locations) {
   if (heap_recorder == NULL) {
-    return Qnil;
+    return;
   }
 
   recording active_recording = heap_recorder->active_recording;
@@ -382,16 +356,15 @@ static VALUE end_heap_allocation_recording(VALUE end_heap_allocation_args) {
   // data required for committing though.
   heap_recorder->active_recording = (recording) {0};
 
-  if (active_recording.object_record == &SKIPPED_RECORD) { // special marker when we decided to skip due to sampling
-    return Qnil;
+  if (active_recording.object_record == &SKIPPED_RECORD) {
+    // special marker when we decided to skip due to sampling
+    return;
   }
 
   heap_record *heap_record = get_or_create_heap_record(heap_recorder, locations);
 
   // And then commit the new allocation.
   commit_recording(heap_recorder, heap_record, active_recording);
-
-  return Qnil;
 }
 
 void heap_recorder_prepare_iteration(heap_recorder *heap_recorder) {

ext/datadog_profiling_native_extension/heap_recorder.h

@@ -114,9 +114,7 @@ void start_heap_allocation_recording(heap_recorder *heap_recorder, VALUE new_obj
 // @param locations The stacktrace representing the location of the allocation.
 //
 // WARN: It is illegal to call this without previously having called ::start_heap_allocation_recording.
-// WARN: This method rescues exceptions with `rb_protect`, returning the exception state integer for the caller to handle.
-__attribute__((warn_unused_result))
-int end_heap_allocation_recording_with_rb_protect(heap_recorder *heap_recorder, ddog_prof_Slice_Location locations);
+void end_heap_allocation_recording(heap_recorder *heap_recorder, ddog_prof_Slice_Location locations);
 
 // Update the heap recorder to reflect the latest state of the VM and prepare internal structures
 // for efficient iteration.

ext/datadog_profiling_native_extension/stack_recorder.c

@@ -592,7 +592,7 @@ static VALUE ruby_time_from(ddog_Timespec ddprof_time) {
   return rb_time_timespec_new(&time, utc);
 }
 
-void record_sample(VALUE recorder_instance, ddog_prof_Slice_Location locations, sample_values values, sample_labels labels, bool placeholder) {
+void record_sample(VALUE recorder_instance, ddog_prof_Slice_Location locations, sample_values values, sample_labels labels) {
   struct stack_recorder_state *state;
   TypedData_Get_Struct(recorder_instance, struct stack_recorder_state, &stack_recorder_typed_data, state);
 
@@ -612,20 +612,12 @@ void record_sample(VALUE recorder_instance, ddog_prof_Slice_Location locations,
   metric_values[position_for[ALLOC_SAMPLES_UNSCALED_VALUE_ID]] = values.alloc_samples_unscaled;
   metric_values[position_for[TIMELINE_VALUE_ID]]      = values.timeline_wall_time_ns;
 
-  if (!placeholder && values.alloc_samples > 0) {
+  if (values.alloc_samples != 0) {
     // If we got an allocation sample end the heap allocation recording to commit the heap sample.
     // FIXME: Heap sampling currently has to be done in 2 parts because the construction of locations is happening
     //        very late in the allocation-sampling path (which is shared with the cpu sampling path). This can
     //        be fixed with some refactoring but for now this leads to a less impactful change.
-    //
-    // NOTE: The heap recorder is allowed to raise exceptions if something's wrong. But we also need to handle it
-    // on this side to make sure we properly unlock the active slot mutex on our way out. Otherwise, this would
-    // later lead to deadlocks (since the active slot mutex is not expected to be locked forever).
-    int exception_state = end_heap_allocation_recording_with_rb_protect(state->heap_recorder, locations);
-    if (exception_state) {
-      sampler_unlock_active_profile(active_slot);
-      rb_jump_tag(exception_state);
-    }
+    end_heap_allocation_recording(state->heap_recorder, locations);
   }
 
   ddog_prof_Profile_Result result = ddog_prof_Profile_add(

ext/datadog_profiling_native_extension/stack_recorder.h

@@ -22,7 +22,7 @@ typedef struct sample_labels {
   int64_t end_timestamp_ns;
 } sample_labels;
 
-void record_sample(VALUE recorder_instance, ddog_prof_Slice_Location locations, sample_values values, sample_labels labels, bool placeholder);
+void record_sample(VALUE recorder_instance, ddog_prof_Slice_Location locations, sample_values values, sample_labels labels);
 void record_endpoint(VALUE recorder_instance, uint64_t local_root_span_id, ddog_CharSlice endpoint);
 void track_object(VALUE recorder_instance, VALUE new_object, unsigned int sample_weight, ddog_CharSlice *alloc_class);
 VALUE enforce_recorder_instance(VALUE object);