spring-cloud/spring-cloud-function

Introduce annotation for removing functional beans from being considered for function registry

elefeint opened this issue · 5 comments

elefeint commented

Consider introducing a new annotation, @DisableFunctionalAutodiscovery, that autoconfiguration providers could use to prevent infrastructural functional beans from being surfaced in the function registry.

Function autodiscovery is very convenient for user applications, but could lead to surprising behavior, in which Spring Cloud Function autodiscovery stops working when an unrelated dependency is added to the project. Spring projects that introduce conflicting autoconfiguration are best positioned to exclude the problematic beans by pulling Spring Cloud Function as a compile-time dependency and adding the @DisableFunctionalAutodiscovery annotation. And we'd love to keep developers from spending time on unnecessary troubleshooting caused by our projects 😩 .

Related issues: #409, #417, spring-attic/spring-cloud-gcp#1997.

artembilan commented

Huh? Why not spring.cloud.function.auto-discovery.enabled instead?

Annotations are not so flexible as configuration properties. You can have them conditionally in different environments, but with annotations you are stuck with compile time condition and nothing you can do afterwards.

That is an opinion in regards conditional features in the project.

As @olegz explained in the #417 this not for production to rely on the auto-discovery. It is really recommended to use spring.cloud.function.definition property in production ready applications.
Such a property is really an equivalent of the requested annotation or enabled property I've just explained.

You may miss the fact that spring.cloud.function.definition can have several function bean names or even their composition.

However I see in docs something like this spring.cloud.function.scan.enabled: https://cloud.spring.io/spring-cloud-function/reference/html/spring-cloud-function.html#_function_component_scan

So, sounds like this issue is Duplicate 😉

olegz commented

@elefeint we actually did discuss it - the disabling part, but decided to opt out with that info message. But let me clarify a few points:

  1. Providing spring.cloud-function.definition property is not a workaround, rather a way to explicitly state your intention and based on our experience that is what we'll see in prod anyway. So the auto-discovery is more of a developer feature to get things going quickly. . and it is also demo friendly.
  2. Unlike previous versions, nothing is loaded into function registry, so it's always empty until the fist successful lookup at which point it will contain only that definition. So in the context of s-c-stream for example, the auto-discovery process simply attempts to determine the function name to do a lookup
  3. Also, the @DisableFunctionalAutodiscovery would do the same thing as @SpringBootApplication(exclude=ContextFactoryCatalogAutoConfiguration.class).
olegz commented

@artembilan the scan property is for totally different thing, so let's not confuse the issue

elefeint commented

@olegz

  1. I actually did not realize that specifying spring.cloud-function.definition explicitly was recommended. It's good to know that we are not depriving Cloud Pub/Sub users of important functionality.
  2. @SpringBootApplication(exclude=ContextFactoryCatalogAutoConfiguration.class) would prevent all function autodiscovery, even for user application-defined beans. @DisableFunctionalAutodiscovery would exclude one specific bean.

@artembilan the reason for annotation over configuration is that, in my mind, configuration is for user applications. At that level, the user application might as well specify the opt-in spring.cloud-function.definition property instead of messing with opt-out properties.

This issue and #417 are ideas and options. The outcome needs to be whatever makes the most sense for the Spring Cloud Function/Spring Cloud Stream direction... On Spring Cloud GCP, we can work around the issue locally.

olegz commented

I think between spring-cloud/spring-cloud-stream#1865 and few previous commits that indirectly addressed this issue, there is not much that needs to be done at the moment, at least in the scope of spring-cloud-function. In other words, the actual issue was that other frameworks hat have dependency on spring-cloud-function but had no intention on using it were being affected by its presence. For example spring-cloud-stream which at the moment supports two programming models. So if anything these type of cases must be handled by such frameworks and in the case of spring-cloud-stream it has been.