[PROF-9963] Add integration testing for Ruby dir interruption monkey patch #41
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…patch **What does this PR do?** This PR adds a new test scenario, the `ruby_dir_interruption_patch`. **Motivation:** This scenario tests DataDog/dd-trace-rb#3720 in a very unique way: by loading the profiler, and then running the upstream Ruby test suite for the `Dir` class with our monkey patches applied. (For context, our "monkey patches" are a couple of Ruby modules which wrap the behavior of the standard library `Dir` class, and thus we need to be quite careful to make sure not to break any existing functionality.) **Additional Notes:** This test does not actually generate or assert on any profiles, it only checks that the profiler is running and the monkey patches were correctly applied prior to running the Ruby test suite. The `mspec_config.rb` file is also quite important, since it ensures that both the profiler as well as the monkey patches were correctly applied (e.g. to avoid the test passing trivially because e.g. the profiler actually didn't start). Because at time of writing DataDog/dd-trace-rb#3720 is not yet merged, this test is expected to fail with: ``` $ TEST_RUN_SECS=5 TEST_SCENARIOS="ruby_dir_interruption_patch" go test -v -run TestScenarios === RUN TestScenarios correctness_test.go:210: Considering only scenarios in ruby_dir_interruption_patch correctness_test.go:225: Extract base image from: scenarios/ruby_dir_interruption_patch/Dockerfile === RUN TestScenarios/scenarios/ruby_dir_interruption_patch correctness_test.go:244: Folder: scenarios/ruby_dir_interruption_patch correctness_test.go:245: Json file: scenarios/ruby_dir_interruption_patch/expected_profile.json correctness_test.go:246: Docker file: scenarios/ruby_dir_interruption_patch/Dockerfile correctness_test.go:248: Built test app with: test-app correctness_test.go:137: Running docker command with output /home/ivo.anjo/datadog/prof-correctness/data/ruby_dir_interruption_patch-1618380497 correctness_test.go:138: '[/bin/sh -c bundle exec ddprofrb exec ruby mspec-master/bin/mspec-run --config mspec_config.rb spec-master/core/dir/]' correctness_test.go:151: Error running the test docker run -v /home/ivo.anjo/datadog/prof-correctness/data/ruby_dir_interruption_patch-1618380497:/app/data:rw -u 1000:1000 --security-opt seccomp=unconfined -e EXECUTION_TIME_SEC=5 -e DD_SERVICE=prof-correctness-scenarios/ruby_dir_interruption_patch test-app:latest - /app/mspec_config.rb:3:in `<top (required)>': uninitialized constant Datadog::Profiling::Ext::DirInstanceMonkeyPatches (NameError) if Dir.ancestors.first == Datadog::Profiling::Ext::DirInstanceMonkeyPatches && ^^^^^^^^^^^^^^^^^^^^^^^^^^ from /app/mspec-master/lib/mspec/utils/script.rb:91:in `load' from /app/mspec-master/lib/mspec/utils/script.rb:91:in `block (2 levels) in try_load' from /app/mspec-master/lib/mspec/utils/script.rb:86:in `each' from /app/mspec-master/lib/mspec/utils/script.rb:86:in `block in try_load' from /app/mspec-master/lib/mspec/utils/script.rb:85:in `each' from /app/mspec-master/lib/mspec/utils/script.rb:85:in `try_load' from /app/mspec-master/lib/mspec/utils/script.rb:102:in `load' from /app/mspec-master/lib/mspec/commands/mspec-run.rb:34:in `block in options' from /app/mspec-master/lib/mspec/utils/options.rb:111:in `process' from /app/mspec-master/lib/mspec/utils/options.rb:143:in `parse' from /app/mspec-master/lib/mspec/commands/mspec-run.rb:76:in `options' from /app/mspec-master/lib/mspec/utils/script.rb:286:in `main' from mspec-master/bin/mspec-run:7:in `<main>' I, [2024-06-19T08:58:37.927905 #7] INFO -- datadog: [datadog] DATADOG CONFIGURATION - CORE - {"date":"2024-06-19T08:58:37Z","os_name":"x86_64-pc-linux","version":"2.1.0","lang":"ruby","lang_version":"3.3.3","env":null,"service":"prof-correctness-scenarios/ruby_dir_interruption_patch","dd_version":null,"debug":false,"tags":null,"runtime_metrics_enabled":false,"vm":"ruby-3.3.3","health_metrics_enabled":false,"profiling_enabled":true} --- FAIL: TestScenarios (19.35s) ``` ...e.g. the monkey patches are missing. For local testing, you can tweak the `gems.rb` to pick up the new branch: ```ruby gem 'datadog', git: 'https://github.com/datadog/dd-trace-rb.git', branch: 'ivoanjo/prof-9342-dir-interruption-workaround' ``` and you'll see the test passing. Furthermore, I've created a branch where I introduce a bug in `Dir.home`: ```ruby gem 'datadog', git: 'https://github.com/datadog/dd-trace-rb.git', branch: 'ivoanjo/prof-9342-dir-interruption-workaround-deleteme' ``` and here's the Ruby test suite flagging this behavior issue: ``` $ TEST_RUN_SECS=5 TEST_SCENARIOS="ruby_dir_interruption_patch" go test -v -run TestScenarios === RUN TestScenarios correctness_test.go:210: Considering only scenarios in ruby_dir_interruption_patch correctness_test.go:225: Extract base image from: scenarios/ruby_dir_interruption_patch/Dockerfile === RUN TestScenarios/scenarios/ruby_dir_interruption_patch correctness_test.go:244: Folder: scenarios/ruby_dir_interruption_patch correctness_test.go:245: Json file: scenarios/ruby_dir_interruption_patch/expected_profile.json correctness_test.go:246: Docker file: scenarios/ruby_dir_interruption_patch/Dockerfile correctness_test.go:248: Built test app with: test-app correctness_test.go:137: Running docker command with output /home/ivo.anjo/datadog/prof-correctness/data/ruby_dir_interruption_patch-1872402346 correctness_test.go:138: '[/bin/sh -c bundle exec ddprofrb exec ruby mspec-master/bin/mspec-run --config mspec_config.rb spec-master/core/dir/]' correctness_test.go:151: Error running the test docker run -v /home/ivo.anjo/datadog/prof-correctness/data/ruby_dir_interruption_patch-1872402346:/app/data:rw -u 1000:1000 --security-opt seccomp=unconfined -e EXECUTION_TIME_SEC=5 -e DD_SERVICE=prof-correctness-scenarios/ruby_dir_interruption_patch test-app:latest - I, [2024-06-19T09:04:55.521052 #7] INFO -- datadog: [datadog] DATADOG CONFIGURATION - CORE - {"date":"2024-06-19T09:04:55Z","os_name":"x86_64-pc-linux","version":"2.1.0","lang":"ruby","lang_version":"3.3.3","env":null,"service":"prof-correctness-scenarios/ruby_dir_interruption_patch","dd_version":null,"debug":false,"tags":null,"runtime_metrics_enabled":false,"vm":"ruby-3.3.3","health_metrics_enabled":false,"profiling_enabled":true} Dir interruption patch is present! ruby 3.3.3 (2024-06-12 revision f1c7b6f435) [x86_64-linux] ...................................................................................................................................................................................................................................................................FFEEFFEF............................................................... 1) Dir.home when called without arguments returns the current user's home directory, reading $HOME first FAILED Expected nil == "/rubyspec_home" to be truthy but was false /app/spec-master/core/dir/home_spec.rb:16:in `block (3 levels) in <top (required)>' /app/spec-master/core/dir/home_spec.rb:4:in `<top (required)>' 2) Dir.home when called without arguments returns a non-frozen string FAILED Expected nil.frozen? to be falsy but was true /app/spec-master/core/dir/home_spec.rb:20:in `block (3 levels) in <top (required)>' /app/spec-master/core/dir/home_spec.rb:4:in `<top (required)>' (...etc...) 32 files, 330 examples, 440 expectations, 5 failures, 3 errors, 0 tagged --- FAIL: TestScenarios (19.42s) ```
|
There seems to be a failure: --- FAIL: TestScenarios/scenarios/ruby_dir_interruption_patch (26.05s) |
Yup! This is expected -- the fix has not been merged to master AND prof-correctness is pulling from master for testing. I'll re-trigger CI once the fix is merged. |
|
@r1viollet I've merged DataDog/dd-trace-rb#3720 and added one tiny extra workaround (af8650b) and CI is now very green so this is ready for review ^^ |
r1viollet
reviewed
Jul 2, 2024
| { | ||
| "test_name": "ruby_dir_interruption_patch", | ||
| "stacks": [], | ||
| "note": "This test has no stacks, as it's not expected to emit profiles -- we're only testing that the mspec run is successful even when the profiler monkey patches are available." |
There was a problem hiding this comment.
do we catch the failure correctly if this does not run ? I think absence of pprof is enough to fail.
There was a problem hiding this comment.
Yes -- you can actually see it failing in https://github.com/DataDog/prof-correctness/actions/runs/9758283260/job/26932502637 and then I added a ugly workaround in af8650b .
|
Thanks for the review, merging away! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR adds a new test scenario, the
ruby_dir_interruption_patch.Motivation:
This scenario tests DataDog/dd-trace-rb#3720 in a very unique way: by loading the profiler, and then running the upstream Ruby test suite for the
Dirclass with our monkey patches applied.(For context, our "monkey patches" are a couple of Ruby modules which wrap the behavior of the standard library
Dirclass, and thus we need to be quite careful to make sure not to break any existing functionality.)Additional Notes:
This test does not actually generate or assert on any profiles, it only checks that the profiler is running and the monkey patches were correctly applied prior to running the Ruby test suite.
The
mspec_config.rbfile is also quite important, since it ensures that both the profiler as well as the monkey patches were correctly applied (e.g. to avoid the test passing trivially because e.g. the profiler actually didn't start).Because at time of writing
DataDog/dd-trace-rb#3720 is not yet merged, this test is expected to fail with:
...e.g. the monkey patches are missing.
For local testing, you can tweak the
gems.rbto pick up the new branch:and you'll see the test passing.
Furthermore, I've created a branch where I introduce a bug in
Dir.home:and here's the Ruby test suite flagging this behavior issue: