envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.71k stars 4.75k forks source link

Shutdown hooks for factories #27990

Open maguro opened 1 year ago

maguro commented 1 year ago

It would be nice if factories had a mechanism whereby they would be notified when the Envoy server is shutting down.

kyessenov commented 1 year ago

Is lifecycle notifier not enough?

maguro commented 1 year ago

Yep, that would do it for me. Is this exposed to golang filters and extensions?

maguro commented 1 year ago

Maybe @doujiang24 may be able to answer?

doujiang24 commented 1 year ago

Is this exposed to golang filters and extensions?

@maguro Not yet. But it could be possible. Could you please describe the use case a bit? Thanks.

maguro commented 1 year ago

@doujiang24 , I was thinking

type StreamFilterFactoryV2 func(ctx context.Context, callbacks FilterCallbackHandler) StreamFilter

would be the most idiomatic.

doujiang24 commented 1 year ago

Thanks @maguro , I see that you want a hook in Go side when the Envoy server is shutting down. Why do you need the hook? for what kind of case? for some cleanup work in Go side, right?

maguro commented 1 year ago

@doujiang24 , that's correct. I had to create a bunch of stuff in init() that I'd like to tidy up and close when the plugin is unloaded. As I think on it, I think this would be more useful

type StreamFilterConfigFactoryV2 func(ctx context.Context, config interface{}) StreamFilterFactory
doujiang24 commented 1 year ago

Oh, I see, that make sense. It also could be possible, I think we could call into Go in the Dso instance destruct method: https://github.com/envoyproxy/envoy/blob/main/contrib/golang/common/dso/dso.cc#L37

@maguro Do you want to have a try? I'm happy to review.

maguro commented 1 year ago

Sure.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

doujiang24 commented 1 year ago

Oh, I think it's useful, please mark it no stale, we could implement it in the feature. @phlax Thanks