Rework how we store log fields

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
+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

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...))
+	}
+}

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)
+			}
+		})
+	}
+}

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
 }