Fields set with SetField leak

[gdavison]

terraform-plugin-log version

v0.7.0 (commit 3ca5214047ab31ddc04b997942a52e35dbc31640)

Relevant provider source code

func TestSetFieldSequential(t *testing.T) {
	t.Parallel()

	var outputBuffer bytes.Buffer

	key1 := "key1"
	value1 := "value1"

	key2 := "key2"
	value2 := "value2"

	expectedOutputOriginalLogger := []map[string]interface{}{
		{
			"@level":   "trace",
			"@message": "original logger",
			"@module":  "provider",
			"key1":     "value1",
		},
	}

	expectedOutputNewLogger := []map[string]interface{}{
		{
			"@level":   "trace",
			"@message": "new logger",
			"@module":  "provider",
			"key1":     "value1",
			"key2":     "value2",
		},
	}

	originalCtx := context.Background()
	originalCtx = loggertest.ProviderRoot(originalCtx, &outputBuffer)
	originalCtx = tflog.SetField(originalCtx, key1, value1)

	newCtx := tflog.SetField(originalCtx, key2, value2)

	tflog.Trace(newCtx, "new logger")

	got, err := loggertest.MultilineJSONDecode(&outputBuffer)

	if err != nil {
		t.Fatalf("unable to read multiple line JSON: %s", err)
	}

	if diff := cmp.Diff(expectedOutputNewLogger, got); diff != "" {
		t.Errorf("unexpected output difference: %s", diff)
	}

	tflog.Trace(originalCtx, "original logger")

	got, err = loggertest.MultilineJSONDecode(&outputBuffer)

	if err != nil {
		t.Fatalf("unable to read multiple line JSON: %s", err)
	}

	if diff := cmp.Diff(expectedOutputOriginalLogger, got); diff != "" {
		t.Errorf("unexpected output difference: %s", diff)
	}
}

Expected Behavior

The log entry generated by calling tflog.Trace(originalCtx, "original logger") should only have the field key1 set and not have the field set key2 set.

Actual Behavior

Both key1 and key2 are set when calling tflog.Trace(originalCtx, "original logger")

--- FAIL: TestSetFieldSequential (0.00s)
    provider_test.go:158: unexpected output difference:   []map[string]any{
          	{
          		... // 2 identical entries
          		"@module": string("provider"),
          		"key1":    string("value1"),
        + 		"key2":    string("value2"),
          	},
          }

Steps to Reproduce

1. Run the included test TestSetFieldSequential

References

This may be related to #126, since the field map is being modified in-place.

[bflad]

This could be happening because LoggerOpts has an underlying map[string]any field that isn't particularly handled special when modifying. It therefore can become a single memory reference can potentially be shared across multiple LoggerOpts. This will require a code fix in this Go module to copy the map (or potentially more appropriately, the whole LoggerOpts) first, before any modifications.

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.