Update use of `published_at_ns` in process context C/C++ implementation + some small fixes by ivoanjo · Pull Request #66 · open-telemetry/sig-profiling

ivoanjo · 2026-03-05T13:53:39Z

What does this PR do?

This PR collects a number of fixes to the process context C/C++ implementation. I recommend reviewing it commit-by-commit (and let me know if this is too annoying to review, I can break it down further).

The changes around "MFD_NOEXEC_SEAL on failure" and "Change publish synchronization to use timestamp instead of signature" I haven't yet pushed the change to the upstream spec, I'll do that next, but I wanted to have a working version first :)

Motivation:

Improve implementation/spec :)

Additional Notes:

N/A

How to test the change?

The built-in decoder and otel_process_ctx_dump.sh have been updated to match.

This variant happens when `memfd_create` fails and we fall back to an anonymous mapping. See [upstream spec](open-telemetry/opentelemetry-specification@25f2ea7) for more details.

The previous naming was mostly a leftover of older versions of the spec.

These `extra_attributes` are for providing attributes that are not part of the Resource. See `process_context.proto` and open-telemetry/opentelemetry-specification#4719 for details.

MFD_NOEXEC_SEAL is a newer Linux flag; kernels that don't support it return EINVAL because unused bits in flags must be 0. On first failure, retry with only MFD_CLOEXEC | MFD_ALLOW_SEALING so older kernels can still use memfd for the OTEL process context.

This ensures the reader doesn't get confused.

With the introduction of `update`, we decided to use `published_at_ns` as the synchronization field for readers. But, for publish, the signature was instead doing that work. To simplify readers, let's always use the `published_at_ns` to synchronize both on `publish` as well as `update`.

…ntext **What does this PR do?** This PR adds support for the `threadlocal.schema_version` and `threadlocal.attribute_key_map` extra attributes to be used by the process context, as per https://docs.google.com/document/d/1eatbHpEXXhWZEPrXZpfR58-5RIx-81mUgF69Zpn3Rz4/edit?tab=t.bmgoq3yor67o **Motivation:** Having these keys being supported makes it easy to experiment with thread context sharing. **Additional Notes:** As discussion around the thread context evolves, it's quite likely these keys might need to change a bit still. I'm stacking this PR on top of open-telemetry#66 to avoid piling more stuff on top of that one, but they are otherwise mostly independent. Reading these extra elements is a bit verbose but since the reader code can be disabled and is mostly only for testing I guess it's still fine? **How to test the change?** As usual, the example has been updated to match!

This variant happens when `memfd_create` fails and we fall back to an anonymous mapping. See [upstream spec](open-telemetry/opentelemetry-specification@25f2ea7) for more details.

christos68k

Thanks, LGTM

christos68k · 2026-03-10T13:54:06Z

 static uint64_t time_now_ns(void) {
  struct timespec ts;
  if (clock_gettime(CLOCK_REALTIME, &ts) == -1) return 0;
  return ts.tv_sec * 1000000000ULL + ts.tv_nsec;
 }


I just saw that you updated the specification in the OTEP to dictate a monotonic clock. As a follow-up, we should also switch to a monotonic clock here.

