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-11203] Bump minimum version of datadog-ruby_core_source to 3.4 #4323

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Jan 27, 2025

What does this PR do?

This PR bumps the minimum version of the datadog-ruby_core_source gem to 3.4

Motivation:

This bump ensures that any customers upgrading dd-trace-rb also pick up the latest version of datadog-ruby_core_source.

Change log entry

Yes. Bump minimum version of datadog-ruby_core_source to 3.4

Additional Notes:

N/A

How to test the change?

Green CI means all ok :)

**What does this PR do?**

This PR bumps the minimum version of the `datadog-ruby_core_source`
gem to 3.4.0.

**Motivation:**

This bump ensures that any customers upgrading dd-trace-rb also pick
up the latest version of `datadog-ruby_core_source`.

**Additional Notes:**

N/A

**How to test the change?**

Green CI means all ok :)
@ivoanjo ivoanjo requested a review from a team as a code owner January 27, 2025 12:05
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Jan 27, 2025

Datadog Report

Branch report: ivoanjo/prof-11203-bump-headers
Commit report: 42edee9
Test service: dd-trace-rb

✅ 0 Failed, 22114 Passed, 1476 Skipped, 5m 25.23s Total Time

@ivoanjo
Copy link
Member Author

ivoanjo commented Jan 27, 2025

Humm, interesting, a new profiler flaky spec. Investigating...

@pr-commenter
Copy link

pr-commenter bot commented Jan 27, 2025

Benchmarks

Benchmark execution time: 2025-01-27 15:14:46

Comparing candidate commit 42edee9 in PR branch ivoanjo/prof-11203-bump-headers with baseline commit e3cec5f in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 2 unstable metrics.

@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.73%. Comparing base (e3cec5f) to head (42edee9).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4323   +/-   ##
=======================================
  Coverage   97.73%   97.73%           
=======================================
  Files        1367     1367           
  Lines       82864    82864           
  Branches     4209     4209           
=======================================
+ Hits        80985    80991    +6     
+ Misses       1879     1873    -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

In CI we ran into a flaky failure from this spec:
<https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/18799/workflows/001aa960-917a-4036-83e1-9965bdf5ff34/jobs/659509>

```
Failures:

  1) Datadog::Profiling::Collectors::ThreadContext#sample timeline support when timeline is enabled when thread starts Waiting for GVL cpu-time behavior on Linux assigns all the cpu-time to the sample before Waiting for GVL started
     Failure/Error: expect(first_sample.values.fetch(:"cpu-time")).to be 12345

       expected #<Integer:24691> => 12345
            got #<Integer:44213> => 22106

       Compared using equal?, which compares object identity,
       but expected and actual are not the same object. Use
       `expect(actual).to eq(expected)` if you don't care about
       object identity in this example.
     # ./spec/datadog/profiling/collectors/thread_context_spec.rb:1201:in `block (7 levels) in <top (required)>'
     # ./spec/spec_helper.rb:238:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:123:in `block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
     # /usr/local/bundle/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block (2 levels) in <top (required)>'

Finished in 34.85 seconds (files took 1.67 seconds to load)
767 examples, 1 failure, 20 pending

Failed examples:

rspec ./spec/datadog/profiling/collectors/thread_context_spec.rb:1196 # Datadog::Profiling::Collectors::ThreadContext#sample timeline support when timeline is enabled when thread starts Waiting for GVL cpu-time behavior on Linux assigns all the cpu-time to the sample before Waiting for GVL started
```

This means that we observed more cpu-time than expected for the
target thread. In theory, the thread should be completely stopped
(since it's sleeping), so it should not have any cpu-time.

In practice, I can imagine that it's not impossible to make the
thread wake up spuriously -- to handle a signal, or it was woken
up on some internal Ruby lock -- just for it to go back to sleep.

The intention of this spec is to validate that all cpu-time
is assigned to the first sample (before the waiting for GVL
started), and none to the second, so I think we're OK with
not coupling to an exact amount of cpu-time to expect.
@ivoanjo ivoanjo requested a review from a team as a code owner January 27, 2025 14:34
@ivoanjo ivoanjo merged commit 3b789a0 into master Jan 27, 2025
388 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/prof-11203-bump-headers branch January 27, 2025 15:25
@github-actions github-actions bot added this to the 2.10.0 milestone Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants