Skip to content

[IGNORE] PR 6108 - #6139

Closed
vpellan wants to merge 7 commits into
masterfrom
community/pr-6108
Closed

[IGNORE] PR 6108#6139
vpellan wants to merge 7 commits into
masterfrom
community/pr-6108

Conversation

@vpellan

@vpellan vpellan commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

PLEASE IGNORE THIS PR

ollym and others added 7 commits July 26, 2026 17:27
…atsd client

Metrics::Client#default_statsd_client always passed an explicit host and
port to Datadog::Statsd.new, which bypasses dogstatsd-ruby's environment
resolution entirely: DD_DOGSTATSD_URL and DD_DOGSTATSD_SOCKET were ignored
and runtime metrics were silently sent to the UDP default (127.0.0.1:8125)
on UDS-only setups — for example pods instrumented by the Datadog Admission
Controller, which injects DD_DOGSTATSD_URL but not DD_AGENT_HOST.

When one of those variables is set, no explicit DD_AGENT_HOST or
DD_METRIC_AGENT_PORT is configured, and the installed dogstatsd-ruby
understands them (>= 5.6), construct the client without positional
arguments so dogstatsd-ruby resolves the transport itself. Explicit
host/port configuration keeps the existing behavior.

Fixes #6107

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review of PR #6108 noted the new public predicate statsd_transport_from_env?
had no entry in sig/datadog/core/metrics/client.rbs (its sibling private
helper dogstatsd_version is declared) and no @return tag. Declare it as
() -> bool and document the return in YARD. client.rb is in the Steepfile
ignore list, so this is signature/documentation consistency, not a Steep fix.
PR #6108 makes the runtime-metrics statsd client honor DD_DOGSTATSD_URL and
DD_DOGSTATSD_SOCKET but did not document them. Per the repo rule to record new
env vars in GettingStarted.md, add a paragraph in the runtime metrics section
covering the two variables, the >= 5.6 dogstatsd-ruby requirement, and the
DD_AGENT_HOST/DD_METRIC_AGENT_PORT precedence.
The `rake local_config_map:generate` template emits `require 'set'` with
single quotes (tasks/local_config_map.rake:69), which standard flags as
Style/StringLiterals (prefer double quotes). The generated file must be
run through `rake standard:fix` after regeneration; that step was skipped
when DD_DOGSTATSD_URL/DD_DOGSTATSD_SOCKET were added, so the standard/lint
CI job failed on line 3.

Change `require 'set'` to `require "set"`, matching what standard:fix
produces (and master's committed generated file).

Verified: `BUNDLE_GEMFILE=gemfiles/ruby-4.0.gemfile bundle exec rake
standard` on Ruby 4.0.6 (the CI config) exits 1 before this change with
the offense at supported_configurations.rb:3:9 and exits 0 after.
Address review comment: statsd_transport_from_env? is an internal helper
used only by default_statsd_client and should not expand the public surface
area of Datadog::Core::Metrics::Client.

- Moved the method (and its doc comment) into the existing private section
  in lib/datadog/core/metrics/client.rb
- Moved its signature under the private keyword in
  sig/datadog/core/metrics/client.rbs

Verified: rspec spec/datadog/core/metrics/client_spec.rb (85 examples, 0
failures); steep check (no type error); standardrb clean.
Address review comment: in the DD_DOGSTATSD_URL (>=5.6) and DD_DOGSTATSD_SOCKET
contexts, the before block referenced the metrics subject before setting the
Datadog::Statsd.new expectation. Because let(:options) is shadowed to
{single_thread: true} (no injected statsd), Client#initialize builds a real
statsd client, and with the URL/SOCKET env set that resolves a real env-based
(UDS) transport during test setup.

- Stub Datadog::Statsd.new with allow before building metrics so the
  initialization call returns the double instead of opening a real transport;
  the explicit expectation then observes only the #default_statsd_client call
- Stub statsd_client#close since @statsd is now the double closed by the
  after hook
- Applied to both flagged locations (URL >=5.6 branch and SOCKET context)

Verified: rspec spec/datadog/core/metrics/client_spec.rb (85 examples, 0
failures); standardrb clean.
@vpellan
vpellan requested review from a team as code owners July 31, 2026 14:17
@vpellan
vpellan marked this pull request as draft July 31, 2026 14:17
@dd-octo-sts dd-octo-sts Bot added the core Involves Datadog core libraries label Jul 31, 2026
@dd-octo-sts

dd-octo-sts Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

👋 Hey @DataDog/ruby-guild, please fill "Change log entry" section in the pull request description.

If changes need to be present in CHANGELOG.md you can state it this way

**Change log entry**

Yes. A brief summary to be placed into the CHANGELOG.md

(possible answers Yes/Yep/Yeah)

Or you can opt out like that

**Change log entry**

None.

(possible answers No/Nope/None)

Visited at: 2026-07-31 14:17:47 UTC

@vpellan

vpellan commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Closing as there's already #6117

@vpellan vpellan closed this Jul 31, 2026
@pr-commenter

pr-commenter Bot commented Jul 31, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-07-31 14:43:06

Comparing candidate commit 344745f in PR branch community/pr-6108 with baseline commit 65e0620 in branch master.

📊 Benchmarking dashboard

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

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:tracing - trace.to_digest - Continue

  • unstable throughput [-1370.126op/s; +1691.281op/s] or [-4.667%; +5.761%]

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.

3 participants