Skip to content

Commit

Permalink
[PROF-7440] Report status of timeline feature as internal metadata
Browse files Browse the repository at this point in the history
**What does this PR do?**:

This PR adds support for reporting the status of the timeline feature
together with profiles, via the internal metadata field.

I ended up doing a bunch of refactoring to make it easier to adds new
reported fields as internal metadata; it may be easier to review this
PR commit-by-commit.

**Motivation**:

While this feature is off-by-default, having this internal metadata
makes it easier to spot profiles that have/don't have this data
without actually opening them up in the UX.

**Additional Notes**:

N/A

**How to test the change?**:

This change includes test coverage. It's possible internally at
Datadog to see this information in profile listings (ping me if you'd
like to know more).
  • Loading branch information
ivoanjo committed Aug 10, 2023
1 parent 13451a8 commit bd6b14f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
5 changes: 4 additions & 1 deletion lib/datadog/profiling/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
# NOTE: Please update the Initialization section of ProfilingDevelopment.md with any changes to this method

no_signals_workaround_enabled = false
timeline_enabled = false

if enable_new_profiler?(settings)
no_signals_workaround_enabled = no_signals_workaround_enabled?(settings)
timeline_enabled = settings.profiling.advanced.experimental_timeline_enabled

recorder = Datadog::Profiling::StackRecorder.new(
cpu_time_enabled: RUBY_PLATFORM.include?('linux'), # Only supported on Linux currently
Expand All @@ -81,7 +83,7 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
gc_profiling_enabled: enable_gc_profiling?(settings),
allocation_counting_enabled: settings.profiling.advanced.allocation_counting_enabled,
no_signals_workaround_enabled: no_signals_workaround_enabled,
timeline_enabled: settings.profiling.advanced.experimental_timeline_enabled,
timeline_enabled: timeline_enabled,
)
else
load_pprof_support
Expand All @@ -92,6 +94,7 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)

internal_metadata = {
no_signals_workaround_enabled: no_signals_workaround_enabled,
timeline_enabled: timeline_enabled,
}.freeze

exporter = build_profiler_exporter(settings, recorder, internal_metadata: internal_metadata)
Expand Down
24 changes: 19 additions & 5 deletions spec/datadog/profiling/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,16 @@
build_profiler_component
end

it 'sets up the Exporter with no_signals_workaround_enabled: false' do
it 'sets up the Exporter internal_metadata with no_signals_workaround_enabled: false, timeline_enabled: false' do
expect(Datadog::Profiling::Exporter)
.to receive(:new).with(hash_including(internal_metadata: { no_signals_workaround_enabled: false }))
.to receive(:new).with(
hash_including(
internal_metadata: {
no_signals_workaround_enabled: false,
timeline_enabled: false,
}
)
)

build_profiler_component
end
Expand Down Expand Up @@ -258,12 +265,19 @@
build_profiler_component
end

it 'sets up the Exporter with no_signals_workaround_enabled setting' do
it 'sets up the Exporter internal_metadata with no_signals_workaround_enabled and timeline_enabled settings' do
allow(Datadog::Profiling::Collectors::CpuAndWallTimeWorker).to receive(:new)

expect(described_class).to receive(:no_signals_workaround_enabled?).and_return(:no_signals_result)
expect(Datadog::Profiling::Exporter)
.to receive(:new).with(hash_including(internal_metadata: { no_signals_workaround_enabled: :no_signals_result }))
expect(settings.profiling.advanced).to receive(:experimental_timeline_enabled).and_return(:timeline_result)
expect(Datadog::Profiling::Exporter).to receive(:new).with(
hash_including(
internal_metadata: {
no_signals_workaround_enabled: :no_signals_result,
timeline_enabled: :timeline_result,
}
)
)

build_profiler_component
end
Expand Down

0 comments on commit bd6b14f

Please sign in to comment.