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

Request to Add Configuration Verification Option #2131

Closed
agrib-01 opened this issue Feb 23, 2023 · 4 comments
Closed

Request to Add Configuration Verification Option #2131

agrib-01 opened this issue Feb 23, 2023 · 4 comments
Labels
good first issue Good for newcomers

Comments

@agrib-01
Copy link
Contributor

Issue Report: Adding a Config Verification Option to Tempo

Problem:
I am trying to validate the Tempo configuration file before deploying it. However, I have noticed that there is currently no option available to verify the configuration file.

Proposed Solution:
I would like to request the addition of a configuration verification option in Tempo, similar to the "verify-config" option available in Loki, which allows for the validation of configuration syntax before applying the configuration. This option would be very useful to have when deploying and managing Tempo, as it would ensure that the configuration file is valid and reduce the risk of errors during deployment.

Additional Context:
Currently, there is no option available to check the syntax of the configuration file in Tempo, which is available in other related tools such as Loki and Mimir. I believe that adding this option to Tempo would make the tool more versatile and user-friendly.

@agrib-01 agrib-01 changed the title "verify-config" option Request to Add Configuration Verification Option to Tempo Feb 23, 2023
@agrib-01 agrib-01 changed the title Request to Add Configuration Verification Option to Tempo Request to Add Configuration Verification Option Feb 23, 2023
@joe-elliott
Copy link
Member

This is a great idea. I will point out that we have a "CheckConfig" method that will provide warnings on configs that parse correctly but have potentially bad combinations of settings:

https://github.com/grafana/tempo/blob/main/cmd/tempo/app/config.go#L128

I think verify config should parse the config, run it through this method and print out any warnings/errors.

@joe-elliott joe-elliott added the good first issue Good for newcomers label Feb 23, 2023
@agrib-01
Copy link
Contributor Author

Thank you for pointing out the "CheckConfig" method as a potential solution. I appreciate your input and will look into implementing this flag in Tempo. Once I have made the necessary changes, I will create a pull request for review.

@agrib-01
Copy link
Contributor Author

Hi @joe-elliott,
I've created a PR that allows configuration validation using the flag -config.verify and then terminates the execution.
I would appreciate it if you could review it.

joe-elliott pushed a commit that referenced this issue Feb 27, 2023
* Add configuration verification flag to CLI

Co-authored-by: Robert Scherbarth <r.scherbarth@icloud.com>

* centralize config verify call to load config

Co-authored-by: Dennis Brigadenko <dennis.brigadenko@paymenttools.com>

---------

Co-authored-by: Robert Scherbarth <r.scherbarth@icloud.com>
@agrib-01
Copy link
Contributor Author

Hi,
since the pull request has been merged, I'm closing this issue here.
Thanks again for pointing me to the correct function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants