-
Notifications
You must be signed in to change notification settings - Fork 294
Documentation for stand-alone mode & plugin diagnostics #1719
Documentation for stand-alone mode & plugin diagnostics #1719
Conversation
docs/STANDALONE_PLUGIN.md
Outdated
@@ -0,0 +1,30 @@ | |||
# Standalone Plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could go with Standalone Mode
instead of Standalone Plugin
, as every plugin can be ran in standalone, and Standalone Plugin
kinda indicates that it's some special kind of plugin.
docs/STANDALONE_PLUGIN.md
Outdated
|
||
Standalone plugin is a plugin which can be launched on different machine than Snap daemon (`snapteld`). | ||
|
||
To run plugin in standalone mode it is needed to launched plugin with `--stand-alone` flag, e.g.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to launch
instead to launched
b0530ca
to
95f00a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -0,0 +1,114 @@ | |||
# Plugin Diagnostics | |||
|
|||
Plugin diagnostics provides a simple way to verify that a plugin is capable of running without requiring the plugin to be loaded and a task started. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@katarzyna-z, could we simplify this sentence to be more clear - I will think about some proposal of that. Also, plugin-diagnostics is only collector plugin feature - that should be mentioned somewhere.
docs/PLUGIN_DIAGNOSTICS.md
Outdated
|
||
Plugin diagnostics provides a simple way to verify that a plugin is capable of running without requiring the plugin to be loaded and a task started. | ||
|
||
It is possible to run plugin diagnostics without any arguments, e.g.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add here notice that if the plugin has some configuration needed, the defaults will be taken.
docs/PLUGIN_DIAGNOSTICS.md
Outdated
``` | ||
|
||
Plugin diagnostics delivers following information: | ||
- Runtime details, e.g.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@katarzyna-z, what do you think about listing all information one by one, and below provide just exemplary output from plugin diagnostics?
Plugin diagnostics delivers following information:
- runtime details (plugin name, version, etc.)
- configuration details
- list of metrics expose by the plugin
- metrics with their values that can be collected right now
- and times required to retrieve this information
Example:
$ ./snap-plugin-collector-psutil
Runtime Details:
PluginName: psutil, Version: 14
RPC Type: gRPC, RPC Version: 1
Operating system: linux
Architecture: amd64
Go version: go1.7
printRuntimeDetails took 11.878µs
Config Policy:
NAMESPACE KEY TYPE REQUIRED DEFAULT MINIMUM MAXIMUM
intel.psutil.disk mount_points string false
printConfigPolicy took 67.654µs
Metric catalog will be updated to include:
Namespace: /intel/psutil/load/load1
Namespace: /intel/psutil/load/load5
Namespace: /intel/psutil/load/load15
Namespace: /intel/psutil/cpu/*/softirq
Namespace: /intel/psutil/cpu/cpu-total/softirq
Namespace: /intel/psutil/cpu/*/steal
Namespace: /intel/psutil/cpu/cpu-total/steal
Namespace: /intel/psutil/cpu/*/guest_nice
Namespace: /intel/psutil/cpu/cpu-total/guest_nice
Namespace: /intel/psutil/cpu/*/user
Namespace: /intel/psutil/cpu/cpu-total/user
printMetricTypes took 504.483µs
Metrics that can be collected right now are:
Namespace: /intel/psutil/load/load1 Type: float64 Value: 1.48
Namespace: /intel/psutil/load/load5 Type: float64 Value: 1.68
Namespace: /intel/psutil/load/load15 Type: float64 Value: 1.65
Namespace: /intel/psutil/cpu/cpu0/softirq Type: float64 Value: 20.36
Namespace: /intel/psutil/cpu/cpu1/softirq Type: float64 Value: 13.62
Namespace: /intel/psutil/cpu/cpu2/softirq Type: float64 Value: 9.96
Namespace: /intel/psutil/cpu/cpu3/softirq Type: float64 Value: 3.6
Namespace: /intel/psutil/cpu/cpu4/softirq Type: float64 Value: 1.42
Namespace: /intel/psutil/cpu/cpu5/softirq Type: float64 Value: 0.69
Namespace: /intel/psutil/cpu/cpu6/softirq Type: float64 Value: 0.54
Namespace: /intel/psutil/cpu/cpu7/softirq Type: float64 Value: 0.31
Namespace: /intel/psutil/cpu/cpu-total/softirq Type: float64 Value: 50.52
Namespace: /intel/psutil/cpu/cpu0/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu1/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu2/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu3/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu4/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu5/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu6/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu7/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu-total/steal Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu0/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu1/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu2/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu3/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu4/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu5/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu6/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu7/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu-total/guest_nice Type: float64 Value: 0
Namespace: /intel/psutil/cpu/cpu0/user Type: float64 Value: 2650.97
Namespace: /intel/psutil/cpu/cpu1/user Type: float64 Value: 2631.81
Namespace: /intel/psutil/cpu/cpu2/user Type: float64 Value: 2523.45
Namespace: /intel/psutil/cpu/cpu3/user Type: float64 Value: 2500.99
Namespace: /intel/psutil/cpu/cpu4/user Type: float64 Value: 2039.26
Namespace: /intel/psutil/cpu/cpu5/user Type: float64 Value: 2082.44
Namespace: /intel/psutil/cpu/cpu6/user Type: float64 Value: 2054.43
Namespace: /intel/psutil/cpu/cpu7/user Type: float64 Value: 1901.43
Namespace: /intel/psutil/cpu/cpu-total/user Type: float64 Value: 18384.81
printCollectMetrics took 7.470091ms
showDiagnostics took 8.076025ms
docs/PLUGIN_DIAGNOSTICS.md
Outdated
``` | ||
## Note | ||
|
||
For now plugin diagnostics works only for collector plugins. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, now I find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to specify that diagnostics only works with plugins written using one of the plugin libs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, should we specify that diagnostics works with both collectors and streaming collectors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kjlyon Great ideas! It is important that plugin must be written using one of our libs and diagnostics works also for streaming collectors.
docs/STANDALONE_MODE.md
Outdated
$ snaptel plugin load http://localhost:8182 | ||
``` | ||
|
||
The rest of operations remains exactly the same as is it for regular plugins. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using regular plugins
might be misleading. Maybe the following is better: The rest of operations remains exactly the same as is it for plugins running in regular mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a couple places that could be reworded. I'm also thinking we need to add documentation about how to write a plugin that has diagnostics and stand-alone mode enabled. That would involve acknowledging the plugin libs, I don't think it needs to be more than a sentence or two. Thoughts?
docs/PLUGIN_DIAGNOSTICS.md
Outdated
``` | ||
## Note | ||
|
||
For now plugin diagnostics works only for collector plugins. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to specify that diagnostics only works with plugins written using one of the plugin libs.
docs/PLUGIN_DIAGNOSTICS.md
Outdated
``` | ||
## Note | ||
|
||
For now plugin diagnostics works only for collector plugins. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, should we specify that diagnostics works with both collectors and streaming collectors?
docs/PLUGIN_DIAGNOSTICS.md
Outdated
Namespace: /intel/psutil/cpu/cpu6/user Type: float64 Value: 2054.43 | ||
Namespace: /intel/psutil/cpu/cpu7/user Type: float64 Value: 1901.43 | ||
Namespace: /intel/psutil/cpu/cpu-total/user Type: float64 Value: 18384.81 | ||
[...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we shorten this list? I think we can say ...
much sooner and users will get the idea without having to list all these metrics.
docs/STANDALONE_MODE.md
Outdated
@@ -0,0 +1,30 @@ | |||
# Standalone mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag currently is --stand-alone
with the hyphen. In this document standalone
is used. These should match. For usability I think the flag should change to one word, but for ease, it makes more sense to just change the documentation to match the flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that it is good idea to change now --stand-alone
flag to --standalone
because some of our plugins have been update to new versions of plugin libs to provide diagnostics information. I think that in documentation I can change "stanalone mode" to "stand-alone mode". @kjlyon what do you think?
In the future if someone decide to change flag he/she should also take into account that it is needed to update all plugins which use old versions of snap-plugin-libs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree.
docs/STANDALONE_MODE.md
Outdated
@@ -0,0 +1,30 @@ | |||
# Standalone mode | |||
|
|||
Standalone mode enables plugin launching on different machine than Snap daemon (`snapteld`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than launching
, I think start
would be a better word. Like this:
Stand-alone mode enables plugins to start and run on a different machine than Snap daemon (`snapteld`).
docs/STANDALONE_MODE.md
Outdated
|
||
Standalone mode enables plugin launching on different machine than Snap daemon (`snapteld`). | ||
|
||
To run plugin in standalone mode it is needed to launch plugin with `--stand-alone` flag, e.g.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reword: To run a plugin in stand-alone mode, you must start it with the `--stand-alone` flag, e.g.:
docs/STANDALONE_MODE.md
Outdated
``` | ||
$ ./snap-plugin-collector-psutil --stand-alone | ||
``` | ||
Plugin in standalone mode creates HTTP server for communication with Snap framework, by default plugin listens on `8182` port. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reword: A plugin running in stand-alone mode will create a HTTP server for communication with the Snap framework. By default the plugin listens on port `8182`.
docs/STANDALONE_MODE.md
Outdated
``` | ||
Plugin in standalone mode creates HTTP server for communication with Snap framework, by default plugin listens on `8182` port. | ||
|
||
It is possible to configure different port for communication with Snap framework using `--stand-alone-port` flag, e.g.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reword: To specify a different listening port, use the `--stand-alone-port` flag, e.g.:
docs/STANDALONE_MODE.md
Outdated
$ ./snap-plugin-collector-psutil --stand-alone --stand-alone-port 8183 | ||
``` | ||
|
||
To load plugin in standalone mode it is required to provide URL to indicate a machine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this section should have its own header so that it stands out more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kjlyon Do you have any proposition of the header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like Loading a plugin in snapteld
or simply, Loading a plugin
docs/STANDALONE_MODE.md
Outdated
$ ./snap-plugin-collector-psutil --stand-alone --stand-alone-port 8183 | ||
``` | ||
|
||
To load plugin in standalone mode it is required to provide URL to indicate a machine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible reword: When loading a stand-alone plugin into snapteld you must provide a URL to idicate which machine the plugin is running (IP address/hostname with port number), e.g.:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kjlyon thanks!
I'm not sure if this is the right location for this, but in the documentation somewhere we should mention that if the connection between snapteld and the plugin running in stand-alone mode is disrupted it will try to reconnect. If it is unable to reconnect the plugin will be unloaded, metric catalog updated, and any dependent tasks will stop. |
According to the last comment:
I agree. There might be added notice about known issue with brief description and link to the GH issue (1697) |
There is nothing special to implement in plugin to enable diagnostics and standalone mode. Diagnostics and standalone mode work for any plugin which uses one of our libs and in my opinion this information will be enough. |
I agree, this information is enough, but that we should add a sentence in each of these documents saying that plugins using our libs will have these features. |
4fe6e17
to
3dfa51d
Compare
@kjlyon @IzabellaRaulin @kdembler I've updated this PR. Please take a look at this once again. Let me know if I forgot about something ;) |
5bca749
to
6b992c4
Compare
docs/PLUGIN_DIAGNOSTICS.md
Outdated
Plugin diagnostics delivers following information: | ||
- runtime details (plugin name, version, etc.) | ||
- configuration details | ||
- list of metrics expose by the plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in exposed
docs/PLUGIN_DIAGNOSTICS.md
Outdated
$ ./snap-plugin-collector-psutil --config {\"mouting_points\":\"/dev/sda\"} | ||
``` | ||
|
||
For example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could be changed to Example output:
, but it's just a suggestion.
75c038f
to
1a3a7cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
docs/PLUGIN_DIAGNOSTICS.md
Outdated
``` | ||
or with plugin configuration in the form of a JSON: | ||
``` | ||
$ ./snap-plugin-collector-psutil --config {\"mouting_points\":\"/dev/sda\"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@katarzyna-z, could you check the way how to provide the config? Ssee my example below, but please verify that. The another thing is that the psutil collector does not have mouting_points
config param, but mount_points
, so it's better to use config's name which really exists
$ ./snap-plugin-collector-psutil --config '{"mount_points":"/dev/sda"}'
docs/STAND-ALONE_MODE.md
Outdated
``` | ||
## Loading a plugin | ||
When loading a plugin in stand-alone mode into `snapteld` you must provide a URL to indicate which | ||
machine the plugin is running (IP address/hostname with port number), e.g.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider rephrasing
When loading a plugin in stand-alone mode into
snapteld you must provide a URL to indicate which machine the plugin is running (IP address/hostname with port number), e.g.:
to
To load a plugin in stand-alone mode, provide a URL to indicate to the machine on which the plugin is running (IP address/hostname with port number), e.g.:
docs/STAND-ALONE_MODE.md
Outdated
## Notice | ||
|
||
If there is any disruption in the connection between Snap and a stand-alone plugin then the task is disabled and the plugin is unloaded, | ||
see [github issue](https://github.com/intelsdi-x/snap/issues/1697). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about renaming the section to "Known issues" instead of "Notice" and rephrasing its content:
If some disruption occurs in the connection between Snap and a stand-alone plugin, the running task will be stopped with disabled status and the plugin will be unloaded. Providing the mechanism of reconnecting stand-alone plugins upon network disruption is in our scope, addressed by issue #1697.
@katarzyna-z, I created a PR to your branch with addressed my comments - katarzyna-z#1. Besides these, LGTM |
Addressed comments to docs stand alone and diagnostics
Fixes #1687
Summary of changes:
Please check if I covered all information related to standalone plugin and plugin diagnostics from user perspective.
I'm open for your suggestion related to this docs 🐢
@intelsdi-x/snap-maintainers