Log struct key-values instead of struct string blobs

[wilkermichael]

Description

Improve logs by allowing for structs to be logged as a list of key-values rather than as a string blob. This would be beneficial in allowing these fields to be queryable.

For example, lets look at config/terraform.go:

https://github.com/hashicorp/consul-terraform-sync/blob/c1cc3009ac7a07a2bdfae3d2fead18386606b274/config/terraform.go#L309-L331

When used in a log string like

logging.Info("some log message", "terraform_config", tf.GoString())

This will print the struct as a string, keyed by "terraform_config" in this structured log: 2021-10-04T15:14:49.861-0700 [INFO] some log message: terraform_config="&TerraformConfig{Version:, Log:false, PersistLog:false, Path:/Users/mwilkerson/dev/consul-terraform-sync/examples/michael-example, WorkingDir:, Backend:map[consul:map[address:localhost:8500 gzip:true path:consul-terraform-sync/terraform]], RequiredProviders:map[local:map[source:hashicorp/local version:2.1.0]]}"

Instead, it would be nice the log printed out queryable key values for the struct

eg. query for persist_log or version

Additional context

Original Suggestion Log Library

EDIT Possible Solution

After doing more investigation myself, I realised that the log library used by CTS, go-hclog, supports JSON output which would be the ideal format for computer parsing. Therefore, it makes more sense to make sure the JSON is queryable, and the focus of the human-readable output should be on clarity. In the future (not in scope) CTS could be configured to output either JSON or human-readable output. For now CTS will only output the human-readable format, and this JSON support will help with future use-cases.

For example, let use the config.TerraformConfig struct.

To support JSON output, json struct tags could be implemented alongside the TerraformConfig struct. These struct tags would then match the mapstructure tags already included.

// TerraformConfig is the configuration for the Terraform driver.
type TerraformConfig struct {
    Version           *string                `mapstructure:"version" json:"version"`
    Log               *bool                  `mapstructure:"log" json:"log"`
    PersistLog        *bool                  `mapstructure:"persist_log" json:"persist_log"`
       ...
}

Then when logging.Global().Info("test", "terraform_config", tc) is called with JSON enabled, the following log would be outputted (formatted for clarity, would be all on one line):

{
   "@level":"info",
   "@message":"test",
   "@timestamp":"2021-11-16T12:06:58.392-0800",
   "terraform_config":{
      "version":"test",
      "log":true,
      "persist_log":false,
      "path":"/consul-terraform-sync/config",
      ...
   }
}

Now terraform_config.version=test can be queried for, and a new method isn't required to create the JSON object.

This handles making the logs queryable in their JSON form, and computer-parsable, but it’s still important to support readable logs as well. Due to the pointers in the terraformConfig struct, if the struct is printed out as is, most variables will include pointer addresses rather than their actual values. Therefore a custom string method is required, so the stringer interface can be used: func (c TerraformConfig) String() string (note the non-pointer receiver).

To introduce the new String() method, I would suggest renaming the current GoString() method to String() and making the following alterations: 1) remove the pointer receiver and nil check 2) remove the &TerraformConfig

Once these changes were made, logging.Global().Info("test", "terraform_config", tc) can be called with JSON disabled, and under the hood it will call tc.String() yielding:

2021-11-16T12:06:58.392-0800 [INFO] test: terraform_config="{Version:test, Log:true, PersistLog:false, Path:/consul-terraform-sync/config, WorkingDir:test, Backend:map[consul:map[address:localhost:8500 gzip:true path:consul-terraform-sync/terraform]], RequiredProviders:map[]}"

In Summary of what would be needed to accomplish this alternative solution 1) Add the JSON struct tags to the config objects which will match the mapstructure tags 2) Replace GoString() with String() function name and make minor alterations 3) Replace all instances of object.GoString() with just object 4) Add test cases to verify the string output and the JSON. I wouldn't use the the logger for this, just call obj.String() directly to verify the string, and marshal the JSON to verify the JSON

Enabled/Disabling JSON output is not in scope, and would be a separate issue.

[fransafu]

Can I try to fix this problem?

[wilkermichael]

Hello @fransafu, that's great that you've taken interest in this issue, and thanks again for your previous contribution! If you're still interested in giving this enhancement a try, let me know and I can assign you.

[thisisommore]

Hey, can I give it a try?

[wilkermichael]

Hello @thisisommore Hacktoberfest is now over, but we still value community contributions to the project! If you're still interested in contributing, feel free to submit a PR.

[thisisommore]

@wilkermichael cool, I am still interested in this, will get my hands dirty and will try to create PR :)