[chore] RFC: OCB Plugins/Hooks #15638
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # OCB Hooks | ||
|
|
||
| Author: @braydonk | ||
|
|
||
| AI USAGE DISCLOSURE: Some parts of the POC development leveraged Google Antigravity. The RFC contents are written entirely by me. | ||
|
|
||
| ## Abstract | ||
|
|
||
| This RFC proposes a new feature in OCB that I am calling "hooks". It introduces a plugin interface for OCB plugins managed via [Hashicorp's Go Plugin framework][Hashicorp Go Plugin] which can be configured as hooks during OCB's process for generating and compiling a distribution. As of now, there are 4 hooks available: Pre/Post Generate, and Pre/Post Compile. Each hook can be individually skipped, or both skipped when Generation/Compilation respectively are disabled. | ||
|
Member
There was a problem hiding this comment. Would it make sense to start with a smaller number of hook types for now? It would be interesting to see what use cases cover each and see if e.g. we can start only with the one(s) needed for the OBI use case
Member
There was a problem hiding this comment. Maybe good to compare with https://ftp.osuosl.org/pub/rpm/max-rpm/s1-rpm-inside-scripts.html |
||
|
|
||
| ## Proof of Concept | ||
|
|
||
| A proof of concept is [available on my Collector fork][POC]. | ||
|
|
||
| ## Example Configuration | ||
|
|
||
| The following is an example configuration that sets up a plugin that can run bash shell scripts, with pregenerate and prebuild setups: | ||
|
|
||
| ```yaml | ||
| dist: | ||
| name: otelcol-custom | ||
| description: Custom Collector Distribution | ||
| output_path: ./build | ||
| receivers: | ||
| - gomod: go.opentelemetry.io/collector/receiver/otlpreceiver v0.156.0 | ||
| exporters: | ||
| - gomod: go.opentelemetry.io/collector/exporter/nopexporter v0.156.0 | ||
| hooks: | ||
| plugins: | ||
| - name: scriptplugin | ||
| module: go.opentelemetry.io/cmd/builder/scriptplugin | ||
| version: v0.157.0 | ||
| pre_generate: | ||
| - plugin: scriptplugin | ||
| path: ./scripts/pre_generate.sh | ||
| args: | ||
| - "--verbose" | ||
|
Comment on lines
+28
to
+37
Member
There was a problem hiding this comment. Since the goal of hooks is to make running OCB as streamlined as possible (without a wrapper shell script), I wonder if we could simplify the config a tad: hooks:
pre_generate:
- gomod: go.opentelemetry.io/cmd/builder/scriptplugin v0.157.0
path: ./scripts/pre_generate.sh
args: ["--verbose"](This proposal and the above are assuming the "actions" are fairly independent and there isn't a strong use case for keeping data in memory between separate actions for the same hook.)
Contributor
Author
There was a problem hiding this comment. The goals for the setup I chose was:
2 would not be much of a downside if we fully rely on Go's module cache in a scenario where we only leverage
Member
There was a problem hiding this comment. I'm not sure I see how 2 and 3 would be an issue here. Regarding 1 however, if "running plugin subprocesses only once per OCB run" is an absolute requirement, then indeed neither of my suggestions are applicable. But my assumption is that using the same plugin in multiple steps is fairly rare, and even in those cases, the overhead of running the same binary a handful of times is nothing compared to the actual work being done in the hook (let alone the Go compilation).
Member
There was a problem hiding this comment. It'd be great to add these goals to the text of the RFC
Comment on lines
+29
to
+37
Member
There was a problem hiding this comment. Similar to the previous comment: could we pick one of these for the initial plugin support? There's a lot of overlap between the two |
||
| pre_build: | ||
| - plugin: scriptplugin | ||
| path: ./scripts/pre_build.sh | ||
| env: | ||
| BUILD_ENV: "staging" | ||
| ``` | ||
|
|
||
| A set of plugin sources are configured, which can be installed remotely via `module`/`version`, or locally via `path`. Those plugins can then be referenced by name in the different hook configurations, along with a generic configuration that will be passed to the hook. | ||
|
|
||
| ## Defining a Plugin | ||
|
|
||
| An `ocbplugin` package will be provided for plugin authors. It will feature an interface for OCB plugins to implement, as well as helpers to facilitate their plugin host setup, which automatically configures the required handshake that OCB will expect. | ||
|
|
||
| See the [Hashicorp Go Plugin][Hashicorp Go Plugin] library for more information on what these plugins generally look like, and [the example implementation of a bash script execution plugin in the POC](https://github.com/braydonk/opentelemetry-collector/tree/ocb_plugin_experiment/cmd/builder/scriptplugin). | ||
|
|
||
| OPEN QUESTION: Should this package live under `cmd/builder`? It is a bit odd for a library module to live under a `cmd` directory, but there is no `pkg` directory in `opentelemetry-collector`. | ||
|
|
||
| ## Plugin Management | ||
|
|
||
| The procedure in OCB is as follows: | ||
|
|
||
| 1. If any hooks are configured, go through the list of plugins and install them. By default the plugins will be installed into `$HOME/.ocb` (or `$(pwd)/.ocb` if user's home directory can't be resolved). Users can configure a plugin directory via the `OCB_PLUGIN_DIR` environment variable. Unless the `--reinstall-hooks` flag is provided, if any plugins are already detected as being installed it can reuse them. | ||
| 1. Hooks are run in their respective places. The first time a hook is requested, OCB will start it as a subprocess and manage an RPC client to it for the rest of the generation/compilation process. If any plugin is incapable of being started for any reason, such as requesting an unsupported hook action, failing the handshake, or just not serving RPCs properly at all, it will fail the full OCB process. | ||
| 1. Hooks will reuse the same plugin subprocess for each time the plugin is configured. After OCB is finished, all subprocesses are cleaned up. | ||
|
|
||
| ## OBI Plugin Example | ||
|
|
||
| Within the POC I also [implemented an example OBI plugin](https://github.com/braydonk/opentelemetry-collector/tree/ocb_plugin_experiment/cmd/builder/obiplugin) and an [example config using it](https://github.com/braydonk/opentelemetry-collector/blob/ocb_plugin_experiment/cmd/builder/obiplugin/example/ocb-obi-config.yaml) that mirrors the functionality of [the `prepare-obi.sh` script in the releases repo](https://github.com/open-telemetry/opentelemetry-collector-releases/blob/145e73e8663aff5ce978ea38cf0cbd4a97017141/scripts/prepare-obi.sh) instead working as an OCB pre_generate hook. Since this feature was inspired by the need to include the OBI receiver in a unique way within OCB, I decided to implement it as part of this POC. In reality, this hook can be created and managed by the OBI SIG to work whatever way makes sense for them, and they can instruct users to use the plugin remotely via the `module` and `version`, rather than the `path` source like I have in the example config. | ||
|
|
||
| [Hashicorp Go Plugin]: https://github.com/hashicorp/go-plugin | ||
| [POC]: https://github.com/open-telemetry/opentelemetry-collector/compare/main...braydonk:opentelemetry-collector:ocb_plugin_experiment | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You had mentioned that using the Go Plugin framework makes it easier on hook authors, but after looking at the POC code I'm not entirely convinced the extra dependency has much value here, since we're typically making a single call, passing in one of four "actions" + the raw YAML config section, and getting back a potential error. An alternative could be to have hooks be simple binaries that communicate through stdin/stderr/files, with the
ocbpluginmodule containing utilities to abstract over the communication method. I may try to fork your POC to try this out if I have the time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be interested to see what you have in mind. The feeling I got was that writing our own standard for how to communicate between OCB and a valid OCB plugin binary was going to essentially be writing our own communication framework from scratch. Maybe there would be a nice way to write a library such that most of the details are abstracted away from plugin authors, but currently the only thing plugin authors explicitly have to know related to the "plugin" framework is that they need a
main()that calls ourocbplugin.Servefunction. Otherwise they're just implementing an interface with normal Go code.I'm open to the idea if you want to put something together.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a quick-and-dirty fork of your prototype on my branch (comparison with your branch, comparison with main), to demonstrate both of my suggestions:
cmd/exec, passing as a command line argument the path to a temporary YAML file containing the action (pre-generate, etc.) and the hook's YAML configmod/versionis replaced by a singlegomodconfig. Thepathconfig was renamedgopathto avoid interfering with thescriptplugin's own config, whether this is the right way to avoid this conflict can be bikeshed later I think.Part of the +150 -537 diff was me just deleting tests that no longer compile, but despite that I think it overall simplifies things. I've tested the prototype with this basic config:
(I tried testing the downloading of remote plugins, but I couldn't get pseudoversions to work like I wanted for this unfortunately. I'm not sure if you necessarily need a
maindirectory to define a new binary?)