search

open-telemetry / weaver

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

Generate attributes from files in symlinked directories #441

Closed awangc closed 2 days ago

awangc commented 3 weeks ago

Is your feature request related to a problem? Please describe. When trying to run weaver registry generate on a local path, I'd like the ability to "compose" the model/ folder via symlinking directories present in the filesystem. Currently if I do symbolic linking (ln -s) weaver does not read from the linked directories when generating the attributes.

Describe the solution you'd like An option that can be passed to weaver that would make it read sym linked folders under model/ and generate attributes

Describe alternatives you've considered Right now the only way to achieve what I'm thinking is if I copy specific directories into the model/ folder. This means if I want to keep an in-house component that need to be coalesced with semantic conventions then changes from either side need be explicitly propagated to the model/ folder, which seems a little more cumbersome than allowing changes to be propagated via symbolic links.

Leo6Leo commented 1 week ago

Hello @lquerel and @jsuereth , I would like to work on this issue!

Here is my approach after some play around and observation:

  1. Modify the functions in Impl RegistryRepo to respect symlink options when walking directories
  2. Add a new bool follow_symlinks flag to RegistryGenerateArgs in the CLI, and pass the value of the flag follow_symlinks into the registry_repo variable.
  3. Add appropriate tests to verify the behavior

The ideal outcome would be: After running the command

weaver registry generate -r ./semantic-conventions/model -t ./semantic-conventions/templates markdown -follow_symlinks=true

weaver will read symbolic linked folders under sementic-conventions/model and generate the artifacts.

And my question would be:

  1. Is this approach on the right track?
  2. Do we need to modify the FileSystemLoader too? (i.e supporting the symbolink for template files)

PS: new to OTel community, Let me know your thoughts on above. Thanks!!

lquerel commented 1 week ago

Hello @Leo6Leo, To start, it's really cool that you’re helping out with Weaver! To start, it’s really great that you’re lending a hand with Weaver!

Regarding the approach you described, RegistryRepo doesn’t handle directory traversal. Instead, it focuses on creating a local repository in the case of a remote registry (git repo or archive). To modify the code for directory traversal, you’ll need to work on the resolution part in SchemaResolver::load_semconv_from_local_path (everything is a local directory for the resolver). You should add a follow-symlinks parameter as you proposed and use the follow_links method of WalkDir. This also means propagating this parameter through the entire upstream call chain.

Then, at a minimum, you’ll need to update the following commands: registry check, registry resolve, and registry generate.

I also recommend defining the follow_symlinks field in a structure annotated with Clap that will be shared among these commands using the #[command(flatten)] annotation.

Let me know if you need more guidance on this! Thanks

Leo6Leo commented 1 week ago

@lquerel Thank you for the warm welcome and quick response! I have created a PR to fix this.

https://github.com/open-telemetry/weaver/pull/468

Since I am new to Rust and OTel, I am open to suggestions and feedback. Again thanks for spending time providing me with guidance!