Skip to content

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Feb 21, 2025

What does this PR do?

This PR adds a new otel appraisal variant for profiling that uses opentelemetry-api >= 1.5 .

This is because 1.5 includes
open-telemetry/opentelemetry-ruby#1807 and we'll need to add special support for it.

Motivation:

I'm adding already the appraisal version (disabled for now) to avoid any future conflicts with changes to the Matrixfile.

Change log entry

None

Additional Notes:

N/A

How to test the change?

This new group is not yet used -- green CI is enough for this PR.

**What does this PR do?**

This PR adds a new otel appraisal variant for profiling that uses
`opentelemetry-api` >= 1.5 .

This is because 1.5 includes
open-telemetry/opentelemetry-ruby#1807
and we'll need to add special support for it.

**Motivation:**

I'm adding already the appraisal version (disabled for now) to
avoid any future conflicts with changes to the `Matrixfile`.

**Additional Notes:**

N/A

**How to test the change?**

This new group is not yet used -- green CI is enough for this PR.
@ivoanjo ivoanjo requested a review from a team as a code owner February 21, 2025 09:11
@pr-commenter
Copy link

pr-commenter bot commented Feb 21, 2025

Benchmarks

Benchmark execution time: 2025-02-21 09:53:54

Comparing candidate commit a298521 in PR branch ivoanjo/prof-11412-add-new-otel-api-variant with baseline commit 917747b in branch master.

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

@datadog-datadog-prod-us1
Copy link
Contributor

Datadog Report

Branch report: ivoanjo/prof-11412-add-new-otel-api-variant
Commit report: a298521
Test service: dd-trace-rb

✅ 0 Failed, 20604 Passed, 1375 Skipped, 3m 9.2s Total Time

@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.72%. Comparing base (917747b) to head (a298521).
Report is 892 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4421      +/-   ##
==========================================
- Coverage   97.73%   97.72%   -0.01%     
==========================================
  Files        1365     1365              
  Lines       83337    83336       -1     
  Branches     4219     4219              
==========================================
- Hits        81449    81444       -5     
- Misses       1888     1892       +4     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TonyCTHsu TonyCTHsu merged commit 29a6bd8 into master Feb 21, 2025
303 checks passed
@TonyCTHsu TonyCTHsu deleted the ivoanjo/prof-11412-add-new-otel-api-variant branch February 21, 2025 09:58
@github-actions github-actions bot added this to the 2.11.0 milestone Feb 21, 2025
ivoanjo added a commit that referenced this pull request Feb 21, 2025
**What does this PR do?**

This PR adds support for correlating profiling wih otel-api 1.5+.

Context storage was moved in
open-telemetry/opentelemetry-ruby#1807
and we needed to update the profiler to account for this.

**Motivation:**

Keep profiling + otel correlation working.

**Additional Notes:**

N/A

**How to test the change?**

In #4421 I had already bootstrapped the new appraisal groups needed
for testing this. This PR enables them, and our existing test
coverage will cover the new code path when used with otel-api 1.5+.
@ivoanjo ivoanjo added profiling Involves Datadog profiling otel OpenTelemetry-related changes labels Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

otel OpenTelemetry-related changes profiling Involves Datadog profiling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants