Skip to content

Add startTime to datadog.ProfilerCounter event#81

Merged
richardstartin merged 1 commit into
mainfrom
malvarez/fix-jfr-dump
Mar 26, 2024
Merged

Add startTime to datadog.ProfilerCounter event#81
richardstartin merged 1 commit into
mainfrom
malvarez/fix-jfr-dump

Conversation

@manuel-alvarez-alvarez
Copy link
Copy Markdown
Member

@manuel-alvarez-alvarez manuel-alvarez-alvarez commented Mar 25, 2024

What does this PR do?:
Fixes the generation of JFR dumps so they can be read by the jfr tool by adding startTime to datadog.ProfilerCounter event

Motivation:
The jfr tool could not open the dumps generated by the profiler, failing with:

java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Long
        at jdk.jfr.consumer.EventParser.parse(EventParser.java:62)

Additional Notes:

How to test the change?:

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.

Unsure? Have a question? Request a review!

@manuel-alvarez-alvarez manuel-alvarez-alvarez added the bug Something isn't working label Mar 25, 2024
@manuel-alvarez-alvarez manuel-alvarez-alvarez changed the title Malvarez/fix jfr dump Fix jfr dump Mar 25, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 25, 2024

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:runner@fv-az711-489
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 Mar 25 17:28:01 2024

Bug Summary

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

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Logic errorAssigned value is garbage or undefineddwarf.cppparseInstructions23520
Unused codeDead initializationlivenessTracker.cppcleanup_table451
Unused codeDead nested assignmentvmStructs.cppcheckNativeBinding8181
Logic errorDereference of null pointerflightRecorder.cppflush14118
Logic errorDereference of null pointersafeAccess.hload3518
Logic errorResult of operation is garbage or undefineddwarf.hgetSLeb14225

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 25, 2024

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Warnings (5)

Style Violations (164)

Comment thread ddprof-lib/src/main/cpp/jfrMetadata.cpp
Comment thread ddprof-lib/src/main/cpp/flightRecorder.cpp Outdated
jbachorik
jbachorik previously approved these changes Mar 25, 2024
Copy link
Copy Markdown
Collaborator

@jbachorik jbachorik left a comment

Choose a reason for hiding this comment

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

Please, update the formatting before merge

@jbachorik jbachorik dismissed their stale review March 25, 2024 10:37

Dismissing until tests are fixed

@richardstartin
Copy link
Copy Markdown
Contributor

@manuel-alvarez-alvarez we need to get to the bottom of why the tests fail on class metadata with this change, which might make this change more complex than it first seems. We've never aimed for compatibility with the JFR tool, only with JMC, but if you need this from us we can ensure support (and testing) in a reasonably short time frame. If you have spare cycles to debug this now, don't be discouraged.

@manuel-alvarez-alvarez
Copy link
Copy Markdown
Member Author

@manuel-alvarez-alvarez we need to get to the bottom of why the tests fail on class metadata with this change, which might make this change more complex than it first seems. We've never aimed for compatibility with the JFR tool, only with JMC, but if you need this from us we can ensure support (and testing) in a reasonably short time frame. If you have spare cycles to debug this now, don't be discouraged.

There's no real business need from having support for JFR (just me trying to open the dumps from IntelliJ). I took the opportunity as a weekend lecture to learn about the format of the dumps and profiling in general 😄. Don't waste your time on this, if I find some spare time I'll take it for another ride and see why the tests are failing, but if nothing comes from it we can close the PR.

@manuel-alvarez-alvarez manuel-alvarez-alvarez added enhancement New feature or request and removed bug Something isn't working labels Mar 25, 2024
@richardstartin
Copy link
Copy Markdown
Contributor

@jbachorik I'm ok with this one when you are. @manuel-alvarez-alvarez thanks for doing this!

@manuel-alvarez-alvarez manuel-alvarez-alvarez changed the title Fix jfr dump Add startTime to datadog.ProfilerCounter event Mar 25, 2024
@richardstartin richardstartin merged commit 642a8d0 into main Mar 26, 2024
@github-actions github-actions Bot added this to the 0.97.0 milestone Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants