-
Notifications
You must be signed in to change notification settings - Fork 10
Initial implementation of OTel process context support #266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Looks like it was very straightforward to wire this in, which was the point :)
Left a few notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I can see why having a no-op fallback may be useful for implementers. I'll borrow steal build on this idea and add it on the reference implementation so we don't need to maintain it separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: https://github.com/DataDog/fullhost-code-hotspots-wip/pull/2 now has a no-op implementation
ddprof-lib/src/main/java/com/datadoghq/profiler/OTelContext.java
Outdated
Show resolved
Hide resolved
ddprof-lib/src/main/java/com/datadoghq/profiler/OTelContext.java
Outdated
Show resolved
Hide resolved
ddprof-lib/src/main/java/com/datadoghq/profiler/OTelContext.java
Outdated
Show resolved
Hide resolved
ddprof-test/src/test/java/com/datadoghq/profiler/context/ProcessContextTest.java
Outdated
Show resolved
Hide resolved
ddprof-test/src/test/java/com/datadoghq/profiler/context/ProcessContextTest.java
Outdated
Show resolved
Hide resolved
ddprof-test/src/test/java/com/datadoghq/profiler/context/ProcessContextTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Ivo Anjo <[email protected]>
- 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 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM left a few small notes, but overall looks very reasonable!
ddprof-test/src/test/java/com/datadoghq/profiler/context/ProcessContextTest.java
Outdated
Show resolved
Hide resolved
ddprof-test/src/test/java/com/datadoghq/profiler/context/ProcessContextTest.java
Outdated
Show resolved
Hide resolved
ddprof-test/src/test/java/com/datadoghq/profiler/context/ProcessContextTest.java
Outdated
Show resolved
Hide resolved
ddprof-test/src/test/java/com/datadoghq/profiler/context/ProcessContextTest.java
Outdated
Show resolved
Hide resolved
ddprof-lib/src/main/java/com/datadoghq/profiler/OTelContext.java
Outdated
Show resolved
Hide resolved
ddprof-lib/src/main/java/com/datadoghq/profiler/OTelContext.java
Outdated
Show resolved
Hide resolved
I've opened #267 to bring the latest version of the reference library on top of this PR/branch :) |
…ference library (#267)
@zhengyu123 I addressed the locking. If you could re-review and potentially approve ... |
What does this PR do?:
It adds the support for publishing process context in OTel friendly way.
Additional Notes:
See https://github.com/DataDog/fullhost-code-hotspots-wip/pull/2
How to test the change?:
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance
.Unsure? Have a question? Request a review!