There are two main ways to go about it (which IIRC we haven't previously clarified):

Switch CLOCK_REALTIME to CLOCK_BOOTTIME or CLOCK_MONOTONIC

Calculate a delta between monotonic and realtime clock (once, on startup, fixed for lifetime of process) and add it to each subsequent monotonic measurement (from CLOCK_BOOTTIME or CLOCK_MONOTONIC).

1 gives us a monotonic counter but loses the correlation with a realtime clock as the resulting nanosecond values have no specified base (starting point).

2 gives us a monotonic counter which can also be interpreted as nanoseconds-since-the-epoch.

For an example of 2 in the eBPF profiler, see here.

No need to do this in this PR, whatever you prefer.

Ah, right, I called it out in open-telemetry/opentelemetry-specification#4719 (comment) and I meant to open a PR with this as follow-up to avoid piling on this one that had already been reviewed

Thanks. I went ahead with the merge since you said it's not blocking for this PR, but I'm sure @ivoanjo will take a look shortly.

So the change I already have locally and was going to open a PR with next used approach 1, not 2....

There are two main ways to go about it (which IIRC we haven't previously clarified):
...

No need to do this in this PR, whatever you prefer.

...and the reason I'm slightly leaning towards 1, because I think if we want 2, we should update the spec as well to effectively codify there the approach you've linked to from the eBPF profiler.

Otherwise, if only some implementations do 2, then it's harder to rely on the value being something sane.

I guess let me know your thoughts either here or on that follow-up PR and we can adjust :)

Follow-up PR is in #79

…ntext **What does this PR do?** This PR adds support for the `threadlocal.schema_version` and `threadlocal.attribute_key_map` extra attributes to be used by the process context, as per https://docs.google.com/document/d/1eatbHpEXXhWZEPrXZpfR58-5RIx-81mUgF69Zpn3Rz4/edit?tab=t.bmgoq3yor67o **Motivation:** Having these keys being supported makes it easy to experiment with thread context sharing. **Additional Notes:** As discussion around the thread context evolves, it's quite likely these keys might need to change a bit still. I'm stacking this PR on top of open-telemetry#66 to avoid piling more stuff on top of that one, but they are otherwise mostly independent. Reading these extra elements is a bit verbose but since the reader code can be disabled and is mostly only for testing I guess it's still fine? **How to test the change?** As usual, the example has been updated to match!

…ntext (#67) **What does this PR do?** This PR adds support for the `threadlocal.schema_version` and `threadlocal.attribute_key_map` extra attributes to be used by the process context, as per https://docs.google.com/document/d/1eatbHpEXXhWZEPrXZpfR58-5RIx-81mUgF69Zpn3Rz4/edit?tab=t.bmgoq3yor67o . I can't stack this PR on top of #66 because both of the branches are on my fork, but only the last commit (implementing thread context) differs between them. Once #66 is merged a quick rebase should clean things up. **Motivation:** Having these keys being supported makes it easy to experiment with thread context sharing. **Additional Notes:** As discussion around the thread context evolves, it's quite likely these keys might need to change a bit still. Reading these extra elements is a bit verbose but since the reader code can be disabled and is mostly only for testing I guess it's still fine? **How to test the change?** As usual, the example has been updated to match!

…` in process context C/C++ impl **What does this PR do?** This PR updates the process context C/C++ implementation to match the change in the upstream specification in open-telemetry/opentelemetry-specification@15256ee to adopt a monotonic clock. **Motivation:** Using a monotonic clock means that readers can't get confused by clocks going backward, and we can trivially about the ABA issue (because if the clock can't ever go back, we can always add 1 to a previous read to ensure no two timestamps are ever the same). **Additional Notes:** In open-telemetry#66 (comment) we discussed that this could be implemented by directly using the `CLOCK_MONOTONIC`, or alternatively we could map the monotonic clock back to a regular ns from epoch. I had this commit already ready and it takes the first approach, but let's discuss? ;) **How to test the change?** In practice readers don't really need to change unless they were trying to interpret the timestamp and pretty-print it (as the bash script was).

… in process context C/C++ impl (#79) **What does this PR do?** This PR updates the process context C/C++ implementation to match the change in the upstream specification in open-telemetry/opentelemetry-specification@15256ee to adopt a monotonic clock. **Motivation:** Using a monotonic clock means that readers can't get confused by clocks going backward, and we can trivially about the ABA issue (because if the clock can't ever go back, we can always add 1 to a previous read to ensure no two timestamps are ever the same). **Additional Notes:** In #66 (comment) we discussed that this could be implemented by directly using the `CLOCK_MONOTONIC`, or alternatively we could map the monotonic clock back to a regular ns from epoch. I had this commit already ready and it takes the first approach, but let's discuss? ;) **How to test the change?** In practice readers don't really need to change unless they were trying to interpret the timestamp and pretty-print it (as the bash script was).

ivoanjo added 6 commits March 5, 2026 13:49

Add missing variant of name in /proc/<pid>/maps

22d2bb7

This variant happens when `memfd_create` fails and we fall back to an anonymous mapping. See [upstream spec](open-telemetry/opentelemetry-specification@25f2ea7) for more details.

Minor: Rename resources -> resource_attributes

de40348

The previous naming was mostly a leftover of older versions of the spec.

Add support for sharing extra_attributes

e070d35

These `extra_attributes` are for providing attributes that are not part of the Resource. See `process_context.proto` and open-telemetry/opentelemetry-specification#4719 for details.

Make sure we don't repeat the same published_at_ns

f754ee8

This ensures the reader doesn't get confused.

ivoanjo requested a review from a team as a code owner March 5, 2026 13:53

ivoanjo mentioned this pull request Mar 5, 2026

Add draft support for thread context configuration keys to process context #67

Merged

Add missing third variant of name to bash script

bb44905

This variant happens when `memfd_create` fails and we fall back to an anonymous mapping. See [upstream spec](open-telemetry/opentelemetry-specification@25f2ea7) for more details.

christos68k approved these changes Mar 9, 2026

View reviewed changes

ivoanjo mentioned this pull request Mar 10, 2026

OTEP: Process Context: Sharing Resource Attributes with External Readers open-telemetry/opentelemetry-specification#4719

Merged

5 tasks

christos68k reviewed Mar 10, 2026

View reviewed changes

felixge approved these changes Mar 10, 2026

View reviewed changes

felixge merged commit beb6a71 into open-telemetry:main Mar 10, 2026
1 check passed

ivoanjo mentioned this pull request Mar 10, 2026

Use CLOCK_BOOTTIME instead of CLOCK_REALTIME in published_at_ns in process context C/C++ impl #79

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update use of `published_at_ns` in process context C/C++ implementation + some small fixes#66

Update use of `published_at_ns` in process context C/C++ implementation + some small fixes#66
felixge merged 7 commits into
open-telemetry:mainfrom
ivoanjo:ivoanjo/thread-context-sharing-attributes

ivoanjo commented Mar 5, 2026

Uh oh!

christos68k left a comment

Uh oh!

christos68k Mar 10, 2026 •

edited

Loading

Uh oh!

ivoanjo Mar 10, 2026

Uh oh!

felixge Mar 10, 2026

Uh oh!

ivoanjo Mar 10, 2026

Uh oh!

ivoanjo Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ivoanjo commented Mar 5, 2026

Uh oh!

christos68k left a comment

Choose a reason for hiding this comment

Uh oh!

christos68k Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivoanjo Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

felixge Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ivoanjo Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ivoanjo Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

christos68k Mar 10, 2026 •

edited

Loading