apache / pulsar

Apache Pulsar - distributed pub-sub messaging system
https://pulsar.apache.org/
Apache License 2.0
14.14k stars 3.57k forks source link

[enhancement][bug] Go functions should take yaml file instance Configuration by default #20483

Open flowchartsman opened 1 year ago

flowchartsman commented 1 year ago

Search before asking

Motivation

My team has been migrating Go functions from using the process runtime to Kubernetes, and kept having functions fail to start. Eventually we were able to identify the problem as whitespace in the shell command generated by getGoInstanceCmd. Our userConfig consists of a single conf key containing JSON for the function to parse, and somehow it was being rendered incorrectly, breaking shell parsing. We have had to start passing the value in using base64 to fix the issue.

Solution

While the root cause can probably be remediated, there's another, better solution, which is to pass the instance configuration via YAML. This can avoid cluttering the logs with huge chunks of JSON, and will help avoid situations like ours.

This method of importing config is already supported by the Go Function SDK, and has in fact been supported for four years now, though the flag enabling this behavior --instance-conf-path is not found anywhere in the source tree outside of the Go function SDK itself, meaning it is never used.

All this to say, we can be reasonably sure that it will be backwards compatible for most, if not all currently-running Go functions in the wild unless they were built more than four years ago.

Alternatives

Any other alternative would involve new code in the function SDK, and would involve rebuilding Go functions to use the new version of the framework, as well as a compatibility layer.

Anything else?

If strict backwards compatibility with Go functions based on older code is desired, it is also possible to identify versions which include older dependencies by examining the build metadata for the version of github.com/apache/pulsar/pulsar-function-go embedded in the function binary. You can see an example of this by using go version -m on any compiled pulsar function:

$  go version -m gofunc |grep -i pulsar-function-go
        dep     github.com/apache/pulsar/pulsar-function-go     v0.0.0-20230603131416-43b3622cc7e7      h1:yZTy3OvBuRpQBY7q3WezuLBBApLUhPMTsjDfHQgAeAE=

However, given that the functions would need to be build more than four year ago, it might be reasonable to assume that there won't be a lot of problems, and that covering the change in the release notes might be enough.

Are you willing to submit a PR?

github-actions[bot] commented 1 year ago

The issue had no activity for 30 days, mark with Stale label.