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

Update documentation to improve visibility and avoid duplicates with Grafana documentation #277

Merged
merged 6 commits into from
Sep 17, 2021

Conversation

AgnesToulet
Copy link
Contributor

This PR does the following documentation update:

  • Move the content of the remote_rendering_using_docker.md file to improve its visibility (mainly in the new image rendering configuration section in Grafana documentation).
  • Move some installation instructions from Grafana documentation to the plugin main README.md (so it appears here: https://grafana.com/grafana/plugins/grafana-image-renderer/).

To be merged with this OSS PR: grafana/grafana#38759

Copy link
Contributor

@pianohacker pianohacker left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link

@mjseaman mjseaman left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

Added doc review.

docs/index.md Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 22 to 26
Minimum free memory recommendation is 16GB on the system doing the rendering.

Rendering images can require a lot of memory, mainly because Grafana creates browser instances in the background for the actual rendering. If multiple images are rendered in parallel, then the rendering has a bigger memory footprint. One advantage of using the remote rendering service is that the rendering will be done on the remote system, so your local system resources will not be affected by rendering.

## Plugin installation
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
Minimum free memory recommendation is 16GB on the system doing the rendering.
Rendering images can require a lot of memory, mainly because Grafana creates browser instances in the background for the actual rendering. If multiple images are rendered in parallel, then the rendering has a bigger memory footprint. One advantage of using the remote rendering service is that the rendering will be done on the remote system, so your local system resources will not be affected by rendering.
## Plugin installation
Rendering images require a lot of memory, mainly because Grafana creates browser instances in the background for the actual rendering.
We recommend a minimum of 16GB of free memory on the system rendering mages.
To render multiple images in parallel requires an even bigger memory footprint. You can use the remote rendering service where images render on the remote system, and your local system resources are not affected.
## Installation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied this with some updates as I had trouble understanding the last paragraph.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

LGTM, if we need any additional change, we can do it later. Thank you!

@AgnesToulet
Copy link
Contributor Author

Great, thanks! I'll wait for the Grafana PR to be approved to merge both PRs around the same time (as some contents were moved between the two docs and there are a lot of links between these docs).

@AgnesToulet AgnesToulet marked this pull request as ready for review September 8, 2021 07:28
@AgnesToulet AgnesToulet merged commit 541eb1d into master Sep 17, 2021
@AgnesToulet AgnesToulet deleted the docs-udpate branch September 17, 2021 06:54
neverping added a commit to neverping/helm-charts that referenced this pull request Jan 6, 2022
The image renderer plugin-in was linking to a broken documentation.

After inspecting the changes on the `grafana-image-render repository`,
I found out that the change below updated most of the documentation,
where the content of the `remote_rendering_using_docker.md` was 
moved to the main `README.md` file.

grafana/grafana-image-renderer#277
joshmeranda pushed a commit to joshmeranda/rancher-charts that referenced this pull request Sep 22, 2023
This commit updates the Grafana Helm chart documentation.

In the section about the image renderer plugin, the reference was linking to
a broken documentation in another GitHub repository.

After inspecting the changes on the `grafana-image-render` repository,
I found out that the change below updated most of the documentation,
where the content of the `remote_rendering_using_docker.md` was
moved to the main `README.md` file.

Then, this fix points towards the new reference.

Here it is the commit that changed the documentation on the remote repo.

grafana/grafana-image-renderer#277

Signed-off-by: Willian Braga <willian.braga@sybogames.com>
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.

4 participants