Skip to content

Commit

Permalink
Merge branch 'master' into forwardport/update-libddwaf
Browse files Browse the repository at this point in the history
  • Loading branch information
lloeki committed Oct 18, 2022
2 parents 6be3263 + 56d764a commit 78df080
Show file tree
Hide file tree
Showing 369 changed files with 3,119 additions and 2,664 deletions.
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ AllCops:
- 'lib/datadog/**/vendor/**/*.rb'
- 'integration/apps/*/bin/*'
- 'integration/apps/*/Gemfile'
- 'integration/apps/hanami/Rakefile'
- 'lib/datadog/profiling/pprof/pprof_pb.rb'
- 'spec/**/**/interesting_backtrace_helper.rb' # This file needs quite a few bizarre code patterns by design
NewCops: disable # Don't allow new cops to be enabled implicitly.
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ gem 'builder'
gem 'climate_control', '~> 0.2.0'
# Leave it open as we also have it as an integration and want Appraisal to control the version under test.
gem 'concurrent-ruby'
gem 'extlz4', '~> 0.3', '>= 0.3.3' if RUBY_PLATFORM != 'java' # Used to test lz4 compression done by libdatadog
gem 'json-schema', '< 3' # V3 only works with 2.5+
gem 'memory_profiler', '~> 0.9'
gem 'os', '~> 1.1'
Expand Down
1 change: 1 addition & 0 deletions LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
Component,Origin,License,Copyright
lib/datadog/core/vendor/multipart-post,https://github.com/socketry/multipart-post,MIT,"Copyright (c) 2007-2013 Nick Sieger."
lib/datadog/tracing/contrib/active_record/vendor,https://github.com/rails/rails/,MIT,"Copyright (c) 2005-2018 David Heinemeier Hansson"
lib/datadog/tracing/contrib/utils/quantization/http.rb,https://github.com/ruby/uri,BSD-2-Clause,"Copyright (C) 1993-2013 Yukihiro Matsumoto. All rights reserved."
ext/ddtrace_profiling_native_extension/private_vm_api_access,https://github.com/ruby/ruby,BSD-2-Clause,"Copyright (C) 1993-2013 Yukihiro Matsumoto. All rights reserved."
msgpack,https://rubygems.org/gems/msgpack,Apache-2.0,"Copyright (c) 2008-2015 Sadayuki Furuhashi"
debase-ruby_core_source,https://rubygems.org/gems/debase-ruby_core_source,MIT for gem and BSD-2-Clause for Ruby sources,"Copyright (c) 2012 Gabriel Horner. Files from Ruby sources are Copyright (C) 1993-2013 Yukihiro Matsumoto. All rights reserved."
1 change: 1 addition & 0 deletions benchmarks/profiler_submission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

require 'benchmark/ips'
require 'ddtrace'
require 'datadog/core/utils/compression'
require 'pry'
require 'digest'
require_relative 'dogstatsd_reporter'
Expand Down
2 changes: 1 addition & 1 deletion ddtrace.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Gem::Specification.new do |spec|
spec.add_dependency 'libddwaf', '~> 1.5.1.0.0'

# Used by profiling (and possibly others in the future)
spec.add_dependency 'libdatadog', '~> 0.7.0.1.1'
spec.add_dependency 'libdatadog', '~> 0.9.0.1.0'

spec.extensions = ['ext/ddtrace_profiling_native_extension/extconf.rb', 'ext/ddtrace_profiling_loader/extconf.rb']
end
4 changes: 2 additions & 2 deletions docs/UpgradeGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ end
| General | Removed | `Datadog.configure` can no longer be called without a block | Remove uses of `Datadog.configure` without a block. |
| CI API | Changed | `DD_TRACE_CI_MODE_ENABLED` environment variable is now `DD_TRACE_CI_ENABLED` | Use `DD_TRACE_CI_ENABLED` instead. |
| Configuration | Changed | Many settings have been namespaced under specific categories | Update your configuration to these [new settings](#1.0-configuration-settings) where appropriate. |
| Configuration | Removed | `Datadog.configure(client, options)` | Use `Datadog::Tracing.configure_onto(client, options)` instead. |
| Configuration | Removed | `Datadog.configure(client, options)` | Use `Datadog.configure_onto(client, options)` instead. |
| Configuration | Removed | `DD_#{integration}_ANALYTICS_ENABLED` and `DD_#{integration}_ANALYTICS_SAMPLE_RATE` environment variables | Use `DD_TRACE_#{integration}_ANALYTICS_ENABLED` and `DD_TRACE_#{integration}_ANALYTICS_SAMPLE_RATE` instead. |
| Configuration | Removed | `DD_PROPAGATION_INJECT_STYLE` and `DD_PROPAGATION_EXTRACT_STYLE` environment variables | Use `DD_PROPAGATION_STYLE_INJECT` and `DD_PROPAGATION_STYLE_EXTRACT` instead. |
| Integrations | Changed | `-` in HTTP header tag names are kept, and no longer replaced with `_` | For example: `http.response.headers.content_type` is changed to `http.response.headers.content-type`. |
Expand All @@ -719,7 +719,7 @@ end
| Tracing API | Removed | `child_of:` option from `Tracer#trace` | Not supported. |
| Tracing API | Removed | `Datadog.tracer` | Use methods in `Datadog::Tracing` instead. |
| Tracing API | Removed | `Pin.get_from(client)` | Use `Datadog::Tracing.configure_for(client)` instead. |
| Tracing API | Removed | `Pin.new(service, config: { option: value }).onto(client)` | Use `Datadog::Tracing.configure_onto(client, service_name: service, option: value)` instead. |
| Tracing API | Removed | `Pin.new(service, config: { option: value }).onto(client)` | Use `Datadog.configure_onto(client, service_name: service, option: value)` instead. |
| Tracing API | Removed | `Pipeline.before_flush` | Use `Datadog::Tracing.before_flush` instead. |
| Tracing API | Removed | `SpanOperation#context` | Use `Datadog::Tracing.active_trace` instead. |
| Tracing API | Removed | `SpanOperation#parent`/`SpanOperation#parent=` | Not supported. |
Expand Down
40 changes: 38 additions & 2 deletions ext/ddtrace_profiling_native_extension/NativeExtensionDesign.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Profiling Native Extension Design

The profiling native extension is used to:

1. Implement features which are expensive (in terms of resources) or otherwise impossible to implement using Ruby code.
2. Bridge between Ruby-specific profiling features and [`libdatadog`](https://github.com/DataDog/libdatadog), a Rust-based
library with common profiling functionality.
Expand All @@ -20,8 +21,8 @@ and disabling the extension will disable profiling.

## Who is this page for?

This documentation is intended to be used by dd-trace-rb developers. Please see the `docs/` folder for user-level
documentation.
**This documentation is intended to be used by dd-trace-rb developers. Please see the `docs/` folder for user-level
documentation.**

## Must not block or break users that cannot use it

Expand Down Expand Up @@ -118,3 +119,38 @@ Note that `pthread_getcpuclockid()` is not available on macOS (nor, obviously, o
is currently Linux-specific. Thus, in the <clock_id_from_pthread.c> file we implement the feature for supported Ruby
setups but if something is missing we instead compile in <clock_id_noop.c> that includes a no-op implementation of the
feature.

## Fork-safety

It's common for Ruby applications to create child processes via the use of `fork`. For instance, this strategy is used
by the puma webserver and the resque job processing tool.

Thus, the profiler needs to be designed to take this into account. I'll call out two important parts of this design:

1. Automatically propagate profiler to child processes. To make onboarding easier, we monkey patch the Ruby `fork` APIs
so that the profiler is automatically restarted in child processes. This way, the user only needs to start profiling at
the beginning of their application, and automatically forks are profiled as well.

2. The profiler must ensure correctness and stability even if the application forks. There must be no impact on the
application or incorrect data generated.

### Fork-safety for libdatadog

Since libdatadog is built in native code (Rust), special care needs to be take to consider how we're using it and how
it can be affected by the use of `fork`.

* Profile-related APIs: `Profile_new` and `Profile_add` and `Profile_free` are only called with the Ruby Global VM Lock
being held. Thus, if Ruby APIs are being used for fork, this prevents any concurrency between profile mutation and
forking, because if we’re holding the lock, then no other thread can call into the fork APIs.
(Calling libc `fork()` directly from a native extension is possible but would break the VM as well, since it does need
to do some of its own work when forking happens, so we’ll ignore that one)

* Exporter-related APIs: Explicitly to make sure we had no issues with forking, we create a new `CancellationToken_new`
and `ProfileExporterV3_new` for every report. We do release the Global VM Lock during exporting, so it's possible for
forking and exporting to be concurrent.

Both the CancellationToken and ProfileExporter are only referenced on the stack of the thread doing the exporting, so
they will not be reused in the child process after the fork. In the worst case, if a report is concurrent with a fork,
then it's possible a small amount of memory will not be cleaned up in the child process.

Because there is no leftover undefined state, we guarantee correctness for the exporter APIs.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct cpu_and_wall_time_collector_state {
// Tracks per-thread state
struct per_thread_context {
char thread_id[THREAD_ID_LIMIT_CHARS];
ddprof_ffi_CharSlice thread_id_char_slice;
ddog_CharSlice thread_id_char_slice;
thread_cpu_time_id thread_cpu_time_id;
long cpu_time_at_previous_sample_ns; // Can be INVALID_TIME until initialized or if getting it fails for another reason
long wall_time_at_previous_sample_ns; // Can be INVALID_TIME until initialized
Expand Down Expand Up @@ -209,12 +209,12 @@ VALUE cpu_and_wall_time_collector_sample(VALUE self_instance) {
bool have_thread_name = thread_name != Qnil;

int label_count = 1 + (have_thread_name ? 1 : 0);
ddprof_ffi_Label labels[label_count];
ddog_Label labels[label_count];

labels[0] = (ddprof_ffi_Label) {.key = DDPROF_FFI_CHARSLICE_C("thread id"), .str = thread_context->thread_id_char_slice};
labels[0] = (ddog_Label) {.key = DDOG_CHARSLICE_C("thread id"), .str = thread_context->thread_id_char_slice};
if (have_thread_name) {
labels[1] = (ddprof_ffi_Label) {
.key = DDPROF_FFI_CHARSLICE_C("thread name"),
labels[1] = (ddog_Label) {
.key = DDOG_CHARSLICE_C("thread name"),
.str = char_slice_from_ruby_string(thread_name)
};
}
Expand All @@ -223,8 +223,8 @@ VALUE cpu_and_wall_time_collector_sample(VALUE self_instance) {
thread,
state->sampling_buffer,
state->recorder_instance,
(ddprof_ffi_Slice_i64) {.ptr = metric_values, .len = ENABLED_VALUE_TYPES_COUNT},
(ddprof_ffi_Slice_label) {.ptr = labels, .len = label_count}
(ddog_Slice_i64) {.ptr = metric_values, .len = ENABLED_VALUE_TYPES_COUNT},
(ddog_Slice_label) {.ptr = labels, .len = label_count}
);
}

Expand Down Expand Up @@ -261,7 +261,7 @@ static struct per_thread_context *get_or_create_context_for(VALUE thread, struct

static void initialize_context(VALUE thread, struct per_thread_context *thread_context) {
snprintf(thread_context->thread_id, THREAD_ID_LIMIT_CHARS, "%ld", thread_id_for(thread));
thread_context->thread_id_char_slice = (ddprof_ffi_CharSlice) {.ptr = thread_context->thread_id, .len = strlen(thread_context->thread_id)};
thread_context->thread_id_char_slice = (ddog_CharSlice) {.ptr = thread_context->thread_id, .len = strlen(thread_context->thread_id)};

thread_context->thread_cpu_time_id = thread_cpu_time_id_for(thread);

Expand Down
Loading

0 comments on commit 78df080

Please sign in to comment.