diff --git a/Makefile b/Makefile index 814cdaa..6364d4d 100644 --- a/Makefile +++ b/Makefile @@ -40,4 +40,13 @@ website/build-local: @docker build https://github.com/hashicorp/terraform-website.git\#$(WEBSITE_BRANCH) \ -t $(WEBSITE_DOCKER_IMAGE_LOCAL) -.PHONY: website website/local website/build-local \ No newline at end of file +lint: + golangci-lint run + +fmt: + gofmt -s -w -e . + +test: + go test -v -cover -timeout=120s -parallel=4 ./... + +.PHONY: lint fmt test website website/local website/build-local diff --git a/internal/fieldutils/field_maps.go b/internal/fieldutils/field_maps.go new file mode 100644 index 0000000..ac78d6d --- /dev/null +++ b/internal/fieldutils/field_maps.go @@ -0,0 +1,45 @@ +package fieldutils + +// MergeFieldMaps takes a slice of field maps, +// and merges all the key/value pairs into a new single field map. +// +// Input order matters: in case two or more maps use the same key, +// the last one to set that key will have the corresponding value +// persisted. +func MergeFieldMaps(maps ...map[string]interface{}) map[string]interface{} { + // Pre-allocate a map to merge all the maps into, + // that has at least the capacity equivalent to the number + // of maps to merge + result := make(map[string]interface{}, len(maps)) + + // Merge all the maps into one; + // in case of clash, only the last key is preserved + for _, m := range maps { + for k, v := range m { + result[k] = v + } + } + + return result +} + +// FieldMapsToKeys will extract all the field maps keys, avoiding repetitions +// in case two or more maps contained the same key. +func FieldMapsToKeys(maps ...map[string]interface{}) []string { + switch len(maps) { + case 0: + return nil + case 1: + result := make([]string, 0, len(maps[0])) + + for k := range maps[0] { + result = append(result, k) + } + + return result + default: + // As we merge all maps into one, we can use this + // same function recursively, falling back on the `switch case 1`. + return FieldMapsToKeys(MergeFieldMaps(maps...)) + } +} diff --git a/internal/fieldutils/field_maps_test.go b/internal/fieldutils/field_maps_test.go new file mode 100644 index 0000000..8e51a02 --- /dev/null +++ b/internal/fieldutils/field_maps_test.go @@ -0,0 +1,238 @@ +package fieldutils_test + +import ( + "sort" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-log/internal/fieldutils" +) + +func TestMergeFieldMaps(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + input []map[string]interface{} + expected map[string]interface{} + }{ + "nil": { + input: nil, + expected: map[string]interface{}{}, + }, + "empty-maps": { + input: []map[string]interface{}{ + {}, + {}, + {}, + }, + expected: map[string]interface{}{}, + }, + "single-map": { + input: []map[string]interface{}{ + { + "k1": 123, + "k2": "two", + "k3": 3.45, + }, + }, + expected: map[string]interface{}{ + "k1": 123, + "k2": "two", + "k3": 3.45, + }, + }, + "multiple-maps": { + input: []map[string]interface{}{ + { + "k1": 123, + "k2": "two", + "k3": 3.45, + }, + { + "k4": true, + "k5": "five", + "k6": 6.78, + }, + }, + expected: map[string]interface{}{ + "k1": 123, + "k2": "two", + "k3": 3.45, + "k4": true, + "k5": "five", + "k6": 6.78, + }, + }, + "key-collision": { + input: []map[string]interface{}{ + { + "k1": 123, + "k2": "two", + "k3": 3.45, + }, + { + "k1": true, + "k5": "five", + "k3": 6.78, + }, + { + "k4": "four", + }, + }, + expected: map[string]interface{}{ + "k1": true, + "k2": "two", + "k3": 6.78, + "k5": "five", + "k4": "four", + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := fieldutils.MergeFieldMaps(testCase.input...) + + if diff := cmp.Diff(got, testCase.expected); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + +func TestFieldMapsToKeys(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + maps []map[string]interface{} + expected []string + }{ + "nil": { + maps: nil, + expected: nil, + }, + "map-single": { + maps: []map[string]interface{}{ + { + "map1-key1": "map1-value1", + "map1-key2": "map1-value2", + "map1-key3": "map1-value3", + }, + }, + expected: []string{ + "map1-key1", + "map1-key2", + "map1-key3", + }, + }, + "map-multiple-different-keys": { + maps: []map[string]interface{}{ + { + "map1-key1": "map1-value1", + "map1-key2": "map1-value2", + "map1-key3": "map1-value3", + }, + { + "map2-key1": "map2-value1", + "map2-key2": "map2-value2", + "map2-key3": "map2-value3", + }, + }, + expected: []string{ + "map1-key1", + "map1-key2", + "map1-key3", + "map2-key1", + "map2-key2", + "map2-key3", + }, + }, + "map-multiple-mixed-keys": { + maps: []map[string]interface{}{ + { + "key1": "map1-value1", + "key2": "map1-value2", + "key3": "map1-value3", + }, + { + "key4": "map2-value4", + "key1": "map2-value1", + "key5": "map2-value5", + }, + }, + expected: []string{ + "key1", + "key2", + "key3", + "key4", + "key5", + }, + }, + "map-multiple-overlapping-keys": { + maps: []map[string]interface{}{ + { + "key1": "map1-value1", + "key2": "map1-value2", + "key3": "map1-value3", + }, + { + "key1": "map2-value1", + "key2": "map2-value2", + "key3": "map2-value3", + }, + }, + expected: []string{ + "key1", + "key2", + "key3", + }, + }, + "map-multiple-overlapping-keys-shallow": { + maps: []map[string]interface{}{ + { + "key1": map[string]interface{}{ + "submap-key1": "map1-value1", + "submap-key2": "map1-value2", + "submap-key3": "map1-value3", + }, + "key2": "map1-value2", + "key3": "map1-value3", + }, + { + "key1": map[string]interface{}{ + "submap-key4": "map2-value4", + "submap-key5": "map2-value5", + "submap-key6": "map2-value6", + }, + "key3": "map2-value3", + "key2": "map2-value2", + }, + }, + expected: []string{ + "key1", + "key2", + "key3", + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := fieldutils.FieldMapsToKeys(testCase.maps...) + + sort.Strings(got) + + if diff := cmp.Diff(got, testCase.expected); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} diff --git a/internal/hclogutils/args.go b/internal/hclogutils/args.go index 9e13e71..44c81ab 100644 --- a/internal/hclogutils/args.go +++ b/internal/hclogutils/args.go @@ -1,12 +1,12 @@ package hclogutils import ( - "fmt" + "github.com/hashicorp/terraform-plugin-log/internal/fieldutils" ) -// MapsToArgs will shallow merge field maps into a slice of key/value pairs +// FieldMapsToArgs will shallow merge field maps into a slice of key/value pairs // arguments (i.e. `[k1, v1, k2, v2, ...]`) expected by hc-log.Logger methods. -func MapsToArgs(maps ...map[string]interface{}) []interface{} { +func FieldMapsToArgs(maps ...map[string]interface{}) []interface{} { switch len(maps) { case 0: return nil @@ -19,45 +19,8 @@ func MapsToArgs(maps ...map[string]interface{}) []interface{} { return result default: - // Pre-allocate a map to merge all the maps into, - // that has at least the capacity equivalent to the number - // of maps to merge - mergedMap := make(map[string]interface{}, len(maps)) - - // Merge all the maps into one; - // in case of clash, only the last key is preserved - for _, m := range maps { - for k, v := range m { - mergedMap[k] = v - } - } - - // As we have merged all maps into one, we can use this - // same function recursively for the `switch case 1`. - return MapsToArgs(mergedMap) - } -} - -// ArgsToKeys will extract all keys from a slice of key/value pairs -// arguments (i.e. `[k1, v1, k2, v2, ...]`) expected by hc-log.Logger methods. -// -// Note that, in case of an odd number of arguments, the last key captured -// will refer to a value that does not actually exist. -func ArgsToKeys(args []interface{}) []string { - // Pre-allocate enough capacity to fit all the keys, - // i.e. all the elements in the input array in even position - keys := make([]string, 0, len(args)/2) - - for i := 0; i < len(args); i += 2 { - // All keys should be strings, but in case they are not - // we format them to string - switch k := args[i].(type) { - case string: - keys = append(keys, k) - default: - keys = append(keys, fmt.Sprintf("%s", k)) - } + // As we merge all maps into one, we can use this + // same function recursively, falling back on the `switch case 1`. + return FieldMapsToArgs(fieldutils.MergeFieldMaps(maps...)) } - - return keys } diff --git a/internal/hclogutils/args_test.go b/internal/hclogutils/args_test.go index 22dd976..2e0e0bc 100644 --- a/internal/hclogutils/args_test.go +++ b/internal/hclogutils/args_test.go @@ -8,7 +8,7 @@ import ( "github.com/hashicorp/terraform-plugin-log/internal/hclogutils" ) -func TestMapsToArgs(t *testing.T) { +func TestFieldMapsToArgs(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -134,7 +134,7 @@ func TestMapsToArgs(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got := hclogutils.MapsToArgs(testCase.maps...) + got := hclogutils.FieldMapsToArgs(testCase.maps...) if len(got)%2 != 0 { t.Fatalf("expected even number of key-value fields, got: %v", got) @@ -171,142 +171,3 @@ func TestMapsToArgs(t *testing.T) { }) } } - -func TestArgsToKeys(t *testing.T) { - t.Parallel() - - testCases := map[string]struct { - args []interface{} - expectedKeys []string - }{ - "nil": { - args: []interface{}{}, - expectedKeys: []string{}, - }, - "simple": { - args: []interface{}{ - "map1-key1", "map1-value1", - "map1-key2", "map1-value2", - "map1-key3", "map1-value3", - }, - expectedKeys: []string{ - "map1-key1", - "map1-key2", - "map1-key3", - }, - }, - "non-even-number-of-args": { - args: []interface{}{ - "map1-key1", "map1-value1", - "map1-key2", "map1-value2", - "map1-key3", - }, - expectedKeys: []string{ - "map1-key1", - "map1-key2", - "map1-key3", - }, - }, - "multiple-different-keys": { - args: []interface{}{ - "map1-key1", "map1-value1", - "map1-key2", "map1-value2", - "map1-key3", "map1-value3", - "map2-key1", "map2-value1", - "map2-key2", "map2-value2", - "map2-key3", "map2-value3", - }, - expectedKeys: []string{ - "map1-key1", - "map1-key2", - "map1-key3", - "map2-key1", - "map2-key2", - "map2-key3", - }, - }, - "multiple-mixed-keys": { - args: []interface{}{ - "key1", "map1-value1", - "key2", "map1-value2", - "key3", "map1-value3", - "key4", "map2-value4", - "key1", "map2-value1", - "key5", "map2-value5", - }, - expectedKeys: []string{ - "key1", - "key2", - "key3", - "key4", - "key1", - "key5", - }, - }, - "multiple-overlapping-keys": { - args: []interface{}{ - "key1", "map1-value1", - "key2", "map1-value2", - "key3", "map1-value3", - "key1", "map2-value1", - "key2", "map2-value2", - "key3", "map2-value3", - }, - expectedKeys: []string{ - "key1", - "key2", - "key3", - "key1", - "key2", - "key3", - }, - }, - "multiple-overlapping-keys-shallow": { - args: []interface{}{ - "key1", map[string]interface{}{ - "submap-key1": "map1-value1", - "submap-key2": "map1-value2", - "submap-key3": "map1-value3", - }, - "key2", "map1-value2", - "key3", "map1-value3", - "key1", map[string]interface{}{ - "submap-key4": "map2-value4", - "submap-key5": "map2-value5", - "submap-key6": "map2-value6", - }, - "key2", "map2-value2", - "key3", "map2-value3", - }, - expectedKeys: []string{ - "key1", - "key2", - "key3", - "key1", - "key2", - "key3", - }, - }, - } - - for name, testCase := range testCases { - name, testCase := name, testCase - - t.Run(name, func(t *testing.T) { - t.Parallel() - - got := hclogutils.ArgsToKeys(testCase.args) - - if got == nil && testCase.expectedKeys == nil { - return // sortedGot will return []interface{}{} below, nil is what we want - } - - sort.Strings(got) - sort.Strings(testCase.expectedKeys) - - if diff := cmp.Diff(got, testCase.expectedKeys); diff != "" { - t.Errorf("unexpected difference: %s", diff) - } - }) - } -} diff --git a/internal/logging/filtering.go b/internal/logging/filtering.go index efc4f94..9e7867f 100644 --- a/internal/logging/filtering.go +++ b/internal/logging/filtering.go @@ -1,27 +1,23 @@ package logging import ( - "fmt" "strings" + "github.com/hashicorp/terraform-plugin-log/internal/fieldutils" "github.com/hashicorp/terraform-plugin-log/internal/hclogutils" ) const logMaskingReplacementString = "***" -// ShouldOmit takes a log's *string message and slices of arguments, +// ShouldOmit takes a log's *string message and slices of fields, // and determines, based on the LoggerOpts configuration, if the // log should be omitted (i.e. prevent it to be printed on the final writer). -func (lo LoggerOpts) ShouldOmit(msg *string, hclogArgSlices ...[]interface{}) bool { - // Omit log if any of the configured keys is found - // either in the logger implied arguments, - // or in the additional arguments +func (lo LoggerOpts) ShouldOmit(msg *string, fieldMaps ...map[string]interface{}) bool { + // Omit log if any of the configured keys is found in the given fields if len(lo.OmitLogWithFieldKeys) > 0 { - for _, args := range hclogArgSlices { - argKeys := hclogutils.ArgsToKeys(args) - if argKeysContain(argKeys, lo.OmitLogWithFieldKeys) { - return true - } + fieldsKeys := fieldutils.FieldMapsToKeys(fieldMaps...) + if argKeysContain(fieldsKeys, lo.OmitLogWithFieldKeys) { + return true } } @@ -46,29 +42,18 @@ func (lo LoggerOpts) ShouldOmit(msg *string, hclogArgSlices ...[]interface{}) bo return false } -// ApplyMask takes a log's *string message and slices of arguments, -// and applies masking of keys' values and/or message, +// ApplyMask takes a log's *string message and slices of fields, +// and applies masking to fields keys' values and/or to log message, // based on the LoggerOpts configuration. // // Note that the given input is changed-in-place by this method. -func (lo LoggerOpts) ApplyMask(msg *string, hclogArgSlices ...[]interface{}) { +func (lo LoggerOpts) ApplyMask(msg *string, fieldMaps ...map[string]interface{}) { if len(lo.MaskFieldValuesWithFieldKeys) > 0 { for _, k := range lo.MaskFieldValuesWithFieldKeys { - for _, args := range hclogArgSlices { - // Here we loop `i` with steps of 2, starting from position 1 (i.e. `1, 3, 5, 7...`). - // We then look up the key for each argument, by looking at `i-1`. - // This ensures that in case of malformed arg slices that don't have - // an even number of elements, we simply skip the last k/v pair. - for i := 1; i < len(args); i += 2 { - switch argK := args[i-1].(type) { - case string: - if k == argK { - args[i] = logMaskingReplacementString - } - default: - if k == fmt.Sprintf("%s", argK) { - args[i] = logMaskingReplacementString - } + for _, f := range fieldMaps { + for fk := range f { + if k == fk { + f[k] = logMaskingReplacementString } } } @@ -92,6 +77,20 @@ func (lo LoggerOpts) ApplyMask(msg *string, hclogArgSlices ...[]interface{}) { } } +func OmitOrMask(tfLoggerOpts LoggerOpts, msg *string, additionalFields []map[string]interface{}) ([]interface{}, bool) { + additionalFieldsMap := fieldutils.MergeFieldMaps(additionalFields...) + + // Apply the provider root LoggerOpts to determine if this log should be omitted + if tfLoggerOpts.ShouldOmit(msg, tfLoggerOpts.Fields, additionalFieldsMap) { + return nil, true + } + + // Apply the provider root LoggerOpts to apply masking to this log + tfLoggerOpts.ApplyMask(msg, tfLoggerOpts.Fields, additionalFieldsMap) + + return hclogutils.FieldMapsToArgs(tfLoggerOpts.Fields, additionalFieldsMap), false +} + func argKeysContain(haystack []string, needles []string) bool { for _, h := range haystack { for _, n := range needles { diff --git a/internal/logging/filtering_test.go b/internal/logging/filtering_test.go index d7c1fb2..9929b09 100644 --- a/internal/logging/filtering_test.go +++ b/internal/logging/filtering_test.go @@ -1,10 +1,11 @@ -package logging +package logging_test import ( "regexp" "testing" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-log/internal/logging" ) const testLogMsg = "System FOO has caused error BAR because of incorrectly configured BAZ" @@ -13,83 +14,83 @@ func TestShouldOmit(t *testing.T) { t.Parallel() testCases := map[string]struct { - lOpts LoggerOpts + lOpts logging.LoggerOpts msg string - hclogArgSlices [][]interface{} + fieldMaps []map[string]interface{} expectedToOmit bool }{ "empty-opts": { - lOpts: LoggerOpts{}, + lOpts: logging.LoggerOpts{}, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedToOmit: false, }, "omit-log-by-key": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ OmitLogWithFieldKeys: []string{"k2"}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedToOmit: true, }, "no-omit-log-by-key-if-case-mismatches": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ OmitLogWithFieldKeys: []string{"K2"}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedToOmit: false, }, "do-not-omit-log-by-key": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ OmitLogWithFieldKeys: []string{"k3"}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedToOmit: false, }, "omit-log-matching-regexp-case-insensitive": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ OmitLogWithMessageRegexes: []*regexp.Regexp{regexp.MustCompile("(?i)(foo|bar)")}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedToOmit: true, }, "do-not-omit-log-matching-regexp-case-sensitive": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ OmitLogWithMessageRegexes: []*regexp.Regexp{regexp.MustCompile("(foo|bar)")}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedToOmit: false, @@ -102,7 +103,7 @@ func TestShouldOmit(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got := testCase.lOpts.ShouldOmit(&testCase.msg, testCase.hclogArgSlices...) + got := testCase.lOpts.ShouldOmit(&testCase.msg, testCase.fieldMaps...) if got != testCase.expectedToOmit { t.Errorf("expected ShouldOmit to return %t, got %t", testCase.expectedToOmit, got) @@ -115,172 +116,122 @@ func TestApplyMask(t *testing.T) { t.Parallel() testCases := map[string]struct { - lOpts LoggerOpts + lOpts logging.LoggerOpts msg string - hclogArgSlices [][]interface{} + fieldMaps []map[string]interface{} expectedMsg string - expectedArgSlices [][]interface{} + expectedFieldMaps []map[string]interface{} }{ "empty-opts": { - lOpts: LoggerOpts{}, + lOpts: logging.LoggerOpts{}, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedMsg: "System FOO has caused error BAR because of incorrectly configured BAZ", - expectedArgSlices: [][]interface{}{ + expectedFieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, }, "mask-log-by-key": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ MaskFieldValuesWithFieldKeys: []string{"k2"}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedMsg: "System FOO has caused error BAR because of incorrectly configured BAZ", - expectedArgSlices: [][]interface{}{ + expectedFieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "***", + "k1": "v1", + "k2": "***", }, }, }, "no-mask-log-by-key-if-case-mismatches": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ MaskFieldValuesWithFieldKeys: []string{"K2"}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedMsg: "System FOO has caused error BAR because of incorrectly configured BAZ", - expectedArgSlices: [][]interface{}{ + expectedFieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", - }, - }, - }, - "mask-log-by-non-even-args-cannot-mask-missing-value": { - lOpts: LoggerOpts{ - MaskFieldValuesWithFieldKeys: []string{"k2", "k4"}, - }, - msg: testLogMsg, - hclogArgSlices: [][]interface{}{ - { - "k1", "v1", - "k2", "v2", - }, - { - "k3", "v3", - "k4", - }, - }, - expectedMsg: "System FOO has caused error BAR because of incorrectly configured BAZ", - expectedArgSlices: [][]interface{}{ - { - "k1", "v1", - "k2", "***", - }, - { - "k3", "v3", - "k4", - }, - }, - }, - "mask-log-by-non-even-args": { - lOpts: LoggerOpts{ - MaskFieldValuesWithFieldKeys: []string{"k2"}, - }, - msg: testLogMsg, - hclogArgSlices: [][]interface{}{ - { - "k1", "v1", - "k2", "v2", - "k3", "v3", - "k4", - }, - }, - expectedMsg: "System FOO has caused error BAR because of incorrectly configured BAZ", - expectedArgSlices: [][]interface{}{ - { - "k1", "v1", - "k2", "***", - "k3", "v3", - "k4", + "k1": "v1", + "k2": "v2", }, }, }, "mask-log-matching-regexp-case-insensitive": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ MaskMessageRegexes: []*regexp.Regexp{regexp.MustCompile("(?i)(foo|bar)")}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedMsg: "System *** has caused error *** because of incorrectly configured BAZ", - expectedArgSlices: [][]interface{}{ + expectedFieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, }, "mask-log-matching-regexp-case-sensitive": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ MaskMessageRegexes: []*regexp.Regexp{regexp.MustCompile("incorrectly configured BAZ")}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedMsg: "System FOO has caused error BAR because of ***", - expectedArgSlices: [][]interface{}{ + expectedFieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, }, "mask-log-by-key-and-matching-regexp": { - lOpts: LoggerOpts{ + lOpts: logging.LoggerOpts{ MaskMessageRegexes: []*regexp.Regexp{regexp.MustCompile("incorrectly configured BAZ")}, MaskFieldValuesWithFieldKeys: []string{"k1", "k2"}, }, msg: testLogMsg, - hclogArgSlices: [][]interface{}{ + fieldMaps: []map[string]interface{}{ { - "k1", "v1", - "k2", "v2", + "k1": "v1", + "k2": "v2", }, }, expectedMsg: "System FOO has caused error BAR because of ***", - expectedArgSlices: [][]interface{}{ + expectedFieldMaps: []map[string]interface{}{ { - "k1", "***", - "k2", "***", + "k1": "***", + "k2": "***", }, }, }, @@ -292,13 +243,13 @@ func TestApplyMask(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - testCase.lOpts.ApplyMask(&testCase.msg, testCase.hclogArgSlices...) + testCase.lOpts.ApplyMask(&testCase.msg, testCase.fieldMaps...) if diff := cmp.Diff(testCase.msg, testCase.expectedMsg); diff != "" { t.Errorf("unexpected difference detected in log message: %s", diff) } - if diff := cmp.Diff(testCase.hclogArgSlices, testCase.expectedArgSlices); diff != "" { + if diff := cmp.Diff(testCase.fieldMaps, testCase.expectedFieldMaps); diff != "" { t.Errorf("unexpected difference detected in log arguments: %s", diff) } }) diff --git a/internal/logging/options.go b/internal/logging/options.go index f79ea24..1f6ef03 100644 --- a/internal/logging/options.go +++ b/internal/logging/options.go @@ -35,12 +35,15 @@ type LoggerOpts struct { // easy to mess up on accident. Output io.Writer - // IncludeTime indicates whether logs should incude the time they were + // IncludeTime indicates whether logs should include the time they were // written or not. It should only be set to true when testing tflog or // tfsdklog; providers and SDKs should always include the time logs // were written as part of the log. IncludeTime bool + // Fields indicates the key/value pairs to be added to each of its log output. + Fields map[string]interface{} + // IncludeRootFields indicates whether a new subsystem logger should // copy existing fields from the root logger. This is only performed // at the time of new subsystem creation. @@ -165,6 +168,43 @@ func WithOutput(output io.Writer) Option { } } +// WithField sets the provided key/value pair, onto the LoggerOpts.Fields field. +// +// Behind the scene, fields are stored in a map[string]interface{}: +// this means that in case the same key is used multiple times (key collision), +// the last one set is the one that gets persisted and then outputted with the logs. +func WithField(key string, value interface{}) Option { + return func(l LoggerOpts) LoggerOpts { + // Lazily create this map, on first assignment + if l.Fields == nil { + l.Fields = make(map[string]interface{}) + } + + l.Fields[key] = value + return l + } +} + +// WithFields sets all the provided key/value pairs, onto the LoggerOpts.Fields field. +// +// Behind the scene, fields are stored in a map[string]interface{}: +// this means that in case the same key is used multiple times (key collision), +// the last one set is the one that gets persisted and then outputted with the logs. +func WithFields(fields map[string]interface{}) Option { + return func(l LoggerOpts) LoggerOpts { + // Lazily create this map, on first assignment + if l.Fields == nil { + l.Fields = make(map[string]interface{}) + } + + for k, v := range fields { + l.Fields[k] = v + } + + return l + } +} + // WithRootFields enables the copying of root logger fields to a new subsystem // logger during creation. func WithRootFields() Option { diff --git a/tflog/provider.go b/tflog/provider.go index c5ef4a5..0be81b5 100644 --- a/tflog/provider.go +++ b/tflog/provider.go @@ -4,24 +4,20 @@ import ( "context" "regexp" - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/terraform-plugin-log/internal/hclogutils" "github.com/hashicorp/terraform-plugin-log/internal/logging" ) // SetField returns a new context.Context that has a modified logger in it which // will include key and value as fields in all its log output. +// +// In case of the same key is used multiple times (i.e. key collision), +// the last one set is the one that gets persisted and then outputted with the logs. func SetField(ctx context.Context, key string, value interface{}) context.Context { - logger := logging.GetProviderRootLogger(ctx) - if logger == nil { - // this essentially should never happen in production - // the root logger for provider code should be injected - // by whatever SDK the provider developer is using, so - // really this is only likely in unit tests, at most - // so just making this a no-op is fine - return ctx - } - return logging.SetProviderRootLogger(ctx, logger.With(key, value)) + lOpts := logging.GetProviderRootTFLoggerOpts(ctx) + + lOpts = logging.WithField(key, value)(lOpts) + + return logging.SetProviderRootTFLoggerOpts(ctx, lOpts) } // Trace logs `msg` at the trace level to the logger in `ctx`, with optional @@ -39,7 +35,7 @@ func Trace(ctx context.Context, msg string, additionalFields ...map[string]inter return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -62,7 +58,7 @@ func Debug(ctx context.Context, msg string, additionalFields ...map[string]inter return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -85,7 +81,7 @@ func Info(ctx context.Context, msg string, additionalFields ...map[string]interf return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -108,7 +104,7 @@ func Warn(ctx context.Context, msg string, additionalFields ...map[string]interf return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -131,7 +127,7 @@ func Error(ctx context.Context, msg string, additionalFields ...map[string]inter return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -139,21 +135,6 @@ func Error(ctx context.Context, msg string, additionalFields ...map[string]inter logger.Error(msg, additionalArgs...) } -func omitOrMask(ctx context.Context, logger hclog.Logger, msg *string, additionalFields []map[string]interface{}) ([]interface{}, bool) { - tfLoggerOpts := logging.GetProviderRootTFLoggerOpts(ctx) - additionalArgs := hclogutils.MapsToArgs(additionalFields...) - impliedArgs := logger.ImpliedArgs() - - // Apply the provider root LoggerOpts to determine if this log should be omitted - if tfLoggerOpts.ShouldOmit(msg, impliedArgs, additionalArgs) { - return nil, true - } - - // Apply the provider root LoggerOpts to apply masking to this log - tfLoggerOpts.ApplyMask(msg, impliedArgs, additionalArgs) - return additionalArgs, false -} - // OmitLogWithFieldKeys returns a new context.Context that has a modified logger // that will omit to write any log when any of the given keys is found // within its fields. diff --git a/tflog/subsystem.go b/tflog/subsystem.go index 3f6226e..b63ed36 100644 --- a/tflog/subsystem.go +++ b/tflog/subsystem.go @@ -32,43 +32,45 @@ func NewSubsystem(ctx context.Context, subsystem string, options ...logging.Opti return ctx } - loggerOptions := logging.GetProviderRootLoggerOptions(ctx) - opts := logging.ApplyLoggerOpts(options...) - - // On the off-chance that the logger options are not available, fallback - // to creating a named logger. This will preserve the root logger options, - // but cannot make changes beyond the level due to the hclog.Logger - // interface. - if loggerOptions == nil { - subLogger := logger.Named(subsystem) - - if opts.AdditionalLocationOffset != 1 { + rootLoggerOptions := logging.GetProviderRootLoggerOptions(ctx) + subLoggerTFLoggerOpts := logging.ApplyLoggerOpts(options...) + + // If root logger options are not available, + // fallback to creating a logger named like the given subsystem. + // This will preserve the root logger options, + // but cannot make changes beyond setting the level + // due to limitations with the hclog.Logger interface. + var subLogger hclog.Logger + if rootLoggerOptions == nil { + subLogger = logger.Named(subsystem) + + if subLoggerTFLoggerOpts.AdditionalLocationOffset != 1 { logger.Warn("Unable to create logging subsystem with AdditionalLocationOffset due to missing root logger options") } + } else { + subLoggerOptions := hclogutils.LoggerOptionsCopy(rootLoggerOptions) + subLoggerOptions.Name = subLoggerOptions.Name + "." + subsystem - if opts.Level != hclog.NoLevel { - subLogger.SetLevel(opts.Level) + if subLoggerTFLoggerOpts.AdditionalLocationOffset != 1 { + subLoggerOptions.AdditionalLocationOffset = subLoggerTFLoggerOpts.AdditionalLocationOffset } - return logging.SetProviderSubsystemLogger(ctx, subsystem, subLogger) + subLogger = hclog.New(subLoggerOptions) } - subLoggerOptions := hclogutils.LoggerOptionsCopy(loggerOptions) - subLoggerOptions.Name = subLoggerOptions.Name + "." + subsystem - - if opts.AdditionalLocationOffset != 1 { - subLoggerOptions.AdditionalLocationOffset = opts.AdditionalLocationOffset + // Set the configured log level + if subLoggerTFLoggerOpts.Level != hclog.NoLevel { + subLogger.SetLevel(subLoggerTFLoggerOpts.Level) } - if opts.Level != hclog.NoLevel { - subLoggerOptions.Level = opts.Level + // Propagate root fields to the subsystem logger + if subLoggerTFLoggerOpts.IncludeRootFields { + loggerTFOpts := logging.GetProviderRootTFLoggerOpts(ctx) + subLoggerTFLoggerOpts = logging.WithFields(loggerTFOpts.Fields)(subLoggerTFLoggerOpts) } - subLogger := hclog.New(subLoggerOptions) - - if opts.IncludeRootFields { - subLogger = subLogger.With(logger.ImpliedArgs()...) - } + // Set the subsystem LoggerOpts in the context + ctx = logging.SetProviderSubsystemTFLoggerOpts(ctx, subsystem, subLoggerTFLoggerOpts) return logging.SetProviderSubsystemLogger(ctx, subsystem, subLogger) } @@ -76,18 +78,15 @@ func NewSubsystem(ctx context.Context, subsystem string, options ...logging.Opti // SubsystemSetField returns a new context.Context that has a modified logger for // the specified subsystem in it which will include key and value as fields // in all its log output. +// +// In case of the same key is used multiple times (i.e. key collision), +// the last one set is the one that gets persisted and then outputted with the logs. func SubsystemSetField(ctx context.Context, subsystem, key string, value interface{}) context.Context { - logger := logging.GetProviderSubsystemLogger(ctx, subsystem) - if logger == nil { - if logging.GetProviderRootLogger(ctx) == nil { - // logging isn't set up, nothing we can do, just silently fail - // this should basically never happen in production - return ctx - } - // create a new logger if one doesn't exist - logger = logging.GetProviderSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewProviderSubsystemLoggerWarning) - } - return logging.SetProviderSubsystemLogger(ctx, subsystem, logger.With(key, value)) + lOpts := logging.GetProviderSubsystemTFLoggerOpts(ctx, subsystem) + + lOpts = logging.WithField(key, value)(lOpts) + + return logging.SetProviderSubsystemTFLoggerOpts(ctx, subsystem, lOpts) } // SubsystemTrace logs `msg` at the trace level to the subsystem logger @@ -107,7 +106,7 @@ func SubsystemTrace(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetProviderSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewProviderSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -132,7 +131,7 @@ func SubsystemDebug(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetProviderSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewProviderSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -157,7 +156,7 @@ func SubsystemInfo(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetProviderSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewProviderSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -182,7 +181,7 @@ func SubsystemWarn(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetProviderSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewProviderSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -207,7 +206,7 @@ func SubsystemError(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetProviderSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewProviderSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetProviderSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -215,21 +214,6 @@ func SubsystemError(ctx context.Context, subsystem, msg string, additionalFields logger.Error(msg, additionalArgs...) } -func subsystemOmitOrMask(ctx context.Context, logger hclog.Logger, subsystem string, msg *string, additionalFields []map[string]interface{}) ([]interface{}, bool) { - tfLoggerOpts := logging.GetProviderSubsystemTFLoggerOpts(ctx, subsystem) - additionalArgs := hclogutils.MapsToArgs(additionalFields...) - impliedArgs := logger.ImpliedArgs() - - // Apply the provider root LoggerOpts to determine if this log should be omitted - if tfLoggerOpts.ShouldOmit(msg, impliedArgs, additionalArgs) { - return nil, true - } - - // Apply the provider root LoggerOpts to apply masking to this log - tfLoggerOpts.ApplyMask(msg, impliedArgs, additionalArgs) - return additionalArgs, false -} - // SubsystemOmitLogWithFieldKeys returns a new context.Context that has a modified logger // that will omit to write any log when any of the given keys is found // within its fields. diff --git a/tfsdklog/sdk.go b/tfsdklog/sdk.go index 3d30a34..eca9727 100644 --- a/tfsdklog/sdk.go +++ b/tfsdklog/sdk.go @@ -97,16 +97,15 @@ func NewRootProviderLogger(ctx context.Context, options ...logging.Option) conte // SetField returns a new context.Context that has a modified logger in it which // will include key and value as fields in all its log output. +// +// In case of the same key is used multiple times (i.e. key collision), +// the last one set is the one that gets persisted and then outputted with the logs. func SetField(ctx context.Context, key string, value interface{}) context.Context { - logger := logging.GetSDKRootLogger(ctx) - if logger == nil { - // this essentially should never happen in production the root - // logger for code should be injected by the in - // question, so really this is only likely in unit tests, at - // most so just making this a no-op is fine - return ctx - } - return logging.SetSDKRootLogger(ctx, logger.With(key, value)) + lOpts := logging.GetSDKRootTFLoggerOpts(ctx) + + lOpts = logging.WithField(key, value)(lOpts) + + return logging.SetSDKRootTFLoggerOpts(ctx, lOpts) } // Trace logs `msg` at the trace level to the logger in `ctx`, with optional @@ -123,7 +122,7 @@ func Trace(ctx context.Context, msg string, additionalFields ...map[string]inter return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -145,7 +144,7 @@ func Debug(ctx context.Context, msg string, additionalFields ...map[string]inter return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -167,7 +166,7 @@ func Info(ctx context.Context, msg string, additionalFields ...map[string]interf return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -189,7 +188,7 @@ func Warn(ctx context.Context, msg string, additionalFields ...map[string]interf return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -211,7 +210,7 @@ func Error(ctx context.Context, msg string, additionalFields ...map[string]inter return } - additionalArgs, shouldOmit := omitOrMask(ctx, logger, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKRootTFLoggerOpts(ctx), &msg, additionalFields) if shouldOmit { return } @@ -219,21 +218,6 @@ func Error(ctx context.Context, msg string, additionalFields ...map[string]inter logger.Error(msg, additionalArgs...) } -func omitOrMask(ctx context.Context, logger hclog.Logger, msg *string, additionalFields []map[string]interface{}) ([]interface{}, bool) { - tfLoggerOpts := logging.GetSDKRootTFLoggerOpts(ctx) - additionalArgs := hclogutils.MapsToArgs(additionalFields...) - impliedArgs := logger.ImpliedArgs() - - // Apply the provider root LoggerOpts to determine if this log should be omitted - if tfLoggerOpts.ShouldOmit(msg, impliedArgs, additionalArgs) { - return nil, true - } - - // Apply the provider root LoggerOpts to apply masking to this log - tfLoggerOpts.ApplyMask(msg, impliedArgs, additionalArgs) - return additionalArgs, false -} - // OmitLogWithFieldKeys returns a new context.Context that has a modified logger // that will omit to write any log when any of the given keys is found // within its fields. diff --git a/tfsdklog/subsystem.go b/tfsdklog/subsystem.go index bb6ce27..bba1d6f 100644 --- a/tfsdklog/subsystem.go +++ b/tfsdklog/subsystem.go @@ -32,43 +32,45 @@ func NewSubsystem(ctx context.Context, subsystem string, options ...logging.Opti return ctx } - loggerOptions := logging.GetSDKRootLoggerOptions(ctx) - opts := logging.ApplyLoggerOpts(options...) - - // On the off-chance that the logger options are not available, fallback - // to creating a named logger. This will preserve the root logger options, - // but cannot make changes beyond the level due to the hclog.Logger - // interface. - if loggerOptions == nil { - subLogger := logger.Named(subsystem) - - if opts.AdditionalLocationOffset != 1 { + rootLoggerOptions := logging.GetSDKRootLoggerOptions(ctx) + subLoggerTFLoggerOpts := logging.ApplyLoggerOpts(options...) + + // If root logger options are not available, + // fallback to creating a logger named like the given subsystem. + // This will preserve the root logger options, + // but cannot make changes beyond setting the level + // due to limitations with the hclog.Logger interface. + var subLogger hclog.Logger + if rootLoggerOptions == nil { + subLogger = logger.Named(subsystem) + + if subLoggerTFLoggerOpts.AdditionalLocationOffset != 1 { logger.Warn("Unable to create logging subsystem with AdditionalLocationOffset due to missing root logger options") } + } else { + subLoggerOptions := hclogutils.LoggerOptionsCopy(rootLoggerOptions) + subLoggerOptions.Name = subLoggerOptions.Name + "." + subsystem - if opts.Level != hclog.NoLevel { - subLogger.SetLevel(opts.Level) + if subLoggerTFLoggerOpts.AdditionalLocationOffset != 1 { + subLoggerOptions.AdditionalLocationOffset = subLoggerTFLoggerOpts.AdditionalLocationOffset } - return logging.SetSDKSubsystemLogger(ctx, subsystem, subLogger) + subLogger = hclog.New(subLoggerOptions) } - subLoggerOptions := hclogutils.LoggerOptionsCopy(loggerOptions) - subLoggerOptions.Name = subLoggerOptions.Name + "." + subsystem - - if opts.AdditionalLocationOffset != 1 { - subLoggerOptions.AdditionalLocationOffset = opts.AdditionalLocationOffset + // Set the configured log level + if subLoggerTFLoggerOpts.Level != hclog.NoLevel { + subLogger.SetLevel(subLoggerTFLoggerOpts.Level) } - if opts.Level != hclog.NoLevel { - subLoggerOptions.Level = opts.Level + // Propagate root fields to the subsystem logger + if subLoggerTFLoggerOpts.IncludeRootFields { + loggerTFOpts := logging.GetSDKRootTFLoggerOpts(ctx) + subLoggerTFLoggerOpts = logging.WithFields(loggerTFOpts.Fields)(subLoggerTFLoggerOpts) } - subLogger := hclog.New(subLoggerOptions) - - if opts.IncludeRootFields { - subLogger = subLogger.With(logger.ImpliedArgs()...) - } + // Set the subsystem LoggerOpts in the context + ctx = logging.SetSDKSubsystemTFLoggerOpts(ctx, subsystem, subLoggerTFLoggerOpts) return logging.SetSDKSubsystemLogger(ctx, subsystem, subLogger) } @@ -76,18 +78,15 @@ func NewSubsystem(ctx context.Context, subsystem string, options ...logging.Opti // SubsystemSetField returns a new context.Context that has a modified logger for // the specified subsystem in it which will include key and value as fields // in all its log output. +// +// In case of the same key is used multiple times (i.e. key collision), +// the last one set is the one that gets persisted and then outputted with the logs. func SubsystemSetField(ctx context.Context, subsystem, key string, value interface{}) context.Context { - logger := logging.GetSDKSubsystemLogger(ctx, subsystem) - if logger == nil { - if logging.GetSDKRootLogger(ctx) == nil { - // logging isn't set up, nothing we can do, just silently fail - // this should basically never happen in production - return ctx - } - // create a new logger if one doesn't exist - logger = logging.GetSDKSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewSDKSubsystemLoggerWarning) - } - return logging.SetSDKSubsystemLogger(ctx, subsystem, logger.With(key, value)) + lOpts := logging.GetSDKSubsystemTFLoggerOpts(ctx, subsystem) + + lOpts = logging.WithField(key, value)(lOpts) + + return logging.SetSDKSubsystemTFLoggerOpts(ctx, subsystem, lOpts) } // SubsystemTrace logs `msg` at the trace level to the subsystem logger @@ -107,7 +106,7 @@ func SubsystemTrace(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetSDKSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewSDKSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -132,7 +131,7 @@ func SubsystemDebug(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetSDKSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewSDKSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -157,7 +156,7 @@ func SubsystemInfo(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetSDKSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewSDKSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -182,7 +181,7 @@ func SubsystemWarn(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetSDKSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewSDKSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -207,7 +206,7 @@ func SubsystemError(ctx context.Context, subsystem, msg string, additionalFields logger = logging.GetSDKSubsystemLogger(NewSubsystem(ctx, subsystem), subsystem).With("new_logger_warning", logging.NewSDKSubsystemLoggerWarning) } - additionalArgs, shouldOmit := subsystemOmitOrMask(ctx, logger, subsystem, &msg, additionalFields) + additionalArgs, shouldOmit := logging.OmitOrMask(logging.GetSDKSubsystemTFLoggerOpts(ctx, subsystem), &msg, additionalFields) if shouldOmit { return } @@ -215,21 +214,6 @@ func SubsystemError(ctx context.Context, subsystem, msg string, additionalFields logger.Error(msg, additionalArgs...) } -func subsystemOmitOrMask(ctx context.Context, logger hclog.Logger, subsystem string, msg *string, additionalFields []map[string]interface{}) ([]interface{}, bool) { - tfLoggerOpts := logging.GetSDKSubsystemTFLoggerOpts(ctx, subsystem) - additionalArgs := hclogutils.MapsToArgs(additionalFields...) - impliedArgs := logger.ImpliedArgs() - - // Apply the provider root LoggerOpts to determine if this log should be omitted - if tfLoggerOpts.ShouldOmit(msg, impliedArgs, additionalArgs) { - return nil, true - } - - // Apply the provider root LoggerOpts to apply masking to this log - tfLoggerOpts.ApplyMask(msg, impliedArgs, additionalArgs) - return additionalArgs, false -} - // SubsystemOmitLogWithFieldKeys returns a new context.Context that has a modified logger // that will omit to write any log when any of the given keys is found // within its fields.