Skip to content

Commit

Permalink
Revert "chore(apm): consolidate External Data Origin Detection (#33703)"
Browse files Browse the repository at this point in the history
This reverts commit cfe926d.
  • Loading branch information
Pythyu committed Feb 6, 2025
1 parent 987338d commit 28809de
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 62 deletions.
8 changes: 1 addition & 7 deletions comp/core/tagger/impl-remote/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,14 +318,8 @@ func (t *remoteTagger) queryContainerIDFromOriginInfo(originInfo origindetection
)
defer queryCancel()

// Call the gRPC method to get the container ID from the OriginInfo.
// Call the gRPC method to get the container ID from the origin info
containerIDResponse, err := t.client.TaggerGenerateContainerIDFromOriginInfo(queryCtx, &pb.GenerateContainerIDFromOriginInfoRequest{
LocalData: &pb.GenerateContainerIDFromOriginInfoRequest_LocalData{
ProcessID: &originInfo.LocalData.ProcessID,
ContainerID: &originInfo.LocalData.ContainerID,
Inode: &originInfo.LocalData.Inode,
PodUID: &originInfo.LocalData.PodUID,
},
ExternalData: &pb.GenerateContainerIDFromOriginInfoRequest_ExternalData{
Init: &originInfo.ExternalData.Init,
ContainerName: &originInfo.ExternalData.ContainerName,
Expand Down
9 changes: 2 additions & 7 deletions comp/core/tagger/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,15 +197,10 @@ func (s *Server) TaggerFetchEntity(_ context.Context, in *pb.FetchEntityRequest)
}, nil
}

// TaggerGenerateContainerIDFromOriginInfo requests the Tagger to generate a container ID from the given OriginInfo.
// TaggerGenerateContainerIDFromOriginInfo request the generation of a container ID from external data from the Tagger.
// This function takes an Origin Info but only uses the ExternalData part of it, this is done for backward compatibility.
func (s *Server) TaggerGenerateContainerIDFromOriginInfo(_ context.Context, in *pb.GenerateContainerIDFromOriginInfoRequest) (*pb.GenerateContainerIDFromOriginInfoResponse, error) {
generatedContainerID, err := s.taggerComponent.GenerateContainerIDFromOriginInfo(origindetection.OriginInfo{
LocalData: origindetection.LocalData{
ProcessID: *in.LocalData.ProcessID,
ContainerID: *in.LocalData.ContainerID,
Inode: *in.LocalData.Inode,
PodUID: *in.LocalData.PodUID,
},
ExternalData: origindetection.ExternalData{
Init: *in.ExternalData.Init,
ContainerName: *in.ExternalData.ContainerName,
Expand Down
65 changes: 35 additions & 30 deletions pkg/trace/api/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,27 +132,20 @@ type cgroupIDProvider struct {
containerIDFromOriginInfo func(originInfo origindetection.OriginInfo) (string, error)
}

// GetContainerID retrieves the container ID associated with the given request.
//
// The container ID can be determined from multiple sources in the following order:
// 1. **Local Data Header** (`LocalData`): If present, it is parsed to extract the container ID or inode.
// If an inode is found instead of a container ID, it is resolved to a container ID.
// 2. **Datadog-Container-ID Header**: A deprecated fallback used for backward compatibility.
// 3. **Process Context (PID)**: If no container ID is found in headers, the function attempts
// to resolve it using the PID from the provided context, checking cgroups.
// 4. **External Data Header** (`ExternalData`): If present, it is parsed as an additional source.
//
// If none of the direct methods return a valid container ID, an attempt is made to generate one
// based on the collected OriginInfo.
// GetContainerID returns the container ID.
// The Container ID can come from either http headers or the context:
// * Local Data header
// * Datadog-Container-ID header
// * Looks for a PID in the ctx which is used to search cgroups for a container ID.
func (c *cgroupIDProvider) GetContainerID(ctx context.Context, h http.Header) string {
originInfo := origindetection.OriginInfo{ProductOrigin: origindetection.ProductOriginAPM}

// Parse LocalData from the headers.
if localData := h.Get(header.LocalData); localData != "" {
// Retrieve container ID from Local Data header
if localDataString := h.Get(header.LocalData); localDataString != "" {
var err error
originInfo.LocalData, err = origindetection.ParseLocalData(localData)
originInfo.LocalData, err = origindetection.ParseLocalData(localDataString)
if err != nil {
log.Errorf("Could not parse local data (%s): %v", localData, err)
log.Errorf("Could not parse local data (%s): %v", localDataString, err)
}

if originInfo.LocalData.ContainerID != "" {
Expand All @@ -163,31 +156,22 @@ func (c *cgroupIDProvider) GetContainerID(ctx context.Context, h http.Header) st
}

// Retrieve container ID from Datadog-Container-ID header.
// Deprecated in favor of LocalData header. This is kept for backward compatibility with older libraries.
// Deprecated in favor of Local Data header. This is kept for backward compatibility with older libraries.
if containerIDFromHeader := h.Get(header.ContainerID); containerIDFromHeader != "" {
return containerIDFromHeader
}

// Retrieve the container-id from the pid in its context.
// Retrieve the container-id from the pid in its context
if containerID := c.resolveContainerIDFromContext(ctx); containerID != "" {
return containerID
}

// Parse ExternalData from the headers.
// Retrieve container ID from External Data header
if externalData := h.Get(header.ExternalData); externalData != "" {
var err error
originInfo.ExternalData, err = origindetection.ParseExternalData(externalData)
if err != nil {
log.Errorf("Could not parse external data (%s): %v", externalData, err)
}
return c.resolveContainerIDFromExternalData(externalData)
}

// Generate container ID from OriginInfo.
generatedContainerID, err := c.containerIDFromOriginInfo(originInfo)
if err != nil {
log.Errorf("Could not generate container ID from OriginInfo: %+v, err: %v", originInfo, err)
}
return generatedContainerID
return ""
}

// resolveContainerIDFromInode returns the container ID for the given cgroupv2 inode.
Expand Down Expand Up @@ -267,6 +251,27 @@ func (c *cgroupIDProvider) getCachedContainerID(key string, retrievalFunc func()
return val, nil
}

// resolveContainerIDFromExternalData returns the container ID for the given External Data.
func (c *cgroupIDProvider) resolveContainerIDFromExternalData(rawExternalData string) string {
var generatedContainerID string

externalData, err := origindetection.ParseExternalData(rawExternalData)
if err != nil {
log.Errorf("Could not parse external data (%s): %v", rawExternalData, err)
return ""
}
generatedContainerID, err = c.containerIDFromOriginInfo(origindetection.OriginInfo{
ExternalData: externalData,
ProductOrigin: origindetection.ProductOriginAPM,
})
if err != nil {
log.Errorf("Could not generate container ID from external data (%s): %v", rawExternalData, err)
return ""
}

return generatedContainerID
}

// The below cache is copied from /pkg/util/containers/v2/metrics/provider/cache.go. It is not
// imported to avoid making the datadog-agent module a dependency of the pkg/trace module. The
// datadog-agent module contains replace directives which are not inherited by packages that
Expand Down
3 changes: 0 additions & 3 deletions pkg/trace/api/container_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (

pb "github.com/DataDog/datadog-agent/pkg/proto/pbgo/trace"
"github.com/DataDog/datadog-agent/pkg/trace/api/internal/header"
"github.com/DataDog/datadog-agent/pkg/trace/config"
"github.com/DataDog/datadog-agent/pkg/trace/testutil"

"github.com/DataDog/datadog-go/v5/statsd"
Expand Down Expand Up @@ -205,8 +204,6 @@ func TestGetContainerID(t *testing.T) {
})
}

// Test invalid LocalData headers
provider.containerIDFromOriginInfo = config.NoopContainerIDFromOriginInfoFunc
invalidLocalDataLists := []string{
"",
",",
Expand Down
2 changes: 1 addition & 1 deletion pkg/trace/api/pipeline_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestPipelineStatsProxy(t *testing.T) {
t.Fatal(err)
}
rec := httptest.NewRecorder()
c := &config.AgentConfig{ContainerIDFromOriginInfo: config.NoopContainerIDFromOriginInfoFunc}
c := &config.AgentConfig{}
newPipelineStatsProxy(c, []*url.URL{u}, []string{"123"}, "key:val", &statsd.NoOpClient{}).ServeHTTP(rec, req)
result := rec.Result()
slurp, err := io.ReadAll(result.Body)
Expand Down
2 changes: 1 addition & 1 deletion pkg/trace/api/profiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestProfileProxy(t *testing.T) {
t.Fatal(err)
}
rec := httptest.NewRecorder()
c := &config.AgentConfig{ContainerIDFromOriginInfo: config.NoopContainerIDFromOriginInfoFunc}
c := &config.AgentConfig{}
newProfileProxy(c, []*url.URL{u}, []string{"123"}, "key:val", &statsd.NoOpClient{}).ServeHTTP(rec, req)
result := rec.Result()
slurp, err := io.ReadAll(result.Body)
Expand Down
15 changes: 3 additions & 12 deletions pkg/trace/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,10 +564,9 @@ func New() *AgentConfig {

GlobalTags: computeGlobalTags(),

Proxy: http.ProxyFromEnvironment,
OTLPReceiver: &OTLP{},
ContainerTags: noopContainerTagsFunc,
ContainerIDFromOriginInfo: NoopContainerIDFromOriginInfoFunc,
Proxy: http.ProxyFromEnvironment,
OTLPReceiver: &OTLP{},
ContainerTags: noopContainerTagsFunc,
TelemetryConfig: &TelemetryConfig{
Endpoints: []*Endpoint{{Host: TelemetryEndpointPrefix + "datadoghq.com"}},
},
Expand Down Expand Up @@ -596,14 +595,6 @@ func noopContainerTagsFunc(_ string) ([]string, error) {
return nil, ErrContainerTagsFuncNotDefined
}

// ErrContainerIDFromOriginInfoFuncNotDefined is returned when the ContainerIDFromOriginInfo function is not defined.
var ErrContainerIDFromOriginInfoFuncNotDefined = errors.New("ContainerIDFromOriginInfo function not defined")

// NoopContainerIDFromOriginInfoFunc is used when the ContainerIDFromOriginInfo function is not defined.
func NoopContainerIDFromOriginInfoFunc(_ origindetection.OriginInfo) (string, error) {
return "", ErrContainerIDFromOriginInfoFuncNotDefined
}

// APIKey returns the first (main) endpoint's API key.
func (c *AgentConfig) APIKey() string {
if len(c.Endpoints) == 0 {
Expand Down
1 change: 0 additions & 1 deletion pkg/trace/info/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ func TestInfoConfig(t *testing.T) {
conf.ProfilingProxy.AdditionalEndpoints = scrubbedAddEp

conf.ContainerTags = nil
conf.ContainerIDFromOriginInfo = nil

assert.Equal(*conf, confCopy) // ensure all fields have been exported then parsed correctly
}
Expand Down

0 comments on commit 28809de

Please sign in to comment.