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

Docs: Profiling a backend plugin #996

Merged
merged 11 commits into from
Sep 3, 2024
Merged

Docs: Profiling a backend plugin #996

merged 11 commits into from
Sep 3, 2024

Conversation

marefr
Copy link
Contributor

@marefr marefr commented Jul 9, 2024

What this PR does / why we need it:

Document how to profile a backend plugin.

Which issue(s) this PR fixes:
Closes #958

Related to grafana/grafana#90048 and grafana/grafana-plugin-sdk-go#1025

Special notes for your reviewer:
Should probably add the configuration details to https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#pluginplugin_id instead of here?

@marefr marefr added type/docs Changes only affect the documentation no-changelog Don't include in changelog and version calculations labels Jul 9, 2024
@marefr marefr self-assigned this Jul 9, 2024
@marefr marefr requested a review from a team as a code owner July 9, 2024 17:34
@marefr marefr requested review from oshirohugo and removed request for a team July 9, 2024 17:34
@CLAassistant
Copy link

CLAassistant commented Jul 9, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

github-actions bot commented Jul 9, 2024

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged. It will not be considered when calculating future versions of the npm packages and will not appear in the changelogs.

Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

Content looks good to me, only a couple of minor comments


In the [Grafana configuration file](https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/) you can configure profiling under a `[plugin.<plugin ID>]` section where `<plugin ID>` is the plugin identifier of your backend plugin you want to profile, e.g. [grafana-github-datasource](https://grafana.com/grafana/plugins/grafana-github-datasource/).

Running a backend plugin with profiling enabled and without block and mutex profiling enabled should only add a fraction of overhead and is suitable for production or continuous profiling scenarios. Adding a small fraction of block and mutex profiling, such as 10-5 (10%-20%) should in general be fine, but each plugin might vary.
Copy link
Contributor

Choose a reason for hiding this comment

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

People (like myself) may not know what block and mutex mean. You probably need to do an intro to what these parameters mean and point to some documentation for more information.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see you are explaining this below, then a link to those sections should suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to improve this moving it to its own section. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks good, it might be a bit confusing to read the whole example but all the parameters are explained below so sounds good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a pointer that options are detailed in sub-sections. WDYT?

@marefr marefr requested a review from andresmgot July 11, 2024 10:29
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

LGTM

@marefr
Copy link
Contributor Author

marefr commented Jul 11, 2024

@josmperez feel free to review/take over this since I'm going to be off for 4 weeks after tomorrow.

Copy link
Contributor

@josmperez josmperez left a comment

Choose a reason for hiding this comment

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

I have committed a variety of edits, including a file rename. Please review edits. No showstoppers. LGTM.

@josmperez
Copy link
Contributor

@josmperez feel free to review/take over this since I'm going to be off for 4 weeks after tomorrow.

Done. I hope you get a chance to take a look at my commits before you go.

Copy link
Member

@xnyo xnyo left a comment

Choose a reason for hiding this comment

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

Awesome work on this! 🙌 Leaving only a few small comments :)

@marefr marefr requested review from andresmgot and xnyo August 28, 2024 10:36
Copy link
Member

@xnyo xnyo left a comment

Choose a reason for hiding this comment

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

LGTM, awesome work!

@marefr marefr merged commit 40a745e into main Sep 3, 2024
12 checks passed
@marefr marefr deleted the docs_958 branch September 3, 2024 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Don't include in changelog and version calculations type/docs Changes only affect the documentation
Projects
Development

Successfully merging this pull request may close these issues.

Docs: Profiling a backend plugin
5 participants