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

DEBUG-3535 use core transport for DI to support unix domain sockets #4426

Merged
merged 12 commits into from
Feb 25, 2025

Conversation

p-datadog
Copy link
Member

What does this PR do?

Changes DI transport code to use the core transport functionality to get unix domain socket support.

Motivation:
Shopist uses unix domain sockets, DI does not work there without them.

Change log entry
Yes: Dynamic instrumentation can now communicate with Datadog agent over Unix domain sockets

Additional Notes:
The code is copy-pasted heavily from existing remote and tracing transports. There are no changes outside of DI in this pull request. I would like to further DRY the transport code in a follow-up PR.

How to test the change?
Existing unit tests/CI + manual testing against shopist

@p-datadog p-datadog requested a review from a team as a code owner February 22, 2025 00:02
@p-datadog p-datadog added this to the 2.11.0 milestone Feb 22, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Feb 22, 2025

Datadog Report

Branch report: di-uds
Commit report: 4de2ea9
Test service: dd-trace-rb

✅ 0 Failed, 20418 Passed, 1369 Skipped, 3m 18.95s Total Time

@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2025

Codecov Report

Attention: Patch coverage is 91.00257% with 35 lines in your changes missing coverage. Please review.

Project coverage is 97.70%. Comparing base (4c0614d) to head (4de2ea9).

Files with missing lines Patch % Lines
lib/datadog/di/transport/http.rb 88.67% 6 Missing ⚠️
lib/datadog/di/transport/http/diagnostics.rb 86.66% 6 Missing ⚠️
lib/datadog/di/transport/http/input.rb 86.95% 6 Missing ⚠️
lib/datadog/di/transport/diagnostics.rb 81.48% 5 Missing ⚠️
lib/datadog/di/transport/input.rb 81.48% 5 Missing ⚠️
...tadog/di/integration/probe_notifier_worker_spec.rb 92.75% 5 Missing ⚠️
lib/datadog/di/transport/http/client.rb 88.23% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4426      +/-   ##
==========================================
- Coverage   97.71%   97.70%   -0.02%     
==========================================
  Files        1368     1374       +6     
  Lines       83438    83716     +278     
  Branches     4220     4236      +16     
==========================================
+ Hits        81533    81796     +263     
- Misses       1905     1920      +15     

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

@pr-commenter
Copy link

pr-commenter bot commented Feb 22, 2025

Benchmarks

Benchmark execution time: 2025-02-24 14:41:39

Comparing candidate commit 58df23e in PR branch di-uds with baseline commit 4c0614d in branch master.

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

scenario:profiler - stack collector

  • 🟩 throughput [+156.213op/s; +157.725op/s] or [+5.761%; +5.817%]

scenario:tracing - Propagation - Trace Context

  • 🟩 throughput [+3695.640op/s; +3813.261op/s] or [+10.781%; +11.124%]

@TonyCTHsu TonyCTHsu removed this from the 2.11.0 milestone Feb 24, 2025
@p-datadog p-datadog merged commit b2274aa into master Feb 25, 2025
499 checks passed
@p-datadog p-datadog deleted the di-uds branch February 25, 2025 13:22
@github-actions github-actions bot added this to the 2.12.0 milestone Feb 25, 2025
TonyCTHsu added a commit that referenced this pull request Feb 27, 2025
* [🤖] Update Latest Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/13477718964

* Remove force-executed tests and scenario from system-tests workflow after release 2.11.0

* Add AppSec RestClient instrumentation for SSRF detection

* Add type signatures for AppSec RestClient integration

* Fix type signatures for excon and faraday AppSec integrations

* Add tests for RestClient AppSec instrumentation

* Extract rest-client-latest appraisal

* [🤖] Lock Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/13455470094

* [🤖] Lock Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/13455581146

* Remove unneeded variable from RestClient SSRF integration test

* [🤖] Lock Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/13457056485

* Add rest-client gem to rake edge:update task

* [🤖] Lock Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/13496444973

* Rename RequestPatch to RequestSSRFDetectionPatch

* [🤖] Lock Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/13497053640

* Disable rubocop warning for RequestSSRFDetectionPatch

* [PROF-11151] Fix test-memory-leaks flaky behavior

**What does this PR do?**

This PR will hopefully fix almost [all of the the
flakiness](https://github.com/DataDog/dd-trace-rb/pulls?q=is%3Apr+asan)
we've seen from the test-memory-leaks GitHub workflow.

The previous "asan" builds we were using were built from ruby-head,
which means that any instability or early breakage in ruby-head would
make test-memory-leaks fail.

To fix this, I've worked with upstream to create these 3.4-asan builds:
these are Ruby builds that are built **from the latest 3.4 stable Ruby**
with asan. Thus any breakages we see in them, should also exist in
regular 3.4 builds.

**Motivation:**

With this change, the test-memory-leaks workflow becomes a lot more
valuable, since it's now never expected to fail.

Thus, any failures we see in it are worth investigating.

**Additional Notes:**

For context, asan (or ASan) is the "AddressSanitizer" tool, see
https://github.com/google/sanitizers/wiki/AddressSanitizer for more
details.

**How to test the change?**

Validate that the updated workflow is running on ruby-3.4.2 and that
it still has the diagnostic output from asan.

* DEBUG-3535 use core transport for DI to support unix domain sockets (#4426)

* Composite action

* Fix publish

* Fix doc step with branch

* Dependency inject logger (partial) (#4432)

* Add 2.12.0 to CHANGELOG.md

* Bump version 2.11.0 to 2.12.0

* Update lockfiles for release 2.12.0

---------

Co-authored-by: TonyCTHsu <16049123+TonyCTHsu@users.noreply.github.com>
Co-authored-by: Yury Lebedev <yury.lebedev@datadoghq.com>
Co-authored-by: y9v <1379701+y9v@users.noreply.github.com>
Co-authored-by: Yury Lebedev <lebedev.yurii@gmail.com>
Co-authored-by: Ivo Anjo <ivo.anjo@datadoghq.com>
Co-authored-by: Oleg Pudeyev <156273877+p-datadog@users.noreply.github.com>
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.

6 participants