Skip to content

[SVLS-8799] Update libdatadog revision#115

Open
kathiehuang wants to merge 7 commits intomainfrom
kathie.huang/update-libdatadog-rev-986aab55cb7941d8453dffb59d35a70599d08665
Open

[SVLS-8799] Update libdatadog revision#115
kathiehuang wants to merge 7 commits intomainfrom
kathie.huang/update-libdatadog-rev-986aab55cb7941d8453dffb59d35a70599d08665

Conversation

@kathiehuang
Copy link
Copy Markdown
Contributor

@kathiehuang kathiehuang commented Apr 17, 2026

What does this PR do?

Motivation

https://datadoghq.atlassian.net/browse/SVLS-8799

DataDog/libdatadog#1857 updates the agent's cloud environment detection logic to enable customers instrumenting their Azure Functions to set the FUNCTION_NAME environment variable. Previously, FUNCTION_NAME was used to identify a Google Cloud Run Function; this change makes the detection logic more specific by checking multiple cloud-specific env vars at once.

Additional Notes

  • feat(capabilities)!: trait architecture http libdatadog#1555 introduced a capability trait architecture (groundwork for WASM support). SendData::send is now generic over H: HttpClientTrait instead of GenericHttpClient<C: Connect>, so our flushers no longer compiled against the new rev.
    • stats_flusher: now useslibdd_common::DefaultHttpClient
    • trace_flusher: needs proxy support which DefaultHttpClient doesn't provide. Introduced a local ProxyHttpClient newtype that wraps GenericHttpClient<ProxyConnector<Connector>> and implements HttpClientTrait
      • The connector construction logic is unchanged and the request() body mirrors libdatadog's own DefaultHttpClient::request
  • Updated rustls-webpki to address this audit check
  • Added FUNCTION_TARGET alongside every KSERVICE in test_default_trace_and_trace_stats_urls since read_cloud_env in libdatadog now requires both env vars to detect a Cloud Run Function when creating a Config

Describe how to test/QA your changes

Deployed an Azure function and set the env var FUNCTION_NAME. Confirmed that traces continued to flow to Datadog

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo to a newer libdatadog revision and adapts serverless trace/stats flushing code and dependency pins to remain compatible with the upstream API and constraints.

Changes:

  • Bump libdd-* git dependencies to rev 986aab55… and add libdd-capabilities where required.
  • Adapt trace_flusher/stats_flusher to the new HttpClientTrait-based SendData::send API (including a local proxy-capable client wrapper).
  • Pin rustls-native-certs to <0.8.3, update cloud env detection tests, and refresh lockfile/license inventory entries.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/datadog-trace-agent/src/trace_flusher.rs Introduces ProxyHttpClient implementing HttpClientTrait and updates trace flushing to use it
crates/datadog-trace-agent/src/stats_flusher.rs Switches stats sending to use libdd_common::DefaultHttpClient
crates/datadog-trace-agent/src/config.rs Updates tests to include FUNCTION_TARGET for Cloud Run Function detection
crates/datadog-trace-agent/Cargo.toml Bumps libdatadog revs and adds libdd-capabilities dependency
crates/datadog-serverless-compat/Cargo.toml Bumps libdd-trace-utils rev
crates/datadog-fips/Cargo.toml Pins rustls-native-certs to <0.8.3
crates/datadog-agent-config/Cargo.toml Bumps libdatadog revs
LICENSE-3rdparty.csv Adds libdd-capabilities* entries and updates openssl-probe repo URL
Cargo.lock Updates resolved dependency graph for the new rev and TLS-related pins

Comment on lines +84 to +85
Err(e) => {
error!("Failed to create HTTP client: {e:?}");
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error log here formats the underlying error with {:?}. Since with_proxy() parses the proxy URL, the error value may include the raw proxy URL (including potential userinfo credentials), which would leak secrets into logs. Consider logging a sanitized/redacted proxy URL (strip userinfo) or logging a higher-level error without embedding the raw URL/error debug that might contain it.

Suggested change
Err(e) => {
error!("Failed to create HTTP client: {e:?}");
Err(_e) => {
error!("Failed to create HTTP client from configured proxy URL");

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the pre-existing behavior

Comment on lines +136 to +139
#[allow(clippy::expect_used)]
fn new_client() -> Self {
Self::with_proxy(None).expect("building proxy connector with default TLS should not fail")
}
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HttpClientTrait::new_client uses expect(...) and will panic if building the default proxy connector fails (e.g., TLS/init issues). Even if it’s not currently called, it’s part of the trait contract and could be exercised by upstream code in the future. Consider making new_client panic-free (e.g., store the construction error in the struct and have request() return HttpError), or otherwise ensure failures are handled without crashing the process.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProxyHttpClient is a private struct, I think it'd be very unlikely for new_client to be exercised by upstream code in the future

@kathiehuang kathiehuang marked this pull request as ready for review April 17, 2026 19:59
@kathiehuang kathiehuang requested review from a team as code owners April 17, 2026 19:59
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