Skip to content

Conversation

@p-datadog
Copy link
Member

@p-datadog p-datadog commented Apr 10, 2025

What does this PR do?

Removes duplicated Core classes from Telemetry transport code.

Motivation:
As part of getting telemetry working for dynamic instrumentation, telemetry needs to work with UDS which is what shopist uses. Currently Telemetry only supports HTTP. Using the Core transport code will bring UDS transport to Telemetry.

Change log entry

Yes: Improve telemetry transport. Customers using datadog-ci-rb and webmock will need to update datadog-ci-rb to version 1.17.0 or later.

Additional Notes:

This is the first PR of a series. the next one will add a Core-based transport. This PR should be quick to review and conceptually it is standalone.

How to test the change?
Existing unit tests

I also ran datadog-ci-rb 1.15.0 tests against this PR.

@github-actions github-actions bot added the core Involves Datadog core libraries label Apr 10, 2025
@github-actions
Copy link

github-actions bot commented Apr 10, 2025

Thank you for updating Change log entry section 👏

Visited at: 2025-04-10 17:30:19 UTC

@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Apr 10, 2025

Datadog Report

Branch report: telemetry-transport
Commit report: 53466ae
Test service: dd-trace-rb

✅ 0 Failed, 20773 Passed, 1462 Skipped, 3m 46.65s Total Time

@pr-commenter
Copy link

pr-commenter bot commented Apr 10, 2025

Benchmarks

Benchmark execution time: 2025-04-29 18:20:35

Comparing candidate commit 53466ae in PR branch telemetry-transport with baseline commit 1b96ec8 in branch master.

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

scenario:tracing - Propagation - Trace Context

  • 🟩 throughput [+3811.871op/s; +3918.856op/s] or [+11.237%; +11.552%]

@p-datadog p-datadog changed the title Telemetry transport wip DEBUG-3700 Remove duplicated classes from Telemetry transport Apr 10, 2025
@p-datadog p-datadog force-pushed the telemetry-transport branch from f69c8e1 to b723f99 Compare April 10, 2025 17:30
@p-datadog p-datadog marked this pull request as ready for review April 10, 2025 17:30
@p-datadog p-datadog requested a review from a team as a code owner April 10, 2025 17:30
@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.72%. Comparing base (1b96ec8) to head (53466ae).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4575      +/-   ##
==========================================
- Coverage   97.73%   97.72%   -0.02%     
==========================================
  Files        1415     1409       -6     
  Lines       86247    85968     -279     
  Branches     4359     4352       -7     
==========================================
- Hits        84293    84010     -283     
- Misses       1954     1958       +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.

@p-datadog
Copy link
Member Author

datadog-ci-rb uses one of the removed clasess thusly:

          begin
            require "datadog/core/telemetry/http/adapters/net"

            # patch gem's telemetry transport layer to use Net::HTTP instead of WebMock's Net::HTTP
            Core::Telemetry::Http::Adapters::Net.include(CI::Transport::Adapters::TelemetryWebmockSafeAdapter)
          rescue => e
            Datadog.logger.warn("Failed to patch Datadog gem's telemetry layer: #{e}")
          end

LoadError is not derived from StandardError and will escape the rescue above.

I will put back the file with a dummy class definition.

@p-datadog p-datadog marked this pull request as draft April 11, 2025 12:57
@p-datadog p-datadog force-pushed the telemetry-transport branch 2 times, most recently from 71db6d6 to 36e7844 Compare April 28, 2025 15:07
@p-datadog p-datadog force-pushed the telemetry-transport branch from 36e7844 to 10fdb16 Compare April 29, 2025 17:53
@p-datadog p-datadog marked this pull request as ready for review April 30, 2025 13:16
@anmarchenko
Copy link
Member

Correction for the changelog entry as 1.16.0 is the current version:

Customers using datadog-ci-rb and webmock will need to update datadog-ci-rb to version 1.17.0 or later.

@anmarchenko
Copy link
Member

I tested this branch together with fixes from DataDog/datadog-ci-rb#309 using Test Optimization integration tests:

https://gitlab.ddbuild.io/DataDog/apm-reliability/test-environment/-/pipelines/63750300

The tests passed:
image

Copy link
Member

@Strech Strech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@p-datadog p-datadog merged commit 723fb81 into master May 5, 2025
444 checks passed
@p-datadog p-datadog deleted the telemetry-transport branch May 5, 2025 15:03
@github-actions github-actions bot added this to the 2.16.0 milestone May 5, 2025
p-datadog pushed a commit to p-datadog/dd-trace-rb that referenced this pull request May 7, 2025
* master:
  Avoid method redefinition warning when replacing time providers (DataDog#4613)
  DEBUG-3700 Remove duplicated classes from Telemetry transport  (DataDog#4575)
@Strech Strech mentioned this pull request May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Involves Datadog core libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants