-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Plugin support #1396
Plugin support #1396
Changes from all commits
a91a30a
f6d5963
f1c38d9
fb2703d
4b984b5
86aeafa
946f88c
a82e894
af0ae4a
dcc2a42
aecb281
4f36ff0
bea978a
b938693
a43fe5d
9dbc2f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,16 +39,19 @@ import ( | |
"github.com/spf13/afero" | ||
"github.com/spf13/cobra" | ||
"github.com/spf13/pflag" | ||
"golang.org/x/sync/errgroup" | ||
null "gopkg.in/guregu/null.v3" | ||
|
||
"github.com/loadimpact/k6/api" | ||
"github.com/loadimpact/k6/core" | ||
"github.com/loadimpact/k6/core/local" | ||
"github.com/loadimpact/k6/js" | ||
"github.com/loadimpact/k6/js/modules" | ||
"github.com/loadimpact/k6/lib" | ||
"github.com/loadimpact/k6/lib/consts" | ||
"github.com/loadimpact/k6/lib/types" | ||
"github.com/loadimpact/k6/loader" | ||
"github.com/loadimpact/k6/plugin" | ||
"github.com/loadimpact/k6/ui" | ||
) | ||
|
||
|
@@ -107,6 +110,37 @@ a commandline interface for interacting with it.`, | |
Left: func() string { return " init" }, | ||
} | ||
|
||
// Load plugins | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving this up from where it was. Long story short: plugins need to be loaded before the runner gets initialized, as lookup paths are going to be resolved by that time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I mention in the other comment, you shouldn't use |
||
cliConf, err := getConfig(cmd.Flags()) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
g, _ := errgroup.WithContext(context.Background()) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
plugins := []plugin.JavaScriptPlugin{} | ||
pluginNames := []string{} | ||
for _, pluginPath := range cliConf.Plugins { | ||
jsPlugin, pluginErr := lib.LoadJavaScriptPlugin(pluginPath) | ||
if pluginErr != nil { | ||
return pluginErr | ||
} | ||
|
||
// Do the part that actually takes time in a runner group. | ||
g.Go(jsPlugin.Setup) | ||
|
||
modules.RegisterPluginModules(jsPlugin.GetModules()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm we should probably emit an error if there are conflicts in plugin module names? |
||
plugins = append(plugins, jsPlugin) | ||
pluginNames = append(pluginNames, jsPlugin.Name()) | ||
} | ||
|
||
if err = g.Wait(); err != nil { | ||
return err | ||
} | ||
|
||
// Create the Runner. | ||
fprintf(stdout, "%s runner\r", initBar.String()) | ||
pwd, err := os.Getwd() | ||
|
@@ -132,10 +166,6 @@ a commandline interface for interacting with it.`, | |
|
||
fprintf(stdout, "%s options\r", initBar.String()) | ||
|
||
cliConf, err := getConfig(cmd.Flags()) | ||
if err != nil { | ||
return err | ||
} | ||
conf, err := getConsolidatedConfig(afero.NewOsFs(), cliConf, r) | ||
if err != nil { | ||
return err | ||
|
@@ -249,6 +279,7 @@ a commandline interface for interacting with it.`, | |
} | ||
|
||
fprintf(stdout, " execution: %s\n", ui.ValueColor.Sprint("local")) | ||
fprintf(stdout, " plugins: %s\n", ui.ValueColor.Sprint(strings.Join(pluginNames, ", "))) | ||
fprintf(stdout, " output: %s%s\n", ui.ValueColor.Sprint(out), ui.ExtraColor.Sprint(link)) | ||
fprintf(stdout, " script: %s\n", ui.ValueColor.Sprint(filename)) | ||
fprintf(stdout, "\n") | ||
|
@@ -473,6 +504,11 @@ a commandline interface for interacting with it.`, | |
} | ||
} | ||
|
||
// Teardown plugins | ||
for _, jsPlugin := range plugins { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we care if teardowns error out? Should we log after the output or are there repercussions for that? (ie. any tools that might start failing because output is slightly different) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. definitely log it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, please log any errors here |
||
_ = jsPlugin.Teardown() | ||
} | ||
|
||
if conf.Linger.Bool { | ||
logrus.Info("Linger set; waiting for Ctrl+C...") | ||
<-sigC | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,8 @@ | |
package modules | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/loadimpact/k6/js/modules/k6" | ||
"github.com/loadimpact/k6/js/modules/k6/crypto" | ||
"github.com/loadimpact/k6/js/modules/k6/crypto/x509" | ||
|
@@ -42,3 +44,15 @@ var Index = map[string]interface{}{ | |
"k6/html": html.New(), | ||
"k6/ws": ws.New(), | ||
} | ||
|
||
// PluginIndex holds an index of plugin module implementations. | ||
var PluginIndex = map[string]interface{}{} | ||
|
||
// RegisterPluginModules takes care of registering a map of paths that a plugin exposes so they can be | ||
// loaded from the JavaScript VM. | ||
func RegisterPluginModules(modules map[string]interface{}) { | ||
for path, impl := range modules { | ||
importPath := fmt.Sprintf("k6-plugin/%s", path) | ||
PluginIndex[importPath] = impl | ||
} | ||
Comment on lines
+54
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: I do think it will probably be easier to strip the prefix in the lookup ... although maybe the error message will need some work so it does actually include the "k6-plugin/" |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -285,6 +285,9 @@ type Options struct { | |
|
||
// Redirect console logging to a file | ||
ConsoleOutput null.String `json:"-" envconfig:"K6_CONSOLE_OUTPUT"` | ||
|
||
// Plugins to load | ||
Plugins []string `json:"plugins" envconfig:"K6_PLUGINS"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still need to figure out how this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. envconfig has issues (#671) and we want to replace it, but the problem here is not related to them... The reason you can't read from the environment is that this reading happens after you've already initialized the plugins. The "consolidated" version of And, as you can see, this "consolidation" consists of combining the {option defaults + JSON config options + the script exported Instead of |
||
} | ||
|
||
// Returns the result of overwriting any fields with any that are set on the argument. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/* | ||
* | ||
* k6 - a next-generation load testing tool | ||
* Copyright (C) 2016 Load Impact | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as | ||
* published by the Free Software Foundation, either version 3 of the | ||
* License, or (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
package lib | ||
|
||
import ( | ||
"fmt" | ||
goplugin "plugin" | ||
"runtime" | ||
|
||
"github.com/loadimpact/k6/plugin" | ||
"github.com/pkg/errors" | ||
) | ||
|
||
// LoadJavaScriptPlugin tries to load a dynamic library that should conform to | ||
// the `plug.JavaScriptPlugin` interface. | ||
func LoadJavaScriptPlugin(path string) (plugin.JavaScriptPlugin, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above. |
||
p, err := loadPlugin(path) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
jsSym, err := p.Lookup("JavaScriptPlugin") | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
var jsPlugin plugin.JavaScriptPlugin | ||
jsPlugin, ok := jsSym.(plugin.JavaScriptPlugin) | ||
if !ok { | ||
return nil, fmt.Errorf("could not cast plugin to the right type") | ||
} | ||
|
||
return jsPlugin, nil | ||
} | ||
|
||
func loadPlugin(path string) (*goplugin.Plugin, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to expand on this (maybe not necessarily for v1) to ensure that we return nice error messages for things like:
|
||
if runtime.GOOS == "windows" { | ||
return nil, errors.New("plugins are not supported in Windows") | ||
} | ||
|
||
p, err := goplugin.Open(path) | ||
if err != nil { | ||
err = fmt.Errorf("error while loading plugin: %w", err) | ||
} | ||
return p, err | ||
} |
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 wasn't sure if this was really necessary, as an empty slice is fine. I just wrote it so we could have a
panic
error handler 😃