sensu / sensu-plugin-sdk

A framework for creating Sensu plugins
MIT License
7 stars 8 forks source link

Convention over configuration #22

Open palourde opened 4 years ago

palourde commented 4 years ago

I believe we could decrease the number of decisions a plugin developer needs to do when using this SDK, by enforcing some conventions, and therefore simplify the process of creating your own plugin.

For example, the various NewGo... (e.g. sensu.NewGoHandler) require you to pass functions as arguments:

func main() {
    handler := sensu.NewGoHandler(&handler.PluginConfig, options, checkArgs, executeHandler)
    handler.Execute()
}

func checkArgs(_ *types.Event) error {}

func executeHandler(event *types.Event) error {}

I believe by adopting some conventions on the name of these functions, we could reduce the number of arguments required and therefore the complexity. For example, if we assumed that a Plugin must adhere to a given interface, we could simply do the following instead:

func main() {
    handler := sensu.NewGoHandler(&plugin)
    handler.Execute()
}

func (p *Plugin) Validate(_ *types.Event) error {}

func (p *Plugin) Execute(event *types.Event) error {}

Similarly, I don't believe declaring a PluginConfigOption should require that much attributes, e.g.

&sensu.PluginConfigOption{
            Path:      "example",
            Env:       "HANDLER_EXAMPLE",
            Argument:  "example",
            Shorthand: "e",
            Default:   "",
            Usage:     "An example configuration option",
            Value:     &handler.endpoint,
        },
    }

Instead, maybe we could simply assume sane default values (e.g. I assume the Path and the Argument could be similar) but allow those values to be overridden for edge cases.

echlebek commented 4 years ago

+1, we could probably do much better with interfaces here.

calebhailey commented 4 years ago

Love it!

jspaleta commented 4 years ago

One comment... I don't think we always want to have an cmdline Argument backed by a path. The name of the path might be a convention we can set and have it match the argument name... but we will still need a bool to indicate if path override is desired.

Similarly with Env, we could enforce a naming convention but we'd still need a bool to enable/disable the optional Env binding.

Looking more closely... we actually could allow a PluginConfigOption with a Path but no cmdline argument set if we wanted to.