Skip to content

drop bad queue time events#89

Merged
richardstartin merged 1 commit intomainfrom
rgs/drop-bad-queue-time-events
Apr 30, 2024
Merged

drop bad queue time events#89
richardstartin merged 1 commit intomainfrom
rgs/drop-bad-queue-time-events

Conversation

@richardstartin
Copy link
Copy Markdown
Contributor

What does this PR do?:
Queue time events which are committed during a profiler dump may end up with null scheduler and task attributes, if they are committed just as the class map is being cleared, because the lock can't be acquired when looking up the class name. These events aren't very useful so we'll drop them for now.

Motivation:

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!

@github-actions
Copy link
Copy Markdown
Contributor

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:runner@fv-az770-114
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 Apr 29 17:47:10 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.cppflush14108
Logic errorDereference of null pointersafeAccess.hload3518
Logic errorResult of operation is garbage or undefineddwarf.hgetSLeb14225

@github-actions
Copy link
Copy Markdown
Contributor

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Warnings (5)

Style Violations (162)

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.

Do we want to track the number of dropped events as a counter or we mostly don't care?

@richardstartin
Copy link
Copy Markdown
Contributor Author

Dropping an event isn't too significant except at the trace level - it doesn't change sampling statistics, the events are latency thresholded and tend to occur in batches, so if we drop some during clearing the class map, we'll usually get some just before or after. We can revisit this if we want to change the way buffering works during dumping the recording.

@richardstartin richardstartin merged commit beb59eb into main Apr 30, 2024
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