Add the print-config subcommand

[gmafrac]

Which problem is this PR solving?

• Resolves Issue [Feature]: Print fullly resolved config #5168

Description of the changes

• Create a print-config CLI subcommand to print the resolved configurations in the path /cmd/internal/config. It iterates through the settings with viper, returning all the keys that contain a value and printing whether that value is default or not.
• Create a --all flag that when called prints all settings, whether they have values or not.

Below is an example of the output without a all flag:

go run cmd/all-in-one/main.go print-config
2024/02/15 13:55:52 maxprocs: Leaving GOMAXPROCS=16: CPU quota undefined
2024/02/15 13:55:52 application version: git-commit=, git-version=, build-date=
-------------------------------------------------------------------------------
| Configuration Option Name                          Value      Source        |
-------------------------------------------------------------------------------
| admin.http.host-port                               :14269     default       |
| admin.http.tls.enabled                             false      default       |
| collector.enable-span-size-metrics                 false      default       |
| collector.grpc-server.host-port                    :14250     default       |
| collector.grpc-server.max-connection-age           0s         default       |
| collector.grpc-server.max-connection-age-grace     0s         default       |
| collector.grpc-server.max-message-size             4194304    default       |
| collector.grpc.tls.enabled                         false      default       |
| collector.http-server.host-port                    :14268     default       |
| collector.http-server.idle-timeout                 0s         default       |
| collector.http-server.read-header-timeout          2s         default       |
| collector.http-server.read-timeout                 0s         default       |
| collector.http.tls.enabled                         false      default       |
| collector.num-workers                              50         default       |
| collector.otlp.enabled                             true       default       |
| collector.otlp.grpc.max-connection-age             0s         default       |
| collector.otlp.grpc.max-connection-age-grace       0s         default       |
| collector.otlp.grpc.max-message-size               4194304    default       |
| collector.otlp.grpc.tls.enabled                    false      default       |
| collector.otlp.grpc.tls.reload-interval            0s         default       |
| collector.otlp.http.idle-timeout                   0s         default       |
| collector.otlp.http.read-header-timeout            2s         default       |
| collector.otlp.http.read-timeout                   0s         default       |
| collector.otlp.http.tls.enabled                    false      default       |
| collector.otlp.http.tls.reload-interval            0s         default       |
| collector.queue-size                               2000       default       |
| collector.queue-size-memory                        0          default       |
| collector.zipkin.keep-alive                        true       default       |
| collector.zipkin.tls.enabled                       false      default       |
| dir                                                ./         default       |
| downsampling.ratio                                 1          default       |
| format                                             md         default       |
| http-server.host-port                              :5778      default       |
| log-level                                          info       default       |
| memory.max-traces                                  0          default       |
| metrics-backend                                    prometheus default       |
| metrics-http-route                                 /metrics   default       |
| multi-tenancy.enabled                              false      default       |
| multi-tenancy.header                               x-tenant   default       |
| processor.jaeger-binary.server-host-port           :6832      default       |
| processor.jaeger-binary.server-max-packet-size     65000      default       |
| processor.jaeger-binary.server-queue-size          1000       default       |
| processor.jaeger-binary.server-socket-buffer-size  0          default       |
| processor.jaeger-binary.workers                    10         default       |
| processor.jaeger-compact.server-host-port          :6831      default       |
| processor.jaeger-compact.server-max-packet-size    65000      default       |
| processor.jaeger-compact.server-queue-size         1000       default       |
| processor.jaeger-compact.server-socket-buffer-size 0          default       |
| processor.jaeger-compact.workers                   10         default       |
| processor.zipkin-compact.server-host-port          :5775      default       |
| processor.zipkin-compact.server-max-packet-size    65000      default       |
| processor.zipkin-compact.server-queue-size         1000       default       |
| processor.zipkin-compact.server-socket-buffer-size 0          default       |
| processor.zipkin-compact.workers                   10         default       |
| query.base-path                                    /          default       |
| query.bearer-token-propagation                     false      default       |
| query.enable-tracing                               false      default       |
| query.grpc-server.host-port                        :16685     default       |
| query.grpc.tls.enabled                             false      default       |
| query.http-server.host-port                        :16686     default       |
| query.http.tls.enabled                             false      default       |
| query.log-static-assets-access                     false      default       |
| query.max-clock-skew-adjustment                    0s         default       |
| reporter.grpc.discovery.min-peers                  3          default       |
| reporter.grpc.retry.max                            3          default       |
| reporter.grpc.tls.enabled                          false      default       |
| reporter.grpc.tls.skip-host-verify                 false      default       |
| reporter.type                                      grpc       default       |
| sampling.strategies-reload-interval                0s         default       |
| span-storage.type                                  memory     user-assigned |
| status.http.host-port                              :14269     default       |
-------------------------------------------------------------------------------

How was this change tested?

• I tested this function by adding some variables using the viper library., calling the CLI command to check them and comparing them in both key and value.

Checklist

• I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
• I have signed all commits
• I have added unit tests for the new functionality
• I have run lint and test steps successfully
  • for jaeger: make lint test
  • for jaeger-ui: yarn lint and yarn test

