Skip to content

Commit

Permalink
[PROF-9650] Enable endpoint profiling for Sidekiq and similar backgro…
Browse files Browse the repository at this point in the history
…und job processors

**What does this PR do?**

We have an opt-in list for which kinds of traces we collect the
resource from; this PR extends that list with the `worker` type
used by background consumers such as Sidekiq.

**Motivation:**

Enable profiling for Sidekiq and similar background job processors.

**Additional Notes:**

We also collect a few Sidekiq internal things (such as
`sidekiq.job_fetch`, `sidekiq.scheduled_poller_wait`,
`sidekiq.scheduled_push`, `sidekiq.stop` and `sidekiq.heartbeat`).

We discussed if it would make sense to hide them or not, but for now
we decided to still show them.

**How to test the change?**

This change includes test coverage; I also booted up sidekiq and tested
manually using the following script:

```ruby
 # Running redis:
 # * `docker-compose up redis`
 # Running server:
 # * `DD_SERVICE=sidekiq-testing DD_ENV=staging DD_PROFILING_ENABLED=true bundle exec sidekiq -r ./sidekiq.rb`
 # Running client:
 # * `DD_TRACE_ENABLED=false DD_TRACING_ENABLED=false be ddprofrb exec irb -r ./sidekiq.rb`
 # * `ThisIsASidekiqJob.perform_async("hello", 10)`

require "sidekiq"
require 'datadog'

Datadog.configure { |c| c.tracing.instrument :sidekiq }

class ThisIsASidekiqJob
  include Sidekiq::Job

  def perform(some_arg = "potato", how_long = 1)
    puts "Start of job!"
    sleep how_long
    puts "End of job!"
  end
end
```
  • Loading branch information
ivoanjo committed May 1, 2024
1 parent 4b7b5fa commit 88679cc
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
17 changes: 13 additions & 4 deletions ext/datadog_profiling_native_extension/collectors_thread_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -1157,11 +1157,12 @@ 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) {
Expand All @@ -1172,8 +1173,16 @@ static bool should_collect_resource(VALUE root_span) {
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
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

0 comments on commit 88679cc

Please sign in to comment.