Repository navigation
[MCC-1554792] - R Lib - Trace ID Propagation - #120
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved result-contract compatibility issues and outdated test fixtures block approval.
Review effort: Lite
Findings: 3
Open (4)
What changed in this PR
This pull request adds trace ID propagation across dataset operations, publishing, datetime formats, and fetched table results.
Changes:
- Extracts trace IDs from Flight metadata and Arrow schema metadata.
- Updates public result shapes to include
trace_id. - Extends tests and documentation for the new structures.
| File | Reviewed changes |
|---|---|
tests/testthat/test-datetime-formats.R |
Datetime-format result and propagation tests |
tests/testthat/test-datasets.R |
Dataset and table trace propagation tests |
R/table.R |
Fetched data results now include trace IDs |
R/datasets.R |
Flight and dataset trace extraction and propagation |
R/dataconnect_client.R |
Documentation for updated public result shapes |
R/commands.R |
Command, publishing, and datetime-format trace handling |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d46a06f to
15a9148
Compare
b4fd8da to
31e36d0
Compare
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
be6e9bf to
44375bc
Compare
9ccc639 to
dee6bac
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The R/reticulate trace-state integration has an unresolved correctness issue and needs runtime validation.
1 open finding
4 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
dee6bac to
e629b2f
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Trace propagation crosses R, Python middleware, and streaming paths that require human runtime integration validation.
2 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
e629b2f to
234bbcf
Compare
f6a4ac9 to
ba57292
Compare
ba57292 to
fa97ca3
Compare
4adf0e4 to
a75c407
Compare



(MCC-1554792 - R Lib - Trace ID Propagation)
Why: Users need the trace ID for support reports, without changing return types.
Changes