In-process log submission in sidecar (for appsec helper) #1051
Conversation
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
cataphract
force-pushed
the
glopes/in-process-log-submission-ffi
branch
from
293a4fc to
c305218
Compare
BenchmarksComparisonBenchmark execution time: 2025-05-12 16:16:30 Comparing candidate commit 166dca5 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 52 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
BaselineOmitted due to size. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1051 +/- ##
==========================================
- Coverage 71.19% 70.81% -0.38%
==========================================
Files 322 324 +2
Lines 49254 49450 +196
==========================================
- Hits 35067 35019 -48
- Misses 14187 14431 +244
🚀 New features to boost your workflow:
|
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
|
cataphract
force-pushed
the
glopes/in-process-log-submission-ffi
branch
from
c305218 to
175db0d
Compare
2 tasks
cataphract
force-pushed
the
glopes/in-process-log-submission-ffi
branch
from
175db0d to
166dca5
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements in-process log submission within the sidecar to eliminate an extra roundtrip between the appsec helper and the PHP extension. Key changes include adding FFI functions for telemetry log submission, introducing an in-process telemetry action receiver task, and minor visibility adjustments in internal modules.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| datadog-sidecar/src/windows.rs | Added cbindgen ignore comment for FFI symbol export |
| datadog-sidecar/src/unix.rs | Added cbindgen ignore comment for FFI symbol export |
| datadog-sidecar/src/shm_remote_config.rs | Added cbindgen ignore comment for an unsafe FFI function |
| datadog-sidecar/src/service/telemetry/mod.rs | Reorganized telemetry modules and re-exported in-process receiver functions |
| datadog-sidecar/src/service/telemetry/in_proc_receiver.rs | Introduced asynchronous in-process telemetry action receiver task |
| datadog-sidecar/src/service/telemetry/ffi.rs | Added new FFI functionality for enqueuing telemetry log actions |
| datadog-sidecar/src/service/sidecar_server.rs | Adjusted visibility modifiers for internal fields and methods |
| datadog-sidecar/src/service/queue_id.rs | Adjusted visibility for the inner field of QueueId |
| datadog-sidecar/src/entry.rs | Spawned telemetry action receiver task during sidecar startup |
| datadog-sidecar/include/sidecar_ffi.h | Updated autogenerated FFI header |
| datadog-sidecar/cbindgen.toml | Added configuration for cbindgen |
| datadog-sidecar/Cargo.toml | Added build dependency for cbindgen |
Comments suppressed due to low confidence (1)
datadog-sidecar/src/service/telemetry/ffi.rs:159
- Typo in field name 'indentifier'; consider renaming it to 'identifier' for clarity.
indentifier: hasher.finish(),
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do?
Adds a task for receiving telemetry log messages in process and a corresponding ffi function for sending them.
Motivation
The appsec helper (loaded in the sidecar process) at this point sends telemetry data by first sending it to the appsec php extension (via unix sockets), that then calls a ddtracer function to send them back to sidecar. This eliminates this roundtrip for telemetry log messages.
If the approach is validated, the same one will likely be taken for telemetry metrics.
How to test the change?
The only tests now are integration tests on the appsec side, in a PR that will link to. Let me know if I should include other tests.