Skip to content
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.

Sign up for GitHub

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

Jump to bottom

better logging on JFR max depth setting #2063

Merged
merged 2 commits into from Nov 11, 2020
Merged

better logging on JFR max depth setting #2063

merged 2 commits into from Nov 11, 2020

Conversation

pmbauer
Copy link
Member

@pmbauer pmbauer commented Nov 10, 2020

  • stop logging scary errors when we can't run diagnostic commands; we make a best-effort attempt to set the stack depth and shouldn't log a scary warning and exception when we can't
  • remove confusing log line by not explicitly setting the max stack depth when a user has already specified a valid value

@pmbauer pmbauer requested review from a team as code owners November 10, 2020 20:30
@pmbauer
JMX commands log debug on exceptions
256d957 
only log the scary exception with debug logging on
@pmbauer
skip setting when user specifies valid max depth
ce1c6eb 
avoids confusing log lines like, avoid the second line
```
INFO ... skip setting JFR.configure stackdepth, using 128
INFO ... setting JFR.configure stackdepth: Stack depth: 128
```
thegreystone
thegreystone reviewed Nov 11, 2020
View reviewed changes
Copy link

@thegreystone thegreystone left a comment •
edited

Choose a reason for hiding this comment

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

Do we also want to add a log message (without the "scary exception") along the lines of "Could not override max stack depth to X going with default", so that we can check the logs to see how common this is?

@thegreystone
Copy link

Do we also want to add a log message (without the "scary exception") along the lines of "Could not override max stack depth to X going with default", so that we can check the logs to see how common this is?

Ah, never mind. Took a closer look.

@pmbauer pmbauer merged commit 1493af5 into master Nov 11, 2020
@pmbauer pmbauer deleted the pmbauer/fix-scary-errors branch November 11, 2020 19:51
@github-actions github-actions bot added this to the 0.68.0 milestone Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thegreystone thegreystone thegreystone left review comments

@tylerbenson tylerbenson tylerbenson approved these changes

@mar-kolya mar-kolya mar-kolya approved these changes

@lpriima lpriima lpriima approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.68.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@pmbauer @thegreystone @tylerbenson @mar-kolya @lpriima