[PROF-12377] Update process context support with latest version of reference library #267
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ference library **What does this PR do?** This PR upgrades the process context support with the latest version of the latest library, taken from https://github.com/DataDog/fullhost-code-hotspots-wip/tree/b33673d801b85a6c38fa0e9f1a139cb246737ce8/lang-exp/anonmapping-clib Of interest, this version of the context support library: * Introduces more fields for publishing information * Simplifies management of active contexts by moving the "keep a static with the current context" inside the library itself * Introduces a no-op version of the library, so we no longer need a `otel_process_ctx_macos.cpp` on this side On the Java side, I: * Used the Datadog names for fields when setting them -- hopefully this will avoid any confusion on which fields map to what on the OTEL side * Removed the `ReentrantReadWriteLock`, replacing it with a `synchronized`, as it's not safe to concurrently read and write the current context -- all access must be serialized. * Removed the `verifyMappingStructure` as I thought it's a bit awkward to maintain if we need to change things until we stabilize the feature. Instead, I think for now it's simpler to assume the native reader is correct. **Motivation:** Make sure Java is up-to-date with the latest improvements on the lib! Also, since I had a lot of context on the latest changes, seems easier for me to shoot out a PR for this update. **Additional Notes:** I ran `./gradlew test` but I'm not sure the tests are running properly... Am I missing something? **How to test the change?** I've updated the tests but at least with `./gradlew test` they don't seem to be running. Hopefully in CI it's all wired up?
3 tasks
jbachorik
reviewed
Aug 29, 2025
| } | ||
|
|
||
| // TODO: The serialization format is still under discussion and is not considered stable yet. | ||
| // Comments **very** welcome: Should we use JSON instead? Or protobuf? |
Collaborator
There was a problem hiding this comment.
Pretty please, stay with msgpack :)
Member
Author
There was a problem hiding this comment.
I share the same feeling haha! But OTEL seems to be json or protobuf everywhere so let's see if they'd be up for keeping msgpack here or they really want those. (In any case, the library will abstract this so the caller doesn't need to care)
jbachorik
approved these changes
Aug 29, 2025
jbachorik
added a commit
that referenced
this pull request
Sep 3, 2025
* Initial implementation of OTel process context support * Update ddprof-lib/src/main/java/com/datadoghq/profiler/OTelContext.java Co-authored-by: Ivo Anjo <ivo@ivoanjo.me> * Use the upstream ebpf-context lib sources * Fix OpenTelemetry process context implementation and build system - Add comprehensive gradle patching for otel_process_ctx.c to .cpp conversion - Add Linux preprocessor guards and C++ explicit casts for compilation - Fix gradle task dependencies and caching for proper file handling - Implement proper publish/update API usage in JNI setProcessCtx0 - Add native read functionality through JNI wrapper - Update ProcessContextTest for test resilience and native read testing - Resolve all compilation failures in gtest tasks 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * CI can't access the ebpf-context repo * Make OTelContext actually thread safe * Remove unused repo locks from build * Typo * Simplify ProcessContextTest.java * [PROF-12377] Update process context support with latest version of reference library (#267) * Optimize locking --------- Co-authored-by: Ivo Anjo <ivo@ivoanjo.me> Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR upgrades the process context support with the latest version of the latest library, taken from
https://github.com/DataDog/fullhost-code-hotspots-wip/tree/b33673d801b85a6c38fa0e9f1a139cb246737ce8/lang-exp/anonmapping-clib
Of interest, this version of the context support library:
otel_process_ctx_macos.cppon this sideOn the Java side, I:
ReentrantReadWriteLock, replacing it with asynchronized, as it's not safe to concurrently read and write the current context -- all access must be serialized.verifyMappingStructureas I thought it's a bit awkward to maintain if we need to change things until we stabilize the feature. Instead, I think for now it's simpler to assume the native reader is correct.Motivation:
Make sure Java is up-to-date with the latest improvements on the lib!
Also, since I had a lot of context on the latest changes, seems easier for me to shoot out a PR for this update.
Additional Notes:
I ran
./gradlew testbut I'm not sure the tests are running properly... Am I missing something?How to test the change?
I've updated the tests but at least with
./gradlew testthey don't seem to be running. Hopefully in CI it's all wired up?For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.