open-telemetry / weaver

OTel Weaver lets you easily develop, validate, document, and deploy semantic conventions
Apache License 2.0
47 stars 17 forks source link

[Feature] Plugin System Design #258

Open 0xNFANZ opened 1 month ago

0xNFANZ commented 1 month ago

Description

Firstly, really like where weaver is going. Great stuff.

As mentioned in the project readme - weaver might support a WASM-based plugin system. I am personally not familiar with WASM, or what building a custom WASM plugin might look like - and I also selfishly don't want to learn it.

A more simple, and extensible solution might be proxying to subcommands, such as how Git does.

Solution thoughts

Any binary in the host path prefixed with weaver- would be executed when run as weaver ${subcommand} args

which weaver
which weaver-gogen # Theoretical program for generating go semconv sdk that would integrate with my custom otel sdk
# returns real paths

weaver gogen --arg1="value"

Reasons

I think this way, you wouldn't be dictating which technologies, frameworks and languages an organisation would have to use. The end result would be that so long as you had an executable or alias with the right name, the behaviour is then completely up to the third-party.

It would also be much simpler to implement and maintain than anything WASM related (probably).

lquerel commented 1 month ago

Thank you for this suggestion; it is indeed an interesting avenue to explore that aligns with the goal of creating Weaver as a platform. I see a few elements to delve into:

I don't see anything insurmountable here; we just need to delve a bit more into the details.

lquerel commented 1 month ago

I also think it’s important to consider that the mechanism for resolving a semconv registry is still evolving and remains a fairly complex process. Therefore, this resolution should stay within the Weaver command itself, offloading this type of processing from subcommands, which would be unnecessary to reimplement in each command and is definitely error-prone. This makes me think of another alternative that is already supported with the current implementation:

weaver registry resolve --registry <registry-path> --format yaml | gogen

# registry-path can be a local folder, a git repo, a git archive
# policy check could also be enabled, etc.
# --format json is also supported

gogen just needs to consume a resolved registry from stdin and do whatever it wants with it.

Note: A resolved semconv registry is a self-contained registry where all the refs, extends, and overrides have been replaced/resolved by the appropriate definitions.

0xNFANZ commented 1 month ago

Great points @lquerel. I think weaver commands should take precedence over subcommand for name clashes.

Great suggestion about weaver reg resolve - hadn't thought of that.

Could we add registry_path to the weaver.yaml config?

lquerel commented 1 month ago

Adding a default_registry field in the weaver.yaml config sounds reasonable. There are some subtle things to figure out regarding the interaction between the default value of the --registry command line parameter and this config field, but it's probably feasible.

Note that by default, if the --registry parameter is not specified in the command line, the registry defaults to https://github.com/open-telemetry/semantic-conventions.git.

To better understand your needs, could you please detail the motivations behind this request?

Adding @jsuereth to this thread for awareness.