Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Plugin System Design #258

Open
0xNFANZ opened this issue Jul 23, 2024 · 4 comments
Open

[Feature] Plugin System Design #258

0xNFANZ opened this issue Jul 23, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@0xNFANZ
Copy link

0xNFANZ commented Jul 23, 2024

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).

@0xNFANZ 0xNFANZ changed the title Plugin System Design [Feature] Plugin System Design Jul 23, 2024
@lquerel
Copy link
Contributor

lquerel commented Jul 24, 2024

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:

  • Weaver commands are grouped by the type of objects to process, e.g., registry, schema. Thus, we have commands like weaver registry check, weaver registry generate, etc. In the future, we will have commands like weaver schema check, weaver schema generate, etc. This probably means prefixing the binaries with weaver_registry_<command> to avoid collisions.
  • Another issue I see is the potential collisions (present and future) between built-in commands (e.g., weaver registry check) and extension commands developed in this manner (e.g., $PATH/weaver_registry_check).
  • Lastly, I would also like to keep the option to integrate extensions as WASM components for the sandboxing properties they offer and to facilitate the creation of an infrastructure to download them from a public repository.

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

@lquerel
Copy link
Contributor

lquerel commented Jul 24, 2024

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
Copy link
Author

0xNFANZ commented Jul 24, 2024

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
Copy link
Contributor

lquerel commented Jul 24, 2024

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.

@jsuereth jsuereth added the enhancement New feature or request label Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants