Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport "[PROF-9650] Enable endpoint profiling for Sidekiq and similar background job processors" to 1.x-stable #3619

Merged
merged 2 commits into from
May 7, 2024
Merged

Backport "[PROF-9650] Enable endpoint profiling for Sidekiq and similar background job processors" to 1.x-stable #3619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b7b5fa
Minor: Refactor `should_collect_resource` to receive root span
ivoanjo Apr 22, 2024
88679cc
[PROF-9650] Enable endpoint profiling for Sidekiq and similar backgro…
ivoanjo Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions ext/datadog_profiling_native_extension/collectors_thread_context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ static long thread_id_for(VALUE thread);
static VALUE _native_stats(VALUE self, VALUE collector_instance);
static VALUE _native_gc_tracking(VALUE self, VALUE collector_instance);
static void trace_identifiers_for(struct thread_context_collector_state *state, VALUE thread, struct trace_identifiers *trace_identifiers_result);
static bool should_collect_resource(VALUE root_span_type);
static bool should_collect_resource(VALUE root_span);
static VALUE _native_reset_after_fork(DDTRACE_UNUSED VALUE self, VALUE collector_instance);
static VALUE thread_list(struct thread_context_collector_state *state);
static VALUE _native_sample_allocation(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE sample_weight, VALUE new_object);
Expand Down Expand Up @@ -1146,10 +1146,7 @@ static void trace_identifiers_for(struct thread_context_collector_state *state,

trace_identifiers_result->valid = true;

if (!state->endpoint_collection_enabled) return;

VALUE root_span_type = rb_ivar_get(root_span, at_type_id /* @type */);
if (root_span_type == Qnil || !should_collect_resource(root_span_type)) return;
if (!state->endpoint_collection_enabled || !should_collect_resource(root_span)) return;

VALUE trace_resource = rb_ivar_get(active_trace, at_resource_id /* @resource */);
if (RB_TYPE_P(trace_resource, T_STRING)) {
Expand All @@ -1160,21 +1157,32 @@ static void trace_identifiers_for(struct thread_context_collector_state *state,
}
}

// We only collect the resource for spans of types:
// We opt-in to collecting the resource for spans of types:
// * 'web', for web requests
// * proxy', used by the rack integration with request_queuing: true (e.g. also represents a web request)
// * 'proxy', used by the rack integration with request_queuing: true (e.g. also represents a web request)
// * 'worker', used for sidekiq and similar background job processors
//
// NOTE: Currently we're only interested in HTTP service endpoints. Over time, this list may be expanded.
// Over time, this list may be expanded.
// Resources MUST NOT include personal identifiable information (PII); this should not be the case with
// ddtrace integrations, but worth mentioning just in case :)
static bool should_collect_resource(VALUE root_span_type) {
static bool should_collect_resource(VALUE root_span) {
VALUE root_span_type = rb_ivar_get(root_span, at_type_id /* @type */);
if (root_span_type == Qnil) return false;
ENFORCE_TYPE(root_span_type, T_STRING);

int root_span_type_length = RSTRING_LEN(root_span_type);
const char *root_span_type_value = StringValuePtr(root_span_type);

return (root_span_type_length == strlen("web") && (memcmp("web", root_span_type_value, strlen("web")) == 0)) ||
bool is_web_request =
(root_span_type_length == strlen("web") && (memcmp("web", root_span_type_value, strlen("web")) == 0)) ||
(root_span_type_length == strlen("proxy") && (memcmp("proxy", root_span_type_value, strlen("proxy")) == 0));

if (is_web_request) return true;

bool is_worker_request =
(root_span_type_length == strlen("worker") && (memcmp("worker", root_span_type_value, strlen("worker")) == 0));

return is_worker_request;
}

// After the Ruby VM forks, this method gets called in the child process to clean up any leftover state from the parent.
Expand Down
6 changes: 6 additions & 0 deletions spec/datadog/profiling/collectors/thread_context_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,12 @@ def another_way_of_calling_sample(profiler_overhead_stack_thread: Thread.current
it_behaves_like 'samples with code hotspots information'
end

context 'when local root span type is worker' do
let(:root_span_type) { 'worker' }

it_behaves_like 'samples with code hotspots information'
end

def self.otel_sdk_available?
begin
require 'opentelemetry/sdk'
Expand Down
Loading