Skip to content

feat(library-config-ffi): expose OTel thread-context metadata opt-in - #2622

Open
y9v wants to merge 3 commits into
mainfrom
feat/otel-thread-context-metadata-ffi
Open

y9v wants to merge 3 commits into
mainfrom
feat/otel-thread-context-metadata-ffi

Conversation

@y9v

@y9v y9v commented Oct 2, 2026

Copy link
Copy Markdown
Member

What does this PR do?

This PR adds ddog_tracer_metadata_include_otel_therad_context function, allowing C callers to include thread-context discovery metadata when storing a tracer-metadata builder.

When metadata is absent, the operation configures the tlsdesc_v1_dev schema and a key map containing datadog.local_root_span_id. Existing metadata is preserved, so repeated calls are safe.

Motivation

The agent currently cannot discover the Ruby TLS without the thread-context metadata in the OTel process context.

libdatadog already supports publishing this metadata, but the C API lacks a way to configure it. Exposing the operation lets SDKs use the existing publisher instead of maintaining their own publication workaround.

Additional Notes

The operation updates the builder; publication happens when ddog_tracer_metadata_store is called.

How to test the change?

cargo nextest run -p libdd-library-config --features otel-thread-ctx,process-context-reader
cargo nextest run -p libdd-library-config-ffi
cargo nextest run -p libdd-library-config-ffi --no-default-features
cargo test -p libdd-library-config-ffi --doc
cargo ffi-tes

@y9v y9v self-assigned this Oct 2, 2026
@y9v
y9v requested review from a team as code owners October 2, 2026 15:30
@y9v
y9v requested review from vpellan and removed request for a team October 2, 2026 15:30
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-02T15:34:08.985290Z 2db36ac PR opened
🔒 Security Review ✅ Completed 2026-10-02T15:35:49.655746Z 2db36ac PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@pr-commenter

pr-commenter Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Benchmarks

Comparison

Benchmark execution time: 2026-10-07 09:30:20

Comparing candidate commit b8d37f1 in PR branch feat/otel-thread-context-metadata-ffi with baseline commit 24680da in branch main.

📊 Benchmarking dashboard

Found 1 performance improvements and 0 performance regressions! Performance is the same for 161 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:vec_map/as_deduped_map/needs_dedup_1_in_2/8

  • 🟩 execution_time [-23.070ns; -22.793ns] or [-4.767%; -4.710%]

Benchmark execution time: 2026-10-07 09:28:46

Comparing candidate commit b8d37f1 in PR branch feat/otel-thread-context-metadata-ffi with baseline commit 24680da in branch main.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 92 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Benchmark execution time: 2026-10-07 09:26:31

Comparing candidate commit b8d37f1 in PR branch feat/otel-thread-context-metadata-ffi with baseline commit 24680da in branch main.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 89 metrics, 10 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:datadog_sample_span/parent_not_sampled_short_circuit/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+555.350%; -555.554%]

scenario:datadog_sample_span/parent_sampled_short_circuit/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+555.927%; -555.825%]

scenario:glob_matcher/ascii_case_insensitive_match/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+557.527%; -556.579%]

scenario:glob_matcher/ascii_exact_match/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+555.735%; -555.735%]

scenario:glob_matcher/ascii_exact_miss/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+560.642%; -558.051%]

scenario:glob_matcher/ascii_wildcard_backtrack_match/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+554.961%; -555.370%]

scenario:glob_matcher/ascii_wildcard_heavy_backtrack/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+553.167%; -554.528%]

scenario:glob_matcher/ascii_wildcard_question_match/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+554.961%; -555.370%]

scenario:glob_matcher/ascii_wildcard_star_match/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+544.930%; -550.680%]

scenario:glob_matcher/star_short_circuit/allocated_bytes

  • unstable execution_time [-0.000ns; +0.000ns] or [+562.199%; -558.788%]

Candidate

Omitted due to size.

Baseline

Omitted due to size.

@y9v
y9v force-pushed the feat/otel-thread-context-metadata-ffi branch from 2db36ac to b5b35f0 Compare October 2, 2026 15:32
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

📚 Documentation Check Results

⚠️ 864 documentation warning(s) found

📦 libdd-library-config-ffi - 588 warning(s)

📦 libdd-library-config - 276 warning(s)