[yurishkuro]

code does not compile

 17:48:17  ~/dev/jaegertracing/jaeger   gmafrac/print-config ✔ 
$ go run ./cmd/all-in-one print-config
cmd/all-in-one/main.go:36:2: found packages printconfig (command.go) and print_config (config.go) in /Users/ysh/dev/jaegertracing/jaeger/cmd/internal/config

[yurishkuro]

make sure commits are signed, see CONTRIBUTING.md

[yurishkuro]

Please include sample output of the new command in the pr description

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (951943f) 94.38% compared to head (f7f03a3) 94.44%.
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5200      +/-   ##
==========================================
+ Coverage   94.38%   94.44%   +0.05%     
==========================================
  Files         331      334       +3     
  Lines       19156    19254      +98     
==========================================
+ Hits        18081    18184     +103     
+ Misses        886      882       -4     
+ Partials      189      188       -1     

Flag	Coverage Δ
cassandra-3.x	25.53% <ø> (-0.05%)	⬇️
cassandra-4.x	25.53% <ø> (-0.05%)	⬇️
elasticsearch-5.x	19.83% <ø> (-0.04%)	⬇️
elasticsearch-6.x	19.83% <ø> (-0.04%)	⬇️
elasticsearch-7.x	19.96% <ø> (-0.04%)	⬇️
elasticsearch-8.x	20.05% <ø> (-0.04%)	⬇️
grpc-badger	19.46% <ø> (-0.08%)	⬇️
kafka	14.06% <ø> (-0.03%)	⬇️
opensearch-1.x	19.96% <ø> (-0.04%)	⬇️
opensearch-2.x	19.96% <ø> (-0.04%)	⬇️
unittests	92.29% <100.00%> (+0.13%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yurishkuro]

go run ./cmd/all-in-one/ print-config
2024/02/12 12:34:27 maxprocs: Leaving GOMAXPROCS=16: CPU quota undefined
2024/02/12 12:34:27 application version: git-commit=, git-version=, build-date=
admin.http.host-port=:14269
admin.http.tls.cert=
admin.http.tls.cipher-suites=
admin.http.tls.client-ca=

Maybe it makes sense to only print flags that have an actual value, either default or user-assigned (and would be nice to distinguish those two). And you could add --all flag for this command to print all values, even empty ones (off by default).

The linter is failing.

[gmafrac]

I followed your suggestions and added a --all flag and a print to identify whether it is a default or user-assigned value. I ran 'make lint test' successfully and I've added to the PR description about these details, updating the sample output as well.

[gmafrac]

Thanks for the explanation and suggestion, I've updated the test, but I still have a doubt: is there another test that would be interesting for the printconfig command? Or would this test be enough?

[gmafrac]

When testing a representation with default values, I read the function I'm using to check if a value has been assigned by the user. I discovered that the "v.IsSet()" function I'm using to check whether a value has been assigned by the user only returns false if the flag is a default.

So I have another doubt that I'd like to understand: is it possible to check whether a value is a default setting using this logic? Is there another way for a setting to be default?

Below, I've posted an extract from the viper library (viper@v1.18.2) showing the IsSet() function and the documentation for the find() function to explain this.

  func (v *Viper) IsSet(key string) bool {
	  lcaseKey := strings.ToLower(key)
	  val := v.find(lcaseKey, false)
	  return val != nil
  }
  
  // Given a key, find the value.
  //
  // Viper will check to see if an alias exists first.
  // Viper will then check in the following order:
  // flag, env, config file, key/value store.
  // Lastly, if no value was found and flagDefault is true, and if the key
  // corresponds to a flag, the flag's default value is returned.
  //
  func (v *Viper) find(lcaseKey string, flagDefault bool) any {
  .....
  }

[gmafrac]

I've made these changes, but I still have doubts about the comment above (about the default and user-assigned values)

[yurishkuro]

from what I see, IsSet calls find() with flagDefault=false, which means it will not take default value into account, only value explicitly set by the user via any of the supported mechanisms

[gmafrac]

Yes, but this function will only indicate whether the flag is default and not whether other types of configurations are set as default (as in the setDefault method, for example). But, looking at the tests and the output generated for the configurations present in the project (in the description), I believe that this implementation makes sense. Therefore, I believe that the changes made could already be reviewed

[yurishkuro]

Yes, but this function will only indicate whether the flag is default and not whether other types of configurations are set as default (as in the setDefault method, for example).

so the best way is to verify this with unit tests. Your tests are written differently from how we usually do them for CLI flags, so it's hard to understand what's happening when you call Set. Compare with

jaeger/plugin/storage/grpc/options_test.go

Line 28 in 5375e1e

func TestOptionsWithFlags(t *testing.T) {

- here we (a) use our default way of declaring flags, and (b) use the usual logic for parsing them and then accessing.

[gmafrac]

I have adapted the unit test to declare the flags as declared in the code example. I imported the Jaeger libraries to do this with the exception of gprc, which I based on the code but didn't import.

As a result: The settings declared in config.Viperize were "default" and those analysed in "parse flags" were "user-assigned".

[yurishkuro]

Thanks!

[gmafrac]

Thank you for your help!