chore(libdd-traceutils): Update cloud environment detection logic for Serverless [SVLS-8799]#1857
chore(libdd-traceutils): Update cloud environment detection logic for Serverless [SVLS-8799]#1857kathiehuang wants to merge 7 commits intomainfrom
Conversation
📚 Documentation Check Results📦
|
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats 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. |
🔒 Cargo Deny Results✅ No issues found! 📦
|
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
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1857 +/- ##
==========================================
+ Coverage 71.69% 71.78% +0.09%
==========================================
Files 429 429
Lines 67886 68019 +133
==========================================
+ Hits 48672 48830 +158
+ Misses 19214 19189 -25
🚀 New features to boost your workflow:
|
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 70a4ee8 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback! |
kathiehuang
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the cloud environment detection logic in libdd-trace-utils to be more restrictive and aligned with the Serverless Compatibility Layers by requiring multiple cloud-specific environment variables to identify each platform, rather than relying on a single environment variable. This prevents misidentification when environment variables like FUNCTION_NAME are manually set in environments where they don't naturally occur.
Changes:
- Modified
read_cloud_env()to require multiple detection environment variables per platform instead of single variables - Refactored to collect all detected environments and handle conflicts (returning None when multiple platforms are detected)
- Added structured logging using the
tracingcrate for debug messages when one environment is detected and error messages for zero, multiple, or incomplete environment detections - Added comprehensive unit tests covering all detection paths, missing name variables, single-var-only cases, multiple conflicting environments, and no environment scenarios
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 89fcc56d6f
ℹ️ 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 EnvGuard { | ||
| fn set(key: &'static str, value: &str) -> Self { | ||
| let saved = env::var(key).ok(); | ||
| unsafe { env::set_var(key, value) }; |
There was a problem hiding this comment.
These tests mutate global environment state via unsafe env::set_var/remove_var without any synchronization, but Rust’s test harness runs tests in parallel by default. That creates cross-test interference (and unsound concurrent env access) where one test can clear variables while another is reading them, which already reproduces as nondeterministic failures with cargo test -p libdd-trace-utils config_utils (without --test-threads=1). Please guard env mutation with a global lock or mark this module’s tests as serial.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated to serialize the unit tests rather than requiring the tests to be run with --test-threads=1 a1b3fea
What does this PR do?
Motivation
A customer who instrumenting a Java Azure Function had set
FUNCTION_NAMEas an environment variable. This led the current libdatadog logic to returnEnvironmentType::CloudFunction, which led to this being called in serverless-components which returned an error after running a Google-specific metadata check.As a result, we want the environment detection logic to check multiple cloud-specific env vars at once.
Additional Notes
Once this PR is merged, we plan to make a new PR in serverless-components to update the libdatadog commit hash.
How to test the change?
Unit tests: Run
cargo test -p libdd-trace-utils config_utilseverywhere that libdatadog is used
DD_LOG_LEVEL=DEBUGin GCP/Azure functions to make sure the correct environment is detectedFUNCTION_NAMEas an env var in Azure Functions to make sure it isn't identified as a Google Cloud Functionserverless-compat-self-monitoring/azure_functions/code/compat/python/datadog_serverless_compat-0.0.0-py3-none-any.whland drop it in the root of the app. Replacedatadog-serverless-compatinrequirements.txtto this file name.Deployed a Java Azure Function without this change with
FUNCTION_NAME=myfunctionand got the same error:Redeployed the Azure Function with this change. The error disappeared and we're getting traces:
Also deployed a Python Google Cloud Function Gen 1 with this change. Interestingly, it's getting detected as Gen 2.
I printed out the env vars and
FUNCTION_TARGETandK_SERVICEare being set, but notFUNCTION_NAMEandGCP_PROJECT:Serverless compatibility layers that don't check
FUNCTION_TARGETorK_SERVICEshould be updated (e.g. serverless-compat-java) to have the same logic as other runtimes (serverless-compat-py)To update: