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

Move conversion of specific provider to separate file #201

Open
fpiwowarczyk opened this issue Feb 8, 2023 · 6 comments
Open

Move conversion of specific provider to separate file #201

fpiwowarczyk opened this issue Feb 8, 2023 · 6 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@fpiwowarczyk
Copy link
Member

fpiwowarczyk commented Feb 8, 2023

Summary

Currently we have Nobl9 convert method in one file as general convert methods. I think we should move vendor specific methods to separate directory to have cleaner code structure and more open for new vendors to add their convertes. Beside that currently we have in spec metadata.annotations which contain vendor specific fields, we don't have place to document things like that beside code, having separate folder for Nobl9 allows to document that there.

Proposed structure

pkg-----convert
            |
            |----convert.go
            |
            |-----convert_test.go
            |
            |-----Nobl9
            |         |
            |         |----convert.go
            |
            |-------Different_provider
                                 |
                                 |----convert.go
                    
@fpiwowarczyk fpiwowarczyk added enhancement New feature or request good first issue Good for newcomers labels Feb 8, 2023
@kenfinnigan
Copy link
Collaborator

This is a good step in the right direction.

I wonder whether we could go further, in the long term, where vendors can "register" a plugin with the converter? Doing so would mean vendors don't need to submit their conversion functions to the repo for use with oslo

@programmer04
Copy link
Member

Good idea @kenfinnigan, I think we have to do it step by step. For now, we don't have a lot of external plugins, thus it won't be a lot of hustle for maintainers to merge, and review PRs.

I think @fpiwowarczyk proposition is valid, we should provide structure and Go interface to be implemented by those plugins. Those plugins would be compiled in binary. They could then perform registration automatically in their init function. So contributors would only touch code in their plugin's directory. It should be enough for quite a long time.

Having mechanisms that allow the use of plugins that are not part of oslo binary is more complicated, but achievable. Approaches that come to my mind

  • similar to krew used by kubectl (separate binaries available in shell path that are invoked by main program - oslo)
  • approach that HashiCorp uses in their products (plugins exposes gRPC server)
    But in both cases maintaining the version of plugins in sync with oslo and allowing to discover, update, etc. them seems a little bit challenging

@nieomylnieja
Copy link
Member

I think we might be able to go in this direction soon. I agree it's not ideal that proprietary standards are maintained in an open source project, if we want to have ready to use conversion in oslo, we should stick to open source formats. Speaking from a perspective of an employee and maintainer of Nobl9 API contracts the current state also adds toil for me, as Ideally I'd prefer to manage the conversion in a plugin which I could take full ownership of and be able to add dependencies from private repos.

I don't have a solution yet on how exactly we would manage it but I'd probably go with the krew approach (that's what I was initially thinking about too), running plugins as servers seems like an overkill to me with the current oslo use cases which do not involve anything long running (like a server).

@proffalken
Copy link
Collaborator

I'm interested in seeing how we can get this working for Grafana SLO, however as that takes a JSON document in the request body, we'd need to convert it (and yes, I know that JSON and YAML are step-siblings, but using Oslo to create vendor-specific data should be a trivial thing IMHO :) )

@nieomylnieja
Copy link
Member

I will try to create a PoC by the end of February, I also need this separation of concerns to better handle OpenSLO intergration with Nobl9 :)

@fpiwowarczyk
Copy link
Member Author

So, as we discussed on community meeting we don't really want expand this idea of converters and focus more on providing good sdk tools for developers, right? We can close this issue then.

Imo the only thing we need to support related to conversion is moving between popular data types json/yaml etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: Todo
Development

No branches or pull requests

5 participants