Updated: 2026-10-07 09:59:14 UTC | Commit: a4c617d | missing-docs job results

@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🔒 Cargo Deny Results

⚠️ 1 issue(s) found, showing only errors (advisories, bans, sources)

📦 libdd-library-config-ffi - 1 error(s)

Show output
error[vulnerability]: TLS 1.3 handshake messages incorrectly accepted across encryption level boundaries
    ┌─ /home/runner/work/libdatadog/libdatadog/Cargo.lock:132:1
    │
132 │ rustls 0.23.37 registry+https://github.com/rust-lang/crates.io-index
    │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ security vulnerability detected
    │
    ├ ID: RUSTSEC-2026-0285
    ├ Advisory: https://rustsec.org/advisories/RUSTSEC-2026-0285
    ├ Rustls accepted TLS 1.3 handshake messages sent at the wrong encryption level
      when they followed a key-changing message in the same record. For example,
      a plaintext `EncryptedExtensions` message packed into the same record as the
      `ServerHello` was accepted.
      
      RFC 8446 section 5.1 requires that handshake messages do not span key changes,
      and that implementations terminate the connection with an "unexpected_message"
      alert if they do.
      
      The handshake transcript is still authenticated, so a network-position attacker
      cannot use this to alter or complete a handshake; the practical effect is that
      a peer could send handshake messages that should be encrypted in plaintext
      without rustls rejecting the connection.
      
      This is functionally the same bug as Go's
      [GO-2026-4340](https://pkg.go.dev/vuln/GO-2026-4340) (CVE-2025-61730).
    ├ Announcement: https://github.com/rustls/rustls/security/advisories/GHSA-2mjx-qc3c-rqvc
    ├ Solution: Upgrade to >=0.23.45 (try `cargo update -p rustls`)
    ├ rustls v0.23.37
      ├── hyper-rustls v0.27.7
      │   └── libdd-common v8.0.0
      │       ├── libdd-common-ffi v44.0.0
      │       │   └── libdd-library-config-ffi v0.0.2
      │       └── libdd-library-config-ffi v0.0.2 (*)
      ├── libdd-common v8.0.0 (*)
      ├── rustls-platform-verifier v0.6.2
      │   └── libdd-common v8.0.0 (*)
      └── tokio-rustls v0.26.0
          └── hyper-rustls v0.27.7 (*)

advisories FAILED, bans ok, sources ok

📦 libdd-library-config - ✅ No issues


Updated: 2026-10-07 09:59:04 UTC | Commit: a4c617d | dependency-check job results

@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Tests

✅ All CI checks and tests passed.

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 93.62%
• Overall Coverage: 80.73% (+0.05%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: b8d37f1 | Docs | View more details | Give us feedback!

@y9v
y9v force-pushed the feat/otel-thread-context-metadata-ffi branch 3 times, most recently from 11fc753 to 91d5dae Compare October 2, 2026 15:48

@yannham yannham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @ivoanjo has an opinion on this, but I wonder if we shouldn't rather publish thread context-related metadata by default, without having to do anything else on the tracer side. Otherwise most likely other tracers will have to know they need to call this additional setter. This also let us delay the "add custom key maps" operation, for which we can implement a separate FFI function if and when we need it.

If it's important to be able to disable this behavior in some cases, we can have a setter to do so, e.g. ddog_tracer_metadata_exclude_otel_thread_context. Wdyt?

Comment thread libdd-library-config-ffi/src/tracer_metadata.rs Outdated
Comment thread libdd-library-config-ffi/src/tracer_metadata.rs
@dd-octo-sts

dd-octo-sts Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Artifact Size Benchmark Report

aarch64-alpine-linux-musl
Artifact Baseline Commit Change
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.so 9.08 MB 9.08 MB 0% (0 B) 👌
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.a 96.64 MB 96.71 MB +.06% (+65.71 KB) 🔍
aarch64-unknown-linux-gnu
Artifact Baseline Commit Change
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.so 12.27 MB 12.28 MB +.01% (+1.82 KB) 🔍
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.a 108.00 MB 108.06 MB +.05% (+65.85 KB) 🔍
libdatadog-x64-windows
Artifact Baseline Commit Change
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.dll 29.15 MB 29.15 MB +.01% (+5.00 KB) 🔍
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.lib 98.35 KB 98.72 KB +.37% (+380 B) 🔍
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.pdb 192.87 MB 192.97 MB +.05% (+104.00 KB) 🔍
/libdatadog-x64-windows/debug/static/datadog_profiling_ffi.lib 818.88 MB 819.37 MB +.05% (+502.84 KB) 🔍
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.dll 9.77 MB 9.77 MB +.02% (+2.50 KB) 🔍
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.lib 98.35 KB 98.72 KB +.37% (+380 B) 🔍
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.pdb 27.62 MB 27.62 MB +.02% (+8.00 KB) 🔍
/libdatadog-x64-windows/release/static/datadog_profiling_ffi.lib 55.85 MB 55.86 MB +.02% (+13.99 KB) 🔍
libdatadog-x86-windows
Artifact Baseline Commit Change
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.dll 25.56 MB 25.57 MB +.01% (+5.00 KB) 🔍
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.lib 99.89 KB 100.26 KB +.37% (+384 B) 🔍
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.pdb 198.58 MB 198.65 MB +.03% (+64.00 KB) 🔍
/libdatadog-x86-windows/debug/static/datadog_profiling_ffi.lib 806.49 MB 807.06 MB +.07% (+583.03 KB) 🔍
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.dll 7.58 MB 7.58 MB +.01% (+1.50 KB) 🔍
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.lib 99.89 KB 100.26 KB +.37% (+384 B) 🔍
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.pdb 29.73 MB 29.74 MB +.02% (+8.00 KB) 🔍
/libdatadog-x86-windows/release/static/datadog_profiling_ffi.lib 52.76 MB 52.77 MB +.02% (+13.17 KB) 🔍
x86_64-alpine-linux-musl
Artifact Baseline Commit Change
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.a 86.53 MB 86.60 MB +.07% (+70.10 KB) 🔍
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.so 10.11 MB 10.11 MB +.07% (+8.00 KB) 🔍
x86_64-unknown-linux-gnu
Artifact Baseline Commit Change
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.a 102.45 MB 102.52 MB +.06% (+70.20 KB) 🔍
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.so 12.36 MB 12.36 MB +.04% (+5.39 KB) 🔍

@ivoanjo ivoanjo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM


Maybe @ivoanjo has an opinion on this, but I wonder if we shouldn't rather publish thread context-related metadata by default, without having to do anything else on the tracer side. Otherwise most likely other tracers will have to know they need to call this additional setter. This also let us delay the "add custom key maps" operation, for which we can implement a separate FFI function if and when we need it.

Good question! TBH no very strong opinion on this one.

The thread context by its definition needs collaboration from the caller so making this one bit slightly more automatic I'm not sure makes a big difference.

Anyway if you don't have that block, outside readers won't be able to work so it's not like you won't notice very quickly...

Comment thread libdd-library-config-ffi/src/tracer_metadata.rs
Comment thread libdd-library-config-ffi/src/tracer_metadata.rs Outdated
Comment thread libdd-library-config-ffi/src/tracer_metadata.rs Outdated

@yannham yannham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion either, so I'll let you decide what you think is best (opt-out vs explicit opt-in). There are remaining comments to acknowledge but otherwise the change is sound 👍

@ivoanjo

ivoanjo commented Oct 5, 2026

Copy link
Copy Markdown
Member

I don't have a strong opinion either, so I'll let you decide what you think is best (opt-out vs explicit opt-in). There are remaining comments to acknowledge but otherwise the change is sound 👍

Ack! We can always add it later if it looks like it's a pain point

@y9v

y9v commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

I decided to go for opt-in, since some tracers don't need this functionality

@y9v
y9v force-pushed the feat/otel-thread-context-metadata-ffi branch from bc9cf28 to b8d37f1 Compare October 7, 2026 08:59
@y9v
y9v removed the request for review from vpellan October 7, 2026 08:59
@y9v

y9v commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-10-07 09:57:06 UTC ℹ️ Start processing command /merge
Use /merge -c to cancel this operation!


2026-10-07 09:57:13 UTC ℹ️ MergeQueue: Pull request is not mergeable yet

It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.

  • Run /code blockers to see what is blocking it.
  • Run /remove to cancel it.

Use /merge -c to cancel this operation!


⏳ Waiting for pull request to become mergeable

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants