fix(sidecar)!: stabilize the sidecar for macos usage - #2475
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
Clippy Allow Annotation ReportTracked Clippy
By file and crateBy file
By crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. Panic-inducing macros in particular should be avoided. In the future, this report may become a PR-blocking quality gate. |
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: a356590 | Docs | View more details | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27440f63ea
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
BenchmarksComparisonBenchmark execution time: 2026-09-11 19:20:28 Comparing candidate commit a356590 in PR branch Found 7 performance improvements and 6 performance regressions! Performance is the same for 98 metrics, 0 unstable metrics.
|
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
On macOS the SharedDirLiaison uses a filesystem lock to coordinate which process creates the sidecar socket. Under concurrent process startup (e.g. many PHP test workers), losing the lock race is normal: the caller simply connects to the socket the winning process creates. Logging this at warn level fires on every race and pollutes any test that captures log output, which is the dominant failure source on the new macOS CI runner. Linux uses abstract sockets (no lock file) and never hits this path, so the noise only surfaces on macOS. Downgrade to trace, matching the existing treatment of the adjacent "already listening" message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Losing process made exactly one unretried connect() attempt right after the lock-contention branch, before the winner's just-forked daemon had started its accept loop. On macOS this fails hard because the SCM_RIGHTS/ DGRAM IPC rendezvous has no kernel-level accept backlog (unlike Linux's real SEQPACKET listen queue), so a connect before the daemon's accept loop is running throws EAGAIN and the process silently runs with no sidecar for its whole lifetime. Retry connect_to_server() with a short backoff (500ms budget) whenever we didn't create the listener ourselves.
…utables
Mach-O dynamic_symbols() only reflects a dylib's export trie; a main
executable (the php binary) has none, so weaken_object_symbols always
matched zero candidates on macOS and no Zend/PHP symbol ever got its
undefined reference weakened in ddtrace.so. This left hard, non-weak
undefined references (e.g. _OnUpdateString) that make the sidecar
daemon's dlopen of ddtrace.so abort outright ('symbol not found in
flat namespace') whenever a code path referencing them runs inside
the daemon (observed with sidecar debug logging enabled), killing the
freshly spawned daemon.
Fall back to the full symbol table filtered to Dynamic-scope
definitions on macOS, which is where a main executable's exported
symbols actually show up. Also stopped excluding symbols the object
crate already reports as is_weak() in both candidate-selection filters
-- for Mach-O relocatable objects it has been observed to misreport
genuinely non-weak symbols as weak, which silently skipped them;
re-marking an already-weak symbol is a harmless no-op.
Concurrent flush tasks (traces, telemetry, ...) can all target the same single-file file:// dump endpoint. write_all for a large record can issue more than one write() syscall, so two concurrent writers could interleave bytes into the file even under O_APPEND (which only makes each individual write() atomic, not the whole record), corrupting the dump file. Serialize the open+write with a mutex so a record is never split.
…e async connection into_async_conn() only extracted the raw inner fd into the AsyncFd wrapper, letting the rest of SeqpacketConn (macOS's _peer and liveness fields) drop immediately on return. Since the daemon's accepted connection carries the client's transferred liveness-pipe read end in that liveness field, this closed it right after every single accept, which the client's liveness check reads as an instant, false POLLHUP disconnect -- even though the actual data connection (self.inner) was still perfectly usable. The client then tore down and reconnected on essentially every call, polluting exact-match .phpt test output with 'transport is closed. Reconnecting' warnings and failing hundreds of tests. Wrap the whole SeqpacketConn in AsyncFd instead of just its raw fd, so _peer/liveness stay alive for as long as the connection is actually in use. Also loosened entry.rs's connect retry to poll the side-effect-free attempt_listen() rather than the handshake-sending connect_to_server() while chasing this, since blindly retrying a call with real side effects (a fresh fd pair + real SCM_RIGHTS send) could leave behind abandoned, server-accepted connections; that turned out not to be the actual bug; the current form is simply more correct. Verified: full local test_c suite went from 155/604 failing (before any of this session's fixes) to 0/604 failing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mitted yet mmap_handle() reads a segment's real size from a negotiation slot shared by every process mapping it (see the block above), populated the first time some process calls ensure_space(). A reader that opens the segment before any writer has done so sees a technically valid but not-yet-populated mapping (size still zero), which panicked via unwrap() -- observed as a SIGABRT crash in a macOS CI run reading a fleet-config remote-config file mid-write. Return an error instead, matching how a not-yet-present mapping would be handled: existing callers already tolerate a transient read failure here gracefully (e.g. shm_remote_config.rs's read_config callers just log and retry on the next poll), so this only changes a process-aborting panic into an already-handled error path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bwoebi
force-pushed
the
bob/macos-improv
branch
from
September 8, 2026 21:50
27440f6 to
9d01a8d
Compare
…acOS The macOS-only retry arm was #[cfg]-gated directly on the match arm, so on every other platform the loop had no branch that continues -- clippy's never_loop lint (deny-by-default) correctly flagged it as dead weight, and non-macOS builds silently never retried (harmlessly, since they don't need to, but the lint failure broke clippy/helper-rust CI jobs. Make the deadline an Option instead: None on non-macOS keeps the retry arm unconditionally present in the source (satisfying clippy) while still never firing at runtime there. EOF )
bwoebi
added a commit
that referenced
this pull request
Sep 9, 2026
- sidecar_mockgen: only strip an actual leading underscore from a Mach-O symbol name before matching, instead of unconditionally slicing off the first byte. A hand-written assembly global need not have the '_' prefix, so the unconditional slice silently mismatched such symbols against weaken_macho's lookup (which already only strips a real '_'), leaving them unweakened. - libdd-ipc/macos sockets: pthread_chdir_np/pthread_fchdir_np return the error number directly rather than setting errno, so last_os_error() could report an unrelated error. Also stop discarding the restore call's result: if it fails, the thread's CWD is left pointing at the socket directory, corrupting later relative-path resolution on that thread; surface that as an error when the wrapped operation itself succeeded. - libdd-capabilities-impl/http.rs: don't use lock_or_panic() for the file:// write-serialization lock -- this is FFI-facing code where a panic can abort the host process. Propagate a poisoned-lock as an HttpError instead. - datadog-sidecar/shm_remote_config.rs: a config that fails to read (e.g. the macOS "SHM mapping not yet committed" transient race) was being dropped from last_read_configs without being re-queued, so it wouldn't actually retry on the next poll as intended -- only a later, unrelated config-list change would resurface it. Re-queue failed configs so they retry on the next fetch_update() call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bwoebi
force-pushed
the
bob/macos-improv
branch
from
September 9, 2026 13:34
4a1bed3 to
6715d70
Compare
bwoebi
changed the title
bwoebi
force-pushed
the
bob/macos-improv
branch
from
September 9, 2026 13:40
6715d70 to
40fd1ab
Compare
Leiyks
approved these changes
Sep 10, 2026
bwoebi
force-pushed
the
bob/macos-improv
branch
from
September 11, 2026 16:29
40fd1ab to
f98aa49
Compare
Contributor
Author
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
bwoebi
changed the title
ekump
approved these changes
Sep 11, 2026
ekump
left a comment
ekump
left a comment
Collaborator
There was a problem hiding this comment.
Nothing blocking, just a couple of questions
Co-authored-by: Edmund Kump <edmund.kump@datadoghq.com>
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
paullegranddc
added a commit
that referenced
this pull request
Sep 24, 2026
…ibdd-data-pipeline, libdd-li... (#2571) <!-- release-proposal-inputs: {"crates":"libdd-capabilities-impl, libdd-common, libdd-data-pipeline, libdd-library-config, libdd-profiling-heap-allocator, libdd-remote-config, libdd-sampling, libdd-shared-runtime, libdd-telemetry, libdd-tinybytes, libdd-trace-utils","main_start_ref":"","level_overrides":"","bypass_standard_checks":false} --> # Release proposal for libdd-capabilities-impl, libdd-common, libdd-data-pipeline, libdd-library-config, libdd-profiling-heap-allocator, libdd-remote-config, libdd-sampling, libdd-shared-runtime, libdd-telemetry, libdd-tinybytes, libdd-trace-utils and their dependencies This PR contains version bumps based on public API changes and commits since last release. ###⚠️ Crates left out of this proposal affected by its major bumps These publishable workspace crates are not part of this release but their dependency requirement was rewritten on this branch while their published version still requires the old major. If they are a dependency on your deployment not including them in the release could result in duplicate packages or symbol incompatibility. - `libdd-capabilities` `3.0.1` → `4.0.0` affects: `libdd-crashtracker`, `libdd-ffe`, `libdd-live-debugger` - `libdd-capabilities-impl` `5.0.0` → `6.0.0` affects: `libdd-crashtracker`, `libdd-live-debugger`, `libdd-tracer-flare` - `libdd-common` `6.0.0` → `7.0.0` affects: `libdd-crashtracker`, `libdd-ffe`, `libdd-http-client`, `libdd-ipc`, `libdd-live-debugger`, `libdd-profiling`, `libdd-tracer-flare` - `libdd-data-pipeline` `10.0.0` → `11.0.0` affects: `libdd-live-debugger` - `libdd-remote-config` `5.0.0` → `6.0.0` affects: `libdd-ffe`, `libdd-live-debugger`, `libdd-tracer-flare` - `libdd-telemetry` `8.0.0` → `9.0.0` affects: `libdd-crashtracker` - `libdd-trace-stats` `9.0.0` → `10.0.0` affects: `libdd-ipc` - `libdd-trace-utils` `12.0.0` → `13.0.0` affects: `libdd-tracer-flare` ## libdd-capabilities **Next version:** `4.0.0` **Semver bump:** `major` **Tag:** `libdd-capabilities-v4.0.0` ### Commits - feat(trace-exporter)!: add API to force flush stats on the trace exporter (#2538) - refactor!: apply small timeout pooling strategy to libdd-http-client as well (#2449) - chore: move all remaining external deps to workspace-level dependencies (#2476) ## libdd-common **Next version:** `7.0.0` **Semver bump:** `major` **Tag:** `libdd-common-v7.0.0` ### Commits - build: enforce workspace-level dependency declarations (#2514) - chore!: remove unused dependencies (v2) (#2511) - chore: move all remaining external deps to workspace-level dependencies (#2476) - chore(libdd-data-pipeline): solve cargo deny [APMSP-3884] (#2318) - fix!: benchmark was doing many samples for deterministic heap usage (#2465) ## libdd-ddsketch **Next version:** `1.1.2` **Semver bump:** `patch` **Tag:** `libdd-ddsketch-v1.1.2` ### Commits - chore: move all remaining external deps to workspace-level dependencies (#2476) - chore(libdd-data-pipeline): solve cargo deny [APMSP-3884] (#2318) ## libdd-profiling-heap-sampler **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-profiling-heap-sampler-v1.1.0` ### Commits - chore: move all remaining external deps to workspace-level dependencies (#2476) - refactor: migrate to workspace dependencies, phase 4 (#2296) - feat(profiling): add USDT semaphores to skip slow path around heap profile sampler (#2266) ## libdd-tinybytes **Next version:** `1.1.4` **Semver bump:** `patch` **Tag:** `libdd-tinybytes-v1.1.4` ### Commits - chore: move all remaining external deps to workspace-level dependencies (#2476) ## libdd-trace-protobuf **Next version:** `5.0.1` **Semver bump:** `patch` **Tag:** `libdd-trace-protobuf-v5.0.1` ### Commits - chore: move all remaining external deps to workspace-level dependencies (#2476) - fix(build): build script change detection (#2467) ## libdd-capabilities-impl **Next version:** `6.0.0` **Semver bump:** `major` **Tag:** `libdd-capabilities-impl-v6.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 ### Commits - refactor!: apply small timeout pooling strategy to libdd-http-client as well (#2449) - fix(sidecar)!: stabilize the sidecar for macos usage (#2475) - chore: move all remaining external deps to workspace-level dependencies (#2476) ## libdd-profiling-heap-allocator **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-profiling-heap-allocator-v1.1.0` ### Commits - refactor(benchmarks): measure allocator thread CPU time (#2500) - refactor: migrate to workspace dependencies, phase 4 (#2296) - feat(profiling): add USDT semaphores to skip slow path around heap profile sampler (#2266) ## libdd-library-config **Next version:** `4.1.0` **Semver bump:** `minor` **Tag:** `libdd-library-config-v4.1.0` ### Commits - chore!: remove unused dependencies (v2) (#2511) - chore: move all remaining external deps to workspace-level dependencies (#2476) ## libdd-trace-normalization **Next version:** `4.1.0` **Semver bump:** `minor` **Tag:** `libdd-trace-normalization-v4.1.0` ### Commits - chore: move all remaining external deps to workspace-level dependencies (#2476) - chore(libdd-data-pipeline): solve cargo deny [APMSP-3884] (#2318) ## libdd-remote-config **Next version:** `6.0.0` **Semver bump:** `major` **Tag:** `libdd-remote-config-v6.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-capabilities-impl`: ^5.0.0 → ^6.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 ### Commits - refactor!: apply small timeout pooling strategy to libdd-http-client as well (#2449) - chore!: remove unused dependencies (v2) (#2511) - fix(rem-cfg)!: remove AsmRawResponseBody (#2540) - chore: move all remaining external deps to workspace-level dependencies (#2476) ## libdd-shared-runtime **Next version:** `5.0.0` **Semver bump:** `major` **Tag:** `libdd-shared-runtime-v5.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-capabilities-impl`: ^5.0.0 → ^6.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 ### Commits - feat(trace-exporter)!: add API to force flush stats on the trace exporter (#2538) - chore: move all remaining external deps to workspace-level dependencies (#2476) ## libdd-trace-utils **Next version:** `13.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-utils-v13.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-capabilities-impl`: ^5.0.0 → ^6.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 ### Commits - fix(trace-utils)!: add forgotten css to agentless v1 encoder (#2557) - fix(otlp): use potential dedupe in otlp serialization path (#2531) - feat(trace-utils)!: add v1-native OTLP encoder brick (#2369) - perf(serialization): bench the actual path used by the trace exporter (#2512) - perf(trace): write agentless JSON directly (#2454) - chore: move all remaining external deps to workspace-level dependencies (#2476) - perf(trace): encode agentless IDs without formatting (#2452) - perf(trace-utils)!: use pooled spans on the send path (#2382) - feat(data-pipeline)!: OTLP gRPC trace export (#2171) - chore(libdd-data-pipeline): solve cargo deny [APMSP-3884] (#2318) - fix!: benchmark was doing many samples for deterministic heap usage (#2465) ## libdd-dogstatsd-client **Next version:** `7.0.0` **Semver bump:** `major` **Tag:** `libdd-dogstatsd-client-v7.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^6.0.0 → ^7.0.0 - `libdd-shared-runtime`: ^4.0.0 → ^5.0.0 ### Commits - chore: move all remaining external deps to workspace-level dependencies (#2476) ## libdd-telemetry **Next version:** `9.0.0` **Semver bump:** `major` **Tag:** `libdd-telemetry-v9.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 - `libdd-shared-runtime`: ^4.0.0 → ^5.0.0 ### Commits - refactor!: apply small timeout pooling strategy to libdd-http-client as well (#2449) - chore!: remove unused dependencies (v2) (#2511) - chore: move all remaining external deps to workspace-level dependencies (#2476) - fix(telemetry): fall back to the proper intake telemetry domain when direct submission is enabled (#2484) ## libdd-sampling **Next version:** `7.0.0` **Semver bump:** `major` **Tag:** `libdd-sampling-v7.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^7.0.0 - `libdd-trace-utils`: ^10.1.0 → ^13.0.0 ### Commits - chore(benchmarks): remove batched-loop sampling noise (#2498) - chore: move all remaining external deps to workspace-level dependencies (#2476) - fix!: benchmark was doing many samples for deterministic heap usage (#2465) - feat(trace-utils)!: add from owned to SpanText (#2403) - test(sampling): make rate_limiter thread-safety test deterministic (#2354) ## libdd-trace-obfuscation **Next version:** `9.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-obfuscation-v9.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^6.0.0 → ^7.0.0 - `libdd-trace-utils`: ^12.0.0 → ^13.0.0 ### Commits - feat(data-pipeline)!: refactor agent's /info obfuscation config format (#2490) - fix(obfuscation): keep URLs whose path or fragment holds a bracket (#2527) - fix(obfuscation): stop forcing serde_json/preserve_order on dependents (#2529) - fix(obfuscation): prevent crash and cap recursion in SQL/HTTP obfuscation (#2441) - chore: move all remaining external deps to workspace-level dependencies (#2476) - fix(data-pipeline-ffi): harden agentless config setters and parse obfuscation config once (#2474) ## libdd-trace-stats **Next version:** `10.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-stats-v10.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-capabilities-impl`: ^5.0.0 → ^6.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 - `libdd-shared-runtime`: ^4.0.0 → ^5.0.0 - `libdd-trace-obfuscation`: ^8.0.0 → ^9.0.0 - `libdd-trace-utils`: ^12.0.0 → ^13.0.0 ### Commits - feat(trace-exporter)!: add API to force flush stats on the trace exporter (#2538) - fix(trace-stats): cardinality limit telemetry name (#2559) - feat(data-pipeline)!: refactor agent's /info obfuscation config format (#2490) - chore!: remove unused dependencies (v2) (#2511) - feat(data-pipeline)!: generate agentless trace stats (#2488) - chore: move all remaining external deps to workspace-level dependencies (#2476) - chore(libdd-data-pipeline): solve cargo deny [APMSP-3884] (#2318) ## libdd-data-pipeline-core **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-data-pipeline-core-v2.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 - `libdd-trace-obfuscation`: ^8.0.0 → ^9.0.0 - `libdd-trace-utils`: ^12.0.0 → ^13.0.0 ### Commits - refactor!: apply small timeout pooling strategy to libdd-http-client as well (#2449) - feat(data-pipeline)!: generate agentless trace stats (#2488) - perf(data-pipeline): avoid parsing static entity header names (#2457) - chore: move all remaining external deps to workspace-level dependencies (#2476) - perf(trace-utils)!: use pooled spans on the send path (#2382) ## libdd-data-pipeline **Next version:** `11.0.0` **Semver bump:** `major` **Tag:** `libdd-data-pipeline-v11.0.0` ###⚠️ major bump forced due to: - `libdd-capabilities`: ^3.0.1 → ^4.0.0 - `libdd-capabilities-impl`: ^5.0.0 → ^6.0.0 - `libdd-common`: ^6.0.0 → ^7.0.0 - `libdd-data-pipeline-core`: ^1.0.0 → ^2.0.0 - `libdd-shared-runtime`: ^4.0.0 → ^5.0.0 - `libdd-trace-obfuscation`: ^8.0.0 → ^9.0.0 - `libdd-trace-stats`: ^9.0.0 → ^10.0.0 - `libdd-trace-utils`: ^12.0.0 → ^13.0.0 ### Commits - feat(trace-exporter)!: add API to force flush stats on the trace exporter (#2538) - refactor!: apply small timeout pooling strategy to libdd-http-client as well (#2449) - feat(data-pipeline)!: refactor agent's /info obfuscation config format (#2490) - test(data-pipeline): stop OTLP gRPC post-connect test from hanging (#2549) - chore!: remove unused dependencies (v2) (#2511) - feat(data-pipeline)!: generate agentless trace stats (#2488) - fix(otlp): use potential dedupe in otlp serialization path (#2531) - refactor(benchmarks): isolate enqueue timing in data-pipeline (#2499) - perf(trace): write agentless JSON directly (#2454) - test(data-pipeline): fix flaky OTLP gRPC exporter integration tests (#2505) - chore: move all remaining external deps to workspace-level dependencies (#2476) - perf(trace-utils)!: use pooled spans on the send path (#2382) - feat(data-pipeline)!: OTLP gRPC trace export (#2171) - chore(libdd-data-pipeline): solve cargo deny [APMSP-3884] (#2318) [APMSP-3884]: https://datadoghq.atlassian.net/browse/APMSP-3884?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: paullegranddc <82819397+paullegranddc@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Macos more or less worked, but with quite a bunch of edge cases, except for weakening (which is fixed here).
This makes it mostly non-flaky.
As a side-note: Working on this, I encountered an issue where telemetry to file:// was causing interleaved data. Guarded it.