Skip to content

JVM Flags access API - #149

Merged
jbachorik merged 15 commits into
mainfrom
jb/jvm_flag_access
Nov 12, 2024
Merged

JVM Flags access API#149
jbachorik merged 15 commits into
mainfrom
jb/jvm_flag_access

Conversation

@jbachorik

@jbachorikjbachorik commented Nov 4, 2024

Copy link
Copy Markdown
Collaborator

What does this PR do?:
It extracts the profiling-independent part of the 'libjavaProfiler' library Java API such that it might be used even if the profiler itself is not used.
It adds a new API for direct access to JVM flags.

Motivation:
The main reason is not be dependent on JMX to eg. read the value of -XX:ErrorFile etc.

Additional Notes:

How to test the change?:
Existing unit/integration tests are passing and added unit tests for the JVM flag access are passing as well.

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: PROF-10836

Unsure? Have a question? Request a review!

@github-actions

github-actionsBot commented Nov 4, 2024

Copy link
Copy Markdown
Contributor

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Warnings (6)

Style Violations (177)

@github-actions

github-actionsBot commented Nov 4, 2024

Copy link
Copy Markdown
Contributor

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:runner@fv-az1535-492
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 clean all
Clang Version:Ubuntu clang version 14.0.0-1ubuntu1.1
Date:Mon Nov 11 13:45:04 2024

Bug Summary

Bug TypeQuantityDisplay?
All Bugs6
Logic error
Assigned value is garbage or undefined1
Dereference of null pointer3
Result of operation is garbage or undefined1
Unused code
Dead nested assignment1

Reports

Bug GroupBug Type ▾FileFunction/MethodLinePath Length
Logic errorAssigned value is garbage or undefineddwarf.cppparseInstructions24420
Unused codeDead nested assignmentvmStructs.cppcheckNativeBinding9391
Logic errorDereference of null pointersafeAccess.hload3318
Logic errorDereference of null pointersymbols_linux.hElfParser12932
Logic errorDereference of null pointerflightRecorder.cppflush15048
Logic errorResult of operation is garbage or undefinedvmStructs.cppfind83416

@jbachorik
jbachorikforce-pushed the jb/jvm_flag_access branch 2 times, most recently from 4c23fa5 to 02f6919CompareNovember 5, 2024 11:26
@jbachorikjbachorik changed the title WIPJVM Flags access APINov 5, 2024
@jbachorik
jbachorik marked this pull request as ready for review November 5, 2024 11:48
@jbachorik
jbachorikforce-pushed the jb/jvm_flag_access branch 3 times, most recently from ef505df to b3ba0f9CompareNovember 6, 2024 10:16

@MattAlpMattAlp 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.

Need to verify a few things with this PR and its interaction with dd-trace-java, will follow-up & approve if no troubles emerge.

/**
* Flags interface to access JVM flags.
* In general, the flags are read-only. However, some flags can be modified at runtime.
* Currently, only string and boolean flags can be modified. Allowing modification of numeric

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 would specify as-of details here

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Sorry, I don't understand - what should I specify here?

Comment threadddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java Outdated
@jbachorik
jbachorik marked this pull request as draft November 6, 2024 16:59
Comment threadddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java Outdated
Comment threadddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java Outdated
jbachorikand others added 2 commits November 8, 2024 18:21
….java
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
…est.java
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
@jbachorik

Copy link
Copy Markdown
CollaboratorAuthor

@MattAlp I have decoupled the library initialization from the profiler initialization. Now, when you don't use profiler only the most important parts will be initialized - basically just the JNI bindings, vmstructs and safeacees. Anything related to JVMTI, capabilities, hooks etc. is registered only when one asks for JavaProfiler.instance()

@jbachorik
jbachorik marked this pull request as ready for review November 11, 2024 14:42
@jbachorik
jbachorik merged commit 82a3272 into mainNov 12, 2024
@jbachorik
jbachorik deleted the jb/jvm_flag_access branch November 12, 2024 15:23
@github-actionsgithub-actionsBot added this to the 1.18.0 milestone Nov 12, 2024
Sign up for freeto 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

@jbachorik@MattAlp