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

Different plugins depend on the same plugin, Will the common plugin Init repeatedly? and how to avoid that? #432

Open
shuaizhilin opened this issue Jan 20, 2021 · 1 comment
Labels

Comments

@shuaizhilin
Copy link

Here is a question:

- Different plugins depend on the same plugin, Will the common plugin Init repeatedly?

I try it with a new test case like this:

func TestMultiPluginsDepsSamePlugin(t *testing.T) {
	RegisterTestingT(t)
	CommonPlugin := TestPlugin{
		PluginName:         "CommonPlugin",
	}
	plugin1 := PluginOneDep{
		PluginName:         "Plugin1",
		Plugin1: MissignCloseMethod{},
		Plugin2: CommonPlugin,
	}
	plugin2 := PluginTwoLevelDeps{
		PluginName:         "Plugin2",
		PluginTwoLevelDep1: plugin1,
		PluginTwoLevelDep2: CommonPlugin,
	}
	a := agent.NewAgent(agent.AllPlugins(&plugin1, &plugin2))
	Expect(a).ToNot(BeNil())
	Expect(a.Options()).ToNot(BeNil())
	Expect(a.Options().Plugins).ToNot(BeNil())
	logrus.DefaultLogger().Info(len(a.Options().Plugins))
	for _, plugin := range a.Options().Plugins {
		logrus.DefaultLogger().Info(plugin.String())
	}
}

The result show the agent.opts.Plugins has 10 plugins and has repeat CommonPlugin
image

Then I review the code of Start(), there isn't any code to avoid calling plugin.Init() repeatly. Is it necessary to avoid repeat
initialization?

@ondrej-fabry
Copy link
Member

ondrej-fabry commented Jan 20, 2021

You should use pointers for the plugin instances. That way the plugin lookup will be able to recognize that it's same instance:

Also try using single parameter in AllPlugins. You can use dummy base plugin that will contain other top-level plugins as fields.

// ...
plugin1 := &PluginOneDep{ /* ... */ }
plugin2 := &PluginTwoDep{ /* ... */ }
base := struct {
	Plugin1 *PluginOneDep
	Plugin2 *PluginTwoDep
} {
    plugin1, plugin2,
}
a := agent.NewAgent(agent.AllPlugins(base))
// ...

I have to agree that this is not very clear behaviour and I will label this as a bug to be fixed. Thanks for reporting it @shuaizhilin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants