Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "chore(apm): consolidate External Data Origin Detection (#33703)" #33793

Merged
merged 1 commit into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading