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

Use protobuf reflection for loading runtime configuration #2025

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Molter73
Copy link
Collaborator

@Molter73 Molter73 commented Feb 3, 2025

Description

This change is based upon a toy project I have at https://github.com/Molter73/config-much.

The basic idea is to use the introspection and reflection APIs from protobuf to populate the runtime configuration structures, which should allow us to add new fields to our protobuf definitions and have them magically show up in our code to use.

This is an alternative implementation to #1993.

Checklist

  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

CI should be enough to ensure no behavioral changes have been introduced.

This change is based upon a toy project I have at
https://github.com/Molter73/config-much.

The basic idea is to use the reflection API from protobuf to populate
the runtime configuration structures, which should allow us to add new
fields to our protobuf definitions and have them magically show up in
our code to use.
@Molter73 Molter73 added the run-multiarch-builds Run steps for non-x86 archs. label Feb 5, 2025
@Molter73 Molter73 marked this pull request as ready for review February 5, 2025 09:55
@Molter73 Molter73 requested a review from a team as a code owner February 5, 2025 09:55
@@ -140,8 +140,12 @@ int main(int argc, char** argv) {

CollectorConfig config;
config.InitCollectorConfig(args);
if (!ConfigLoader::LoadConfiguration(config)) {
CLOG(FATAL) << "Unable to parse configuration file";
switch (ConfigLoader(config).LoadConfiguration()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
switch (ConfigLoader(config).LoadConfiguration()) {
if (ConfigLoader(config).LoadConfiguration()== collecotr::ConfigLoader::PARSE_ERROR) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-multiarch-builds Run steps for non-x86 archs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants