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

Add init handler and invoke it after config get deserialized #161

Open
rucciva opened this issue Oct 21, 2023 · 6 comments
Open

Add init handler and invoke it after config get deserialized #161

rucciva opened this issue Oct 21, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@rucciva
Copy link

rucciva commented Oct 21, 2023

Sometimes, it is necesary to do some kine of initialization based on config of the plugin. Maybe invoke instanceConfig.Init() here?

@holowinski
Copy link

It would be great to have that capability.

@StarlightIbuki
Copy link
Contributor

The initialization could be achieved with a function called only once when the access handler is invoked for the first time.
Do we have some benefits to introduce a new API for this? Could this workaround work for you?
Also, I feel that we need to find another name for this as it would be confused with the init phase (and it's different both in actual behavior and implementation)

@rucciva
Copy link
Author

rucciva commented Oct 30, 2023

The initialization could be achieved with a function called only once when the access handler is invoked for the first time.
Do we have some benefits to introduce a new API for this? Could this workaround work for you?

Actually, this is what i'm doing right now as a workaround. But i think its not simple, e.g. you need to guard with mutex to prevent more than once initialization.

Another thing regarding the current structure of the api to be considered is that it is quite easy to be trapped to do initialization inside the constructor, meanwhile the configuration is not yet loaded when the constructor is invoked so the initialization would be invalid. well at least thats what happened to me at first.

Also, I feel that we need to find another name for this as it would be confused with the init phase (and it's different both in actual behavior and implementation)

PreInit() or PostConfig ?

@fffonion
Copy link
Contributor

Hi @rucciva, due to the design of PDK and because it derives from the Lua plugin design, each plugin instance (the instanceData instance you mentioned in go-pdk) must be implemented stateless. As Kong handle many connections in a non-blocking way, a (go) plugin instance could be handling lots of requests at the same time. So we are not providing this init API to prevent user to program a stateful plugin handler.
That means either the init happens in global level or "per-instance" level, you will need a mutex to access it anyway.

@rucciva
Copy link
Author

rucciva commented Oct 31, 2023

Hi @fffonion , thanks for responding, but forgive me, i'm not sure i understand your reasoning.

a (go) plugin instance could be handling lots of requests at the same time

having an init won't make the plugin unable to handle a lots of requests, it would be executed only once before the handling of any request.

So we are not providing this init API to prevent user to program a stateful plugin handler

I'm not sure what you means with "stateful". The init that i propose was to derive or extend the state that was loaded from the config passed by kong one time only. For example there is no easy way to specify dynamic object as kong's config, so one of the possible way is to specify it as json/yaml string and decode that string to get back the dynamic object.

That means either the init happens in global level or "per-instance" level

If the init is implemented and invoked here only after the config is loaded, won't that means it is "per-instance" level?

@StarlightIbuki
Copy link
Contributor

@fffonion I think what they want is pretty much like a configure handler (Kong/kong#11703). We could introduce this to Go PDK. What do you think?

@gszr gszr added the enhancement New feature or request label Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants