Skip to content

Misc CI fixes - #4219

Merged
cataphract merged 7 commits into
masterfrom
glopes/ci-misc
Sep 30, 2026
Merged

cataphract merged 7 commits into
masterfrom
glopes/ci-misc

Conversation

@cataphract

@cataphract cataphract commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 68.26% (+0.00%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 6e29d76 | Docs | View more details | Give us feedback!

@pr-commenter

pr-commenter Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Benchmarks [ profiler ]

Benchmark execution time: 2026-09-23 15:37:21

Comparing candidate commit 66f1a60 in PR branch glopes/ci-misc with baseline commit 7646f7f in branch glopes/ci-misc-no-libdatadog.

Found 0 performance improvements and 3 performance regressions! Performance is the same for 24 metrics, 9 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 ----------------------------------'

scenario:php-profiler-timeline-memory-control

  • 🟥 cpu_user_time [+31.797ms; +41.675ms] or [+4.907%; +6.431%]
  • 🟥 execution_time [+33.530ms; +37.950ms] or [+4.810%; +5.444%]

scenario:php-profiler-timeline-memory-with-profiler

  • 🟥 execution_time [+26.309ms; +46.228ms] or [+2.258%; +3.967%]

@pr-commenter

pr-commenter Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Benchmarks [ tracer ]

Benchmark execution time: 2026-09-30 11:58:23

Comparing candidate commit 6e29d76 in PR branch glopes/ci-misc with baseline commit cc71bd6 in branch master.

📊 Benchmarking dashboard

Found 6 performance improvements and 0 performance regressions! Performance is the same for 188 metrics, 0 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 ----------------------------------'

scenario:SpanBench/benchDatadogAPI

  • 🟩 execution_time [-5.816µs; -4.222µs] or [-7.936%; -5.761%]

scenario:SpanBench/benchDatadogAPI-opcache

  • 🟩 execution_time [-5.022µs; -3.611µs] or [-6.964%; -5.007%]

scenario:TraceFlushBench/benchFlushTrace

  • 🟩 execution_time [-72.415µs; -64.285µs] or [-20.373%; -18.085%]

scenario:TraceFlushBench/benchFlushTrace-opcache

  • 🟩 execution_time [-75.446µs; -62.454µs] or [-19.146%; -15.849%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟩 execution_time [-88.999µs; -81.601µs] or [-20.462%; -18.761%]

scenario:TraceSerializationBench/benchSerializeTrace-opcache

  • 🟩 execution_time [-94.790µs; -83.810µs] or [-23.341%; -20.638%]

@cataphract
cataphract force-pushed the glopes/ci-misc branch 3 times, most recently from 99ed328 to f1ce464 Compare September 21, 2026 13:48
Comment thread libdatadog
@cataphract
cataphract marked this pull request as ready for review September 21, 2026 19:11
@cataphract
cataphract requested review from a team as code owners September 21, 2026 19:11
@cataphract
cataphract requested review from LobeTia, danyal002 and vjfridge and removed request for a team September 21, 2026 19:11
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-21T19:18:56.126842Z 9919b0d Draft marked ready
🔒 Security Review ✅ Completed 2026-09-21T19:22:16.724854Z 9919b0d Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Comment thread ext/remote_config.c Outdated
Comment thread .gitlab/generate-shared.php Outdated
@bwoebi

bwoebi commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

build: update libdatadog sidecar deadlock fix = DataDog/libdatadog@0e8c8db...d3be017

Seems like the wrong fix to me, can we instead hold the lock the other way round on telemetry shutdown? TelemetryCachedEntry anyway is Arc<Mutex<Option<TelemetryCachedClient>>> - so we can just take() the Option, and then drop the lock. Then acquire the TelemetryCachedSet lock and check whether that entry is still None, and if yes, drop it.

Saves us from duplicating the map every time we want to flush.

@cataphract
cataphract force-pushed the glopes/ci-misc branch 4 times, most recently from c0ec079 to 7a68e85 Compare September 22, 2026 16:43
@cataphract
cataphract changed the base branch from master to glopes/ci-misc-no-libdatadog September 23, 2026 15:08
Base automatically changed from glopes/ci-misc-no-libdatadog to master September 24, 2026 12:04
@cataphract
cataphract changed the base branch from master to glopes/ssi-shutdown-crash September 24, 2026 13:27
@cataphract
cataphract force-pushed the glopes/ci-misc branch 7 times, most recently from cd4706e to 575f436 Compare September 25, 2026 13:36
Base automatically changed from glopes/ssi-shutdown-crash to master September 25, 2026 23:52
gh-worker-dd-mergequeue-cf854d Bot pushed a commit to DataDog/libdatadog that referenced this pull request Sep 28, 2026
# What does this PR do?

1. Fix a verified hang on Windows when sending data to a full pipe. The previous code used the legacy `PIPE_NOWAIT` mode to simulate unix-style nonblocking I/O to (unsuccessfully) try to achieve this. Use instead `PIPE_WAIT` and overlapped I/O (the way the pipe is already opened) and, in a somewhat hacky fashion, to simulate Unix behavior, cancel pending requests when the operation returns `ERROR_IO_PENDING`.
2. Avoid leaking duplicated handles when there is a failure transmitting them to the other process.
3. ipc protocol improvement: support optional handles in messages

# Motivation

See DataDog/dd-trace-php#4219


Co-authored-by: gustavo.lopes <gustavo.lopes@datadoghq.com>
gh-worker-dd-mergequeue-cf854d Bot pushed a commit to DataDog/libdatadog that referenced this pull request Sep 29, 2026
# What does this PR do?

Snapshot telemetry client handles before stats and flush inspect them. This keeps the cache lock out of application and client lock scopes while preserving Stop sequencing.

Guard Stop removal by client identity and cover retirement, snapshot lifetime, and replacement races with regression tests.

# How to test the change?

See DataDog/dd-trace-php#4219

Co-authored-by: bwoebi <bob.weinand@datadoghq.com>
Co-authored-by: gustavo.lopes <gustavo.lopes@datadoghq.com>
@cataphract
cataphract force-pushed the glopes/ci-misc branch 2 times, most recently from 9a8c508 to cece80c Compare September 30, 2026 08:45
Advance libdatadog to c80125280 before adopting the later Windows
remote-config notification fix. This keeps the mutable-metadata ABI
additions separate from the notification lifetime change.

Regenerate the headers at this revision, including the existing signal
flush declarations. Keep the dynamic configuration sentinel explicit in
cbindgen because its strict-provenance Rust initializer is unsupported.
Windows sidecar notifications previously passed an extension DLL entry
point to libdatadog, which invoked it through CreateRemoteThread. PHP
could unload php_ddtrace.dll while that remote thread was still running,
causing an access violation in unloaded module code.

Adopt libdatadog client-owned notifications instead. Create one
process-wide registration during MINIT, pass it with every sidecar
configuration, and drop it during MSHUTDOWN. The drop drains callbacks
before the DLL can unload. Unix retains its signal-based notification
path.

NOTE: this includes cbindgen-generated header deduplication which will
need to be included in libdatadog later.
Send an untimed trace before each measured worker.

Require agent info and sampling rates before timing.
Reuse the p0 decision for sibling and inferred spans.

Later emitted chunks still reevaluate automatic sampling, as before this
change. This departs from the Priority Sampling RFC and from the other
tracers, which lock the decision; it is kept as-is here.
@cataphract
cataphract merged commit 7f80da2 into master Sep 30, 2026
2197 checks passed
@cataphract
cataphract deleted the glopes/ci-misc branch September 30, 2026 12:50
@github-actions github-actions Bot added this to the 1.26.0 milestone Sep 30, 2026
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.

2 participants