feat(sidecar)!: Set telemetry session id#1921
feat(sidecar)!: Set telemetry session id#1921gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit intomainfrom
Conversation
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. |
![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: d5eefa47c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| root_session_id: ffi::CharSlice, | ||
| parent_session_id: ffi::CharSlice, |
There was a problem hiding this comment.
Adding root_session_id and parent_session_id to this exported extern "C" symbol changes its call signature in place, which breaks binary compatibility for tracers/extensions built against the previous header but loaded with this newer shared library. In that version-skew scenario the callee reads missing arguments as garbage CharSlices and may dereference invalid memory while evaluating is_empty(), causing undefined behavior or crashes. Preserve the old symbol and add a versioned entrypoint for the extended arguments.
Useful? React with 👍 / 👎.
| cfg.telemetry_heartbeat_interval = config.telemetry_heartbeat_interval; | ||
| cfg.telemetry_extended_heartbeat_interval = | ||
| config.telemetry_extended_heartbeat_interval; | ||
| cfg.session_id = self.session_id.clone(); |
There was a problem hiding this comment.
This update only copies session_id into the telemetry config, so the newly accepted root_session_id and parent_session_id values from session config are never applied to cfg. When callers provide non-empty root/parent session IDs, telemetry payloads will omit that lineage and lose intended parent/root correlation, even though the FFI API now accepts those fields.
Useful? React with 👍 / 👎.
bwoebi
force-pushed
the
bob/set-telemetry-session-id
branch
3 times, most recently
from
504f667 to
fa6b378
Compare
bwoebi
changed the title
bwoebi
force-pushed
the
bob/set-telemetry-session-id
branch
from
fa6b378 to
063b6e8
Compare
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 063b6e8 | Docs | Datadog PR Page | Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1921 +/- ##
==========================================
+ Coverage 71.73% 71.83% +0.09%
==========================================
Files 434 434
Lines 69951 69962 +11
==========================================
+ Hits 50182 50256 +74
+ Misses 19769 19706 -63
🚀 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
|
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
bob/set-telemetry-session-id
branch
3 participants
Follow-up to #1817.