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.

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

[PROF-7307] Add support for allocation samples to ThreadContext #2657

Merged
merged 4 commits into from
Mar 3, 2023

Commits on Mar 1, 2023

  1. Minor: Name Ruby main thread

    This comes in handy while debugging.
    ivoanjo committed Mar 1, 2023
    Configuration menu
    Copy the full SHA
    67b7fa8 View commit details
    Browse the repository at this point in the history
  2. [PROF-7307] Add support for allocation samples to ThreadContext

    **What does this PR do?**:
    
    This PR adds the `thread_context_collector_sample_allocation` API to
    the `ThreadContext` collector.
    
    This will be used to support allocation profiling (but right now
    it's not being called automatically when allocations happen, yet).
    
    **Motivation**:
    
    This is an incremental step towards supporting allocation profiling.
    
    I'm still working on the scaling/weighting strategy, and may
    end up adding that logic directly on the `ThreadContext`, haven't
    decided yet.
    
    **Additional Notes**:
    
    N/A
    
    **How to test the change?**:
    
    Change includes test coverage. End-to-end testing will come later
    once this is wired up in the `CpuAndWallTimeWorker`.
    ivoanjo committed Mar 1, 2023
    Configuration menu
    Copy the full SHA
    1b92ef1 View commit details
    Browse the repository at this point in the history

Commits on Mar 3, 2023

  1. Configuration menu
    Copy the full SHA
    1a3caef View commit details
    Browse the repository at this point in the history
  2. Tweak threshold on flaky profiler test

    This test failed in a flaky way in CI
    (<https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/9211/workflows/8431a38e-fa1d-4a93-a937-b8488b960a89/jobs/342577/tests#failed-test-0>).
    
    Interesting observation in the output:
    
    ```
    Failure/Error: expect(sample_count).to be >= 8, "sample_count: #{sample_count}, stats: #{stats}, debug_failures: #{debug_failures}"
      sample_count: 7, stats: {:trigger_sample_attempts=>8, :trigger_simulated_signal_delivery_attempts=>8, :simulated_signal_delivery=>8, :signal_handler_enqueued_sample=>8, :signal_handler_wrong_thread=>0, :sampled=>7, :sampling_time_ns_min=>122586, :sampling_time_ns_max=>2849779, :sampling_time_ns_total=>3664094, :sampling_time_ns_avg=>523442.0}, debug_failures: {:thread_list=>[#<Thread:0x000055df58fcafc0@Thread.main run>, #<Thread:0x000055df5b3d1f90 /app/spec/spec_helper.rb:241 sleep>, #<Thread:0x000055df5d235c48 /usr/local/bundle/gems/webrick-1.7.0/lib/webrick/utils.rb:158 sleep_forever>],
    ```
    
    There was at least one sample that was super slow
    (`sampling_time_ns_max` -- almost 3ms).
    
    Because of that, the profiler throttled sampling down (using the dynamic
    sampling rate component), and so did not take as many samples as we
    were expecting. This is by design, although it's really hard to know
    _why_ the profiler slowed down -- maybe the CI machine was busy?
    Maybe that leftover webrick thread leaked from another test was
    the culprit?
    
    Right now the dynamic sampling rate is by design not very configurable,
    so I've chosen to tweak the number of samples down yet again.
    
    If this last adjustment isn't enough, I'll bite the bullet and make it
    possible to disable the dynamic sampling rate (or tweak it), so as to
    not cause issues for this spec.
    
    (Like I mentioned in the comment -- I still think this test is worth
    the trouble, as it validates an important behavior of the profiler
    that otherwise may not get exercised in most validations we run --
    profiling while idle).
    ivoanjo committed Mar 3, 2023
    Configuration menu
    Copy the full SHA
    7d22e54 View commit details
    Browse the repository at this point in the history