[SVLS-7018] Support profiling in Azure Functions #40
Conversation
kathiehuang
changed the title
…ushed in proxy flusher
kathiehuang
force-pushed
the
kathie.huang/receive-profile-payload
branch
from
c0abc20 to
d9ea13b
Compare
…andle forwarding and retries in background task
kathiehuang
changed the title
kathiehuang
changed the title
|
@codex review |
![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.
ℹ️ 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".
| // Extract headers and body | ||
| let (parts, body) = request.into_parts(); | ||
|
|
||
| let body_bytes = match body.collect().await { | ||
| Ok(collected) => collected.to_bytes(), | ||
| Err(e) => { | ||
| return log_and_create_http_response( | ||
| &format!("Error reading profiling request body: {e}"), | ||
| StatusCode::BAD_REQUEST, | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| // Create proxy request | ||
| let proxy_request = ProxyRequest { | ||
| headers: parts.headers, | ||
| body: body_bytes, | ||
| target_url: config.profiling_intake.url.to_string(), | ||
| }; |
There was a problem hiding this comment.
The profiling proxy handler builds the outgoing request URL from the static profiling_intake endpoint and ignores parts.uri.query(). Profiling clients pass service, environment, and other metadata via the query string when posting to /profiling/v1/input; dropping these parameters means the flusher forwards payloads without required metadata, causing ingestion failures or misattribution. Append the incoming query parameters to ProxyRequest.target_url before sending to the proxy flusher.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The lambda extension doesn't preserve query parameters: https://github.com/DataDog/datadog-lambda-extension/blob/main/bottlecap/src/traces/proxy_flusher.rs#L119
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| /// Handles incoming proxy requests for profiling - can be abstracted into a generic proxy handler for other proxy requests in the future | ||
| async fn profiling_proxy_handler( | ||
| config: Arc<config::Config>, | ||
| request: hyper_migration::HttpRequest, | ||
| proxy_tx: Sender<ProxyRequest>, | ||
| ) -> http::Result<hyper_migration::HttpResponse> { | ||
| debug!("Trace Agent | Received profiling request"); | ||
|
|
||
| // Extract headers and body | ||
| let (parts, body) = request.into_parts(); | ||
|
|
||
| let body_bytes = match body.collect().await { | ||
| Ok(collected) => collected.to_bytes(), | ||
| Err(e) => { | ||
| return log_and_create_http_response( | ||
| &format!("Error reading profiling request body: {e}"), | ||
| StatusCode::BAD_REQUEST, | ||
| ); |
There was a problem hiding this comment.
The new profiling proxy endpoint reads the entire request body into memory (body.collect().await) without validating the Content-Length. The trace and stats handlers call verify_request_content_length to reject payloads larger than Config::max_request_content_length before allocation, but the profiling handler omits this guard. A client can therefore POST an arbitrarily large profiling payload and the mini agent will buffer it in RAM before forwarding, allowing a single request to exhaust memory and crash the function. Consider applying the same content length check as the other endpoints before collecting the body.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The lambda extension also doesn't validate the content-length, probably because profiling payloads are typically much larger and they're coming from the Datadog profiler
kathiehuang
changed the title
What does this PR do?
mpscchannel where the proxy flusher receives it and forwards it to Datadog, handling retries and exponential backoffsdatadog-fipstoCargo.tomlto use its reqwest client builder methodMotivation
Profiling is not yet supported in Azure Functions.
Additional Notes
Tested across all hosting plans. This works for Python and Node.js.
This worked in Java as well in testing, but the Java Continuous Profiler in serverless is not currently supported by the Profiling team.
The .NET Continuous Profiler in serverless is also not currently supported by the Profiling team - could not test because the .NET tracer shuts down the profiler if it detects it's in an Azure Function:
Describe how to test/QA your changes
To test this, follow the Serverless Compatibility Layer instructions to deploy function apps across runtimes and hosting plans with the built binary. Generate load on the function app and check Datadog to see profiles show up!
Follow up task: Talk with APM Serverless about changes in their tracer code that will need to be made. SVLS-7841