fix(remote-config): Expose HttpClientCapability in remote config#2252
Open
bwoebi wants to merge 1 commit into
Open
fix(remote-config): Expose HttpClientCapability in remote config#2252bwoebi wants to merge 1 commit into
bwoebi wants to merge 1 commit into
Conversation
Contributor
📚 Documentation Check Results📦
|
bwoebi
changed the title
Contributor
🔒 Cargo Deny Results📦
|
Contributor
|
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
More details
The body_bytes = response.into_body() change is correct: HttpClientCapability::request returns http::Response<bytes::Bytes> with the body fully buffered inside NativeHttpClient, so no further .collect().await is needed. All 24 existing unit tests pass, covering the full fetch pipeline through a local mock HTTP server. The timeout fix is legitimate — the body collection is now inside the timed future, closing a real hang risk in the old code.
📊 Validated against 24 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 36072ea · What is Autotest? · Any feedback? Reach out in #autotest
![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: 36072ea7cf
ℹ️ 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".
| impl<S> ConfigFetcherState<S> { | ||
| impl<S> ConfigFetcherState<S, NativeHttpClient> { | ||
| pub fn new(invariants: ConfigInvariants) -> Self { | ||
| Self::with_client(invariants, NativeHttpClient::new_client()) |
There was a problem hiding this comment.
With the default constructor, every SingleFetcher/SharedFetcher now stores one NativeHttpClient for the whole polling loop. NativeHttpClient caches a new_default_client() in a OnceLock, and new_default_client() keeps idle connections, while http_common::new_client_periodic() explicitly disables pooling because the agent's low keep-alive can close the pipe on the next periodic request. For default RC polling, this changes the previous per-request client behavior into pooled reuse and can make subsequent polls fail intermittently; please use a periodic/no-idle native client for the default.
Useful? React with 👍 / 👎.
Comment on lines
130
to
133
| impl<S: FileStorage> SingleChangesFetcher<S, NativeHttpClient> | ||
| where | ||
| S::StoredFile: FilePath, | ||
| { |
There was a problem hiding this comment.
Although the struct is now generic over C, this only public constructor remains on SingleChangesFetcher<S, NativeHttpClient>, and the private changes field prevents downstream code from constructing SingleChangesFetcher<S, C> around SingleFetcher::with_client. Callers that use the change-tracking API rather than raw fetch_once still cannot inject a custom HttpClientCapability; add a with_client constructor mirroring SingleFetcher::with_client.
Useful? React with 👍 / 👎.
| client = [ | ||
| "libdd-trace-protobuf", | ||
| "libdd-capabilities", | ||
| "libdd-capabilities-impl", |
There was a problem hiding this comment.
Enabling the client feature now always enables libdd-capabilities-impl, even when a downstream only wants to instantiate ConfigFetcherState::with_client with its own HttpClientCapability. I checked libdd-capabilities-impl/src/http.rs, and that implementation imports native-only pieces such as libdd_common::connector::Connector, so custom-client builds for non-native targets still have to compile the native backend and fail before they can supply their own client. Please keep the native implementation behind the default/native constructors or a target-specific dependency instead of making it mandatory for client.
Useful? React with 👍 / 👎.
As a side-effect, this fixes the missing timeout on the remote config body awaiting. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
bwoebi
force-pushed
the
bob/rc-native-client
branch
from
July 17, 2026 17:59
36072ea to
39d4b97
Compare
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-07-17 18:31:57 Comparing candidate commit 39d4b97 in PR branch Found 12 performance improvements and 7 performance regressions! Performance is the same for 123 metrics, 0 unstable metrics.
|
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
1 participant
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.
As a side-effect, this fixes the missing timeout on the remote config body awaiting.