Skip to content

Commit 6df76c3

Browse files
authored
[ AGNTLOG-623 ] copy Encoding field in container-to-file source translation and add guard test (#49944)
### What does this PR do? Fixes silent field loss in the container-to-file `LogsConfig` translation performed by `makeDockerFileSource` and `makeK8sFileSource` in `pkg/logs/launchers/container/tailerfactory/file.go`. **Bug fix:** Adds the missing `Encoding` field to both copy sites. Previously, if a user configured `encoding: utf-16-le` via a pod annotation or container label, the setting was silently dropped when the container source was translated to a file source — no error, no warning. **Guard test:** Adds `TestLogsConfigFieldCoverage`, a reflection-based test that automatically detects when a new field is added to `LogsConfig` but not accounted for in the copy logic. The test maintains an exclusion list of fields intentionally not copied (network, journald, Windows Event, Docker identity, channel fields), each with a reason. Any new field that is non-zero on the input but zero on the output and not in the exclusion list causes a test failure with a clear message telling the developer what to do. ### Motivation The `LogsConfig` struct has ~40 fields spanning many source types. The container-to-file translation uses Go struct literals, where omitted fields silently zero-initialize. This has caused repeated silent regressions — `Encoding` was one such missing field. The guard test makes this class of bug impossible to introduce without a test failure. ### Describe how you validated your changes - Ran `dda inv test --targets=./pkg/logs/launchers/container/tailerfactory` — all 57 tests pass. - Verified the guard test catches omissions by temporarily removing `Encoding` from the copy and confirming the test fails with: `Missing fields: [Encoding]`. - Verified the meta-test (`TestLogsConfigFieldCoverage_detectsMissingField`) independently validates the guard works. Co-authored-by: ryan.hall <ryan.hall@datadoghq.com>
1 parent 59d9551 commit 6df76c3

2 files changed

Lines changed: 307 additions & 0 deletions

File tree

pkg/logs/launchers/container/tailerfactory/file.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ func (tf *factory) makeDockerFileSource(source *sources.LogSource) (*sources.Log
140140
Path: path,
141141
Service: serviceName,
142142
Source: sourceName,
143+
Encoding: source.Config.Encoding,
144+
SIEMParsing: source.Config.SIEMParsing,
143145
Tags: source.Config.Tags,
144146
ProcessingRules: source.Config.ProcessingRules,
145147
FingerprintConfig: source.Config.FingerprintConfig,
@@ -255,6 +257,8 @@ func (tf *factory) makeK8sFileSource(source *sources.LogSource) (*sources.LogSou
255257
Path: path,
256258
Service: serviceName,
257259
Source: sourceName,
260+
Encoding: source.Config.Encoding,
261+
SIEMParsing: source.Config.SIEMParsing,
258262
Tags: source.Config.Tags,
259263
ProcessingRules: source.Config.ProcessingRules,
260264
FingerprintConfig: source.Config.FingerprintConfig,

pkg/logs/launchers/container/tailerfactory/file_test.go

Lines changed: 303 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ package tailerfactory
1111

1212
import (
1313
"context"
14+
"fmt"
1415
"os"
1516
"path/filepath"
17+
"reflect"
1618
"runtime"
1719
"testing"
1820

@@ -30,6 +32,7 @@ import (
3032
configmock "github.com/DataDog/datadog-agent/pkg/config/mock"
3133
"github.com/DataDog/datadog-agent/pkg/logs/internal/util/containersorpods"
3234
"github.com/DataDog/datadog-agent/pkg/logs/sources"
35+
"github.com/DataDog/datadog-agent/pkg/logs/types"
3336
"github.com/DataDog/datadog-agent/pkg/util/fxutil"
3437
pkglog "github.com/DataDog/datadog-agent/pkg/util/log"
3538
"github.com/DataDog/datadog-agent/pkg/util/option"
@@ -561,3 +564,303 @@ func TestGetPodAndContainer_pod_not_found(t *testing.T) {
561564
require.Nil(t, pod)
562565
require.ErrorContains(t, err, "cannot find pod for container")
563566
}
567+
568+
// fullyPopulatedLogsConfig returns a LogsConfig with every exported, settable
569+
// field set to a distinguishable non-zero value. The reflection walk ensures
570+
// that newly added fields are automatically populated without manual updates.
571+
func fullyPopulatedLogsConfig() *config.LogsConfig {
572+
cfg := &config.LogsConfig{}
573+
v := reflect.ValueOf(cfg).Elem()
574+
t := v.Type()
575+
576+
for i := 0; i < t.NumField(); i++ {
577+
field := v.Field(i)
578+
if !field.CanSet() {
579+
continue
580+
}
581+
setNonZero(field, t.Field(i).Name)
582+
}
583+
return cfg
584+
}
585+
586+
// setNonZero sets a reflect.Value to a non-zero sentinel appropriate for its kind.
587+
func setNonZero(v reflect.Value, name string) {
588+
switch v.Kind() {
589+
case reflect.String:
590+
v.SetString(name + "_val")
591+
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
592+
v.SetInt(42)
593+
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
594+
v.SetUint(42)
595+
case reflect.Float32, reflect.Float64:
596+
v.SetFloat(0.42)
597+
case reflect.Bool:
598+
v.SetBool(true)
599+
case reflect.Ptr:
600+
elem := reflect.New(v.Type().Elem())
601+
setNonZero(elem.Elem(), name)
602+
v.Set(elem)
603+
case reflect.Slice:
604+
elemType := v.Type().Elem()
605+
elem := reflect.New(elemType).Elem()
606+
setNonZero(elem, name)
607+
v.Set(reflect.Append(reflect.MakeSlice(v.Type(), 0, 1), elem))
608+
case reflect.Chan:
609+
v.Set(reflect.MakeChan(v.Type(), 1))
610+
case reflect.Struct:
611+
for i := 0; i < v.NumField(); i++ {
612+
f := v.Field(i)
613+
if f.CanSet() {
614+
setNonZero(f, v.Type().Field(i).Name)
615+
}
616+
}
617+
case reflect.Map:
618+
v.Set(reflect.MakeMap(v.Type()))
619+
}
620+
}
621+
622+
// TestLogsConfigFieldCoverage verifies that every field on LogsConfig is either
623+
// copied by the container-to-file translation functions or explicitly listed in
624+
// the exclusion set. When a new field is added to LogsConfig, this test fails
625+
// unless the developer either copies it or adds it to the exclusion list.
626+
func TestLogsConfigFieldCoverage(t *testing.T) {
627+
// Fields intentionally NOT copied from container source to file source.
628+
// If you add a new field to LogsConfig, you MUST either:
629+
// (a) copy it in makeDockerFileSource and makeK8sFileSource, or
630+
// (b) add it here with a comment explaining why it doesn't apply to file tailing.
631+
excludedFromFileCopy := map[string]string{
632+
// Hardcoded or computed differently per function
633+
"Type": "hardcoded to FileType",
634+
"Path": "computed from container ID / pod metadata",
635+
"Source": "computed via defaultSourceAndService",
636+
637+
// Network fields (tcp/udp only)
638+
"Port": "network source only",
639+
"BindHost": "network source only",
640+
"IdleTimeout": "network source only",
641+
"MaxConnections": "network source only",
642+
"TLS": "network source only",
643+
"AllowedIPs": "network source only",
644+
"DeniedIPs": "network source only",
645+
646+
// Journald fields
647+
"ConfigID": "journald only",
648+
"IncludeSystemUnits": "journald only",
649+
"ExcludeSystemUnits": "journald only",
650+
"IncludeUserUnits": "journald only",
651+
"ExcludeUserUnits": "journald only",
652+
"IncludeMatches": "journald only",
653+
"ExcludeMatches": "journald only",
654+
"ContainerMode": "journald only",
655+
"DefaultApplicationName": "journald only",
656+
657+
// Docker identity fields (container metadata, not log content config)
658+
"Image": "docker container identity, not log config",
659+
"Label": "docker container identity, not log config",
660+
"Name": "docker container identity, not log config",
661+
662+
// Windows Event fields
663+
"ChannelPath": "windows event only",
664+
"Query": "windows event only",
665+
666+
// Channel tailer fields
667+
"Channel": "internal channel tailer only",
668+
"ChannelTags": "internal channel tailer only",
669+
"ChannelTagsMutex": "internal channel tailer only (sync.Mutex)",
670+
671+
// File-only fields not relevant to container-sourced file tailing
672+
"ExcludePaths": "file wildcard exclusion, not applicable to container log paths",
673+
674+
// Integration metadata (set by integration config loader, not user config)
675+
"IntegrationName": "integration loader metadata",
676+
"IntegrationSource": "integration loader metadata",
677+
"IntegrationSourceIndex": "integration loader metadata",
678+
679+
// Structured-log tailer fields (journald, windows event only)
680+
"ProcessRawMessage": "only affects structured-message tailers (journald, windows event); file tailer always emits unstructured messages",
681+
682+
// Misc fields not relevant to container-to-file
683+
"SourceCategory": "deprecated/unused in container context",
684+
}
685+
686+
inputCfg := fullyPopulatedLogsConfig()
687+
688+
// Override fields that the functions expect specific values for
689+
inputCfg.Type = "docker"
690+
inputCfg.Identifier = "abc"
691+
692+
fileTestSetup(t)
693+
694+
// Create the docker log file so makeDockerFileSource can open it
695+
dockerPath := filepath.Join(platformDockerLogsBasePath, filepath.FromSlash("containers/abc/abc-json.log"))
696+
require.NoError(t, os.MkdirAll(filepath.Dir(dockerPath), 0o777))
697+
require.NoError(t, os.WriteFile(dockerPath, []byte("{}"), 0o666))
698+
699+
// Set up K8s workloadmeta so makeK8sFileSource can resolve the pod
700+
store := fxutil.Test[workloadmetamock.Mock](t, fx.Options(
701+
fx.Provide(func() log.Component { return logmock.New(t) }),
702+
fx.Provide(func() compConfig.Component { return compConfig.NewMock(t) }),
703+
fx.Supply(context.Background()),
704+
workloadmetafxmock.MockModule(workloadmeta.NewParams()),
705+
))
706+
pod, container := makeTestPod()
707+
store.Set(pod)
708+
store.Set(container)
709+
710+
// Create the K8s log directory
711+
k8sDir := filepath.Join(podLogsBasePath, filepath.FromSlash("podns_podname_poduuid/cname"))
712+
require.NoError(t, os.MkdirAll(k8sDir, 0o777))
713+
require.NoError(t, os.WriteFile(filepath.Join(k8sDir, "0.log"), []byte("{}"), 0o666))
714+
715+
type testCase struct {
716+
name string
717+
makeSource func(*sources.LogSource) (*sources.LogSource, error)
718+
}
719+
720+
dockerFactory := &factory{
721+
pipelineProvider: pipeline.NewMockProvider(),
722+
cop: containersorpods.NewDecidedChooser(containersorpods.LogContainers),
723+
dockerUtilGetter: &dockerUtilGetterImpl{},
724+
}
725+
726+
k8sFactory := &factory{
727+
pipelineProvider: pipeline.NewMockProvider(),
728+
cop: containersorpods.NewDecidedChooser(containersorpods.LogPods),
729+
dockerUtilGetter: &dockerUtilGetterImpl{},
730+
workloadmetaStore: option.New[workloadmeta.Component](store),
731+
}
732+
733+
cases := []testCase{
734+
{"makeDockerFileSource", dockerFactory.makeDockerFileSource},
735+
{"makeK8sFileSource", k8sFactory.makeK8sFileSource},
736+
}
737+
738+
for _, tc := range cases {
739+
t.Run(tc.name, func(t *testing.T) {
740+
source := sources.NewLogSource("test", inputCfg)
741+
child, err := tc.makeSource(source)
742+
require.NoError(t, err)
743+
744+
checkFieldCoverage(t, inputCfg, child.Config, excludedFromFileCopy)
745+
})
746+
}
747+
}
748+
749+
// checkFieldCoverage iterates over every exported field on LogsConfig and
750+
// verifies that non-zero input fields are either non-zero on the output or
751+
// present in the exclusion set. It also checks for stale exclusion entries.
752+
func checkFieldCoverage(t *testing.T, input, output *config.LogsConfig, excluded map[string]string) {
753+
t.Helper()
754+
755+
inVal := reflect.ValueOf(input).Elem()
756+
outVal := reflect.ValueOf(output).Elem()
757+
structType := inVal.Type()
758+
759+
allFields := make(map[string]bool, structType.NumField())
760+
var missing []string
761+
762+
for i := 0; i < structType.NumField(); i++ {
763+
fieldName := structType.Field(i).Name
764+
allFields[fieldName] = true
765+
766+
inField := inVal.Field(i)
767+
outField := outVal.Field(i)
768+
769+
if !inField.CanInterface() || !outField.CanInterface() {
770+
continue
771+
}
772+
773+
if _, ok := excluded[fieldName]; ok {
774+
continue
775+
}
776+
777+
if inField.IsZero() {
778+
continue
779+
}
780+
781+
if outField.IsZero() {
782+
missing = append(missing, fieldName)
783+
}
784+
}
785+
786+
if len(missing) > 0 {
787+
t.Errorf("LogsConfig fields are set on the container source but missing from the file source output.\n"+
788+
"Either copy them in makeDockerFileSource/makeK8sFileSource, or add them to\n"+
789+
"excludedFromFileCopy in TestLogsConfigFieldCoverage with a reason.\n"+
790+
"Missing fields: %v", missing)
791+
}
792+
793+
for fieldName := range excluded {
794+
if !allFields[fieldName] {
795+
t.Errorf("Stale entry in excludedFromFileCopy: %q is no longer a field on LogsConfig. Remove it.", fieldName)
796+
}
797+
}
798+
}
799+
800+
// TestLogsConfigFieldCoverage_detectsMissingField is a meta-test that verifies
801+
// the guard logic catches a newly added field that is set on input but missing
802+
// from output. It simulates the bug by building an output config that
803+
// deliberately omits Encoding, then checks that Encoding appears in the
804+
// missing-fields list.
805+
func TestLogsConfigFieldCoverage_detectsMissingField(t *testing.T) {
806+
excluded := map[string]string{
807+
"Type": "hardcoded", "Path": "computed", "Source": "computed", "Service": "computed",
808+
"Port": "n/a", "BindHost": "n/a", "IdleTimeout": "n/a", "MaxConnections": "n/a",
809+
"TLS": "n/a", "AllowedIPs": "n/a", "DeniedIPs": "n/a",
810+
"ConfigID": "n/a", "IncludeSystemUnits": "n/a", "ExcludeSystemUnits": "n/a",
811+
"IncludeUserUnits": "n/a", "ExcludeUserUnits": "n/a",
812+
"IncludeMatches": "n/a", "ExcludeMatches": "n/a",
813+
"ContainerMode": "n/a", "DefaultApplicationName": "n/a",
814+
"Image": "n/a", "Label": "n/a", "Name": "n/a",
815+
"ChannelPath": "n/a", "Query": "n/a",
816+
"Channel": "n/a", "ChannelTags": "n/a", "ChannelTagsMutex": "n/a",
817+
"ExcludePaths": "n/a",
818+
"IntegrationName": "n/a", "IntegrationSource": "n/a", "IntegrationSourceIndex": "n/a",
819+
"SourceCategory": "n/a",
820+
}
821+
822+
input := fullyPopulatedLogsConfig()
823+
output := &config.LogsConfig{
824+
Type: config.FileType,
825+
TailingMode: input.TailingMode,
826+
Identifier: input.Identifier,
827+
Path: "some/path",
828+
Service: input.Service,
829+
Source: input.Source,
830+
Tags: input.Tags,
831+
ProcessingRules: input.ProcessingRules,
832+
FingerprintConfig: &types.FingerprintConfig{
833+
FingerprintStrategy: "md5",
834+
},
835+
}
836+
837+
inVal := reflect.ValueOf(input).Elem()
838+
outVal := reflect.ValueOf(output).Elem()
839+
structType := inVal.Type()
840+
841+
var missing []string
842+
for i := 0; i < structType.NumField(); i++ {
843+
fieldName := structType.Field(i).Name
844+
inField := inVal.Field(i)
845+
outField := outVal.Field(i)
846+
if !inField.CanInterface() || !outField.CanInterface() {
847+
continue
848+
}
849+
if _, ok := excluded[fieldName]; ok {
850+
continue
851+
}
852+
if !inField.IsZero() && outField.IsZero() {
853+
missing = append(missing, fieldName)
854+
}
855+
}
856+
857+
require.NotEmpty(t, missing, "Expected to detect missing fields")
858+
found := false
859+
for _, name := range missing {
860+
if name == "Encoding" {
861+
found = true
862+
break
863+
}
864+
}
865+
require.True(t, found, fmt.Sprintf("Expected 'Encoding' in missing fields, got: %v", missing))
866+
}

0 commit comments

Comments
 (0)