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

Update JFR and monitoring docs #33107

Merged
merged 2 commits into from
May 15, 2023
Merged

Update JFR and monitoring docs #33107

merged 2 commits into from
May 15, 2023

Conversation

roberttoyonaga
Copy link
Contributor

Summary of Changes
May I please have a review of this PR that attempts to update and improve documentation around JFR in native Quarkus applications? During the Quarkus face-to-face it was mentioned that this documentation is in need of an update. I'll try to keep these docs up-to-date in the future as well.

  • Update docs/src/main/asciidoc/native-reference.adoc to reflect the current way of including JFR support (-Dquarkus.native.monitoring=jfr) as well as add more recent information about JFR support sorted by GraalVM version.
  • Add more information monitoring options javadoc in core/deployment/src/main/java/io/quarkus/deployment/pkg/NativeConfig.java. This should also update the generated table in the Building a Native Executable configuration reference.
  • Add JMXCLIENT and JMXSERVER monitoring options to the MonitoringOption enum. These options are available with experimental support in GraalVM for JDK 17/20
  • Add a section on how to include monitoring support in native builds in docs/src/main/asciidoc/building-native-image.adoc

@quarkus-bot
Copy link

quarkus-bot bot commented May 3, 2023

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@roberttoyonaga roberttoyonaga changed the title update JFR and monitoring docs Update JFR and monitoring docs May 3, 2023
@roberttoyonaga roberttoyonaga force-pushed the docs-monitoring-options branch 2 times, most recently from e38e226 to 6d7ef30 Compare May 3, 2023 15:33
@github-actions
Copy link

github-actions bot commented May 3, 2023 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand geoand requested review from zakkak and galderz May 7, 2023 08:50
@zakkak zakkak added the f2f label May 8, 2023
zakkak
zakkak requested changes May 8, 2023
View reviewed changes
Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

LGTM in general. Please add Mandrel to the native-reference.adoc table as well and it's good to go.

docs/src/main/asciidoc/native-reference.adoc Outdated
|===
|GraalVM Version |Supports |Limitations

|GraalVM CE 21.3
Copy link
Contributor

Choose a reason for hiding this comment

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

@roberttoyonaga can you please add Mandrel alongside like you did in the building-native-image.doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! I can add it as Mandrel 23 for now.

docs/src/main/asciidoc/building-native-image.adoc

|jmxclient
|Adds support for connections to JMX servers.
|GraalVM for JDK 17/20 Mandrel 23.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: I am still not sure whether we will go with "Mandrel 23.0" or "Mandrel for JDK 17/20" as well.

cc @jerboaa @Karm

Copy link
Member

Choose a reason for hiding this comment

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

We probably won't know for a bit, so we can leave it as it is and create an issue to revisit it once the June release comes in?

galderz
galderz approved these changes May 11, 2023
View reviewed changes
docs/src/main/asciidoc/building-native-image.adoc

|jmxclient
|Adds support for connections to JMX servers.
|GraalVM for JDK 17/20 Mandrel 23.0
Copy link
Member

Choose a reason for hiding this comment

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

We probably won't know for a bit, so we can leave it as it is and create an issue to revisit it once the June release comes in?

jerboaa
jerboaa approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

@roberttoyonaga roberttoyonaga force-pushed the docs-monitoring-options branch 3 times, most recently from 1f1fcad to 6d7ef30 Compare May 11, 2023 16:27
@roberttoyonaga roberttoyonaga force-pushed the docs-monitoring-options branch from 6d7ef30 to c3d44db Compare May 11, 2023 16:27
zakkak
zakkak approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @roberttoyonaga

@quarkus-bot
Copy link

quarkus-bot bot commented May 11, 2023

Failing Jobs - Building f2a8788

Status Name Step Failures Logs Raw logs
⌛ Native Tests - HTTP Build ⚠️ Check → Logs Raw logs
⌛ Native Tests - Security1 Build ⚠️ Check → Logs Raw logs

@zakkak zakkak merged commit 5a617cd into quarkusio:main May 15, 2023
@quarkus-bot quarkus-bot bot added this to the 3.1 - main milestone May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zakkak zakkak zakkak approved these changes

@jerboaa jerboaa jerboaa approved these changes

@galderz galderz galderz approved these changes

Assignees
No one assigned
Labels
area/core area/documentation
Projects
None yet
Milestone
3.1.0.CR1
Development

Successfully merging this pull request may close these issues.
4 participants
@roberttoyonaga @galderz @jerboaa @zakkak