Skip to content

Improve validation checking/error handling process context C/C++ impl#88

Open
ivoanjo wants to merge 5 commits intoopen-telemetry:mainfrom
ivoanjo:ivoanjo/ref-small-improvements
Open

Improve validation checking/error handling process context C/C++ impl#88
ivoanjo wants to merge 5 commits intoopen-telemetry:mainfrom
ivoanjo:ivoanjo/ref-small-improvements

Conversation

@ivoanjo
Copy link
Copy Markdown
Contributor

@ivoanjo ivoanjo commented Mar 18, 2026

What does this PR do?

This PR collects a few more improvements to the process context C/C++ impl:

  • Add a few more size checks that were missing to make sure we stay within the limits of the embedded protobuf encoder
  • Fix for leaking the memfd file descriptor in a weird corner case
  • Make sure the no-op implementation does not need any Linux-specific headers

Motivation:

These came up as we were adding process context support to Datadog's dd-trace-java profiler in DataDog/java-profiler#414 .

Additional Notes:

I've considered if the encoder should follow the design of the decoder and always check the position of the pointer... It'd be a bigger change so I hesitated in going for it.

How to test the change?

The no-op implementation build validates the minimal set of headers is correct; for the other changes the build still passes and the example script works fine.

ivoanjo added 4 commits March 18, 2026 11:16
In practice I'm not sure `ftruncate` could fail unless the kernel is
already on fire but no reason not to fix the leak anyway.
We were accidentally importing at least one Linux-specific header,
which made NOOP not work.

To make sure NOOP keeps working, let's move everything it doesn't need
inside the ifdef -- this way it'll be very obvious what's being compiled
for NOOP and what's not.
@ivoanjo ivoanjo requested a review from a team as a code owner March 18, 2026 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants