apache / apache/pulsar

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

Open
#20,483 1 comment 0 reactions 0 assignees View on GitHub
Stale type/enhancement
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

### Search before asking

- [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.

### 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](https://github.com/apache/pulsar/blob/43b3622cc7e7f746ca9920fdc704dc7448767ac7/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java#L286-L293). 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](https://github.com/apache/pulsar/blame/43b3622cc7e7f746ca9920fdc704dc7448767ac7/pulsar-function-go/conf/conf.go#L109-L117), 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?

- [X] I'm willing to submit a PR!

Contributor guide

Open the contributing guide

Research direction

Start in pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java at getGoInstanceCmd, then read pulsar-function-go/conf/conf.go where --instance-conf-path is handled. Trace how the runtime constructs the Go function command and confirm the default YAML configuration path works without breaking older functions; done means Go functions start with the intended configuration and existing compatibility concerns are addressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, java
Domain
backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.