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.

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

Add FIPS-specific JMXFetch documentation #26624

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

vickenty
Copy link
Contributor

@vickenty vickenty commented Dec 5, 2024

What does this PR do? What is the motivation?

Merge instructions

Merge readiness:

  • Ready for merge

Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the <yourname>/description naming convention) and then add the following PR comment:

/merge

Additional notes

@github-actions github-actions bot added the Guide Content impacting a guide label Dec 5, 2024
Copy link
Contributor

github-actions bot commented Dec 5, 2024

Preview links (active after the build_preview check completes)

New or renamed files

Modified Files

@vickenty vickenty marked this pull request as ready for review December 11, 2024 15:17
@vickenty vickenty requested a review from a team as a code owner December 11, 2024 15:17
@brett0000FF brett0000FF self-assigned this Dec 11, 2024
Copy link
Contributor

@brett0000FF brett0000FF left a comment

Choose a reason for hiding this comment

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

Thanks for adding this guide! Let me know if you have any questions about my edits or need any help. 🙇‍♂️

@@ -0,0 +1,151 @@
# JMXFetch FIPS-140 mode
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add a new entry for this guide to content/en/integrations/guide/_index.md too? Thanks!

content/en/integrations/guide/jmxfetch-fips.md Outdated Show resolved Hide resolved
content/en/integrations/guide/jmxfetch-fips.md Outdated Show resolved Hide resolved
content/en/integrations/guide/jmxfetch-fips.md Outdated Show resolved Hide resolved
content/en/integrations/guide/jmxfetch-fips.md Outdated Show resolved Hide resolved
content/en/integrations/guide/jmxfetch-fips.md Outdated Show resolved Hide resolved
content/en/integrations/guide/jmxfetch-fips.md Outdated Show resolved Hide resolved
Comment on lines 123 to 125
See application documentation for specific instructions about how to configure JMX properties. At least the
following properties must to be set. Adjust paths as needed to point to actual location of the key and trust
store files.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
See application documentation for specific instructions about how to configure JMX properties. At least the
following properties must to be set. Adjust paths as needed to point to actual location of the key and trust
store files.
See application documentation for specific instructions about how to configure JMX properties. Add these JMX properties to your application configuration. Adjust the paths to your key and trust store files.

Can we add a link to the application documentation we're referring to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't one document to link to, JMXFetch can be used to pull metrics from any Java application. For some off-the-shelf products we have official integrations and those come with more specific instructions, for example ActiveMQ and Kafka.

Copy link
Contributor

Choose a reason for hiding this comment

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

Noted, appreciate the context.

following properties must to be set. Adjust paths as needed to point to actual location of the key and trust
store files.

```
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```
```text

Copy link
Contributor

@brett0000FF brett0000FF left a comment

Choose a reason for hiding this comment

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

@vickenty - Thanks, looks good! Heads up, I pushed a small commit where I indented the shell commands so they are inline with the numbered list.

Feel free to add this to the merge queue with /merge or ping me, and I can merge it. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Guide Content impacting a guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants