Trace-agent doesn't forward 404s to tracers (#21016)

* Trace-agent forward 404s to tracers

When remote configuration is in a failed state the trace
agent always returned 500s

* Simplify code that can't file

* Added release note

* Removed log message

* Update releasenotes/notes/fix-tracer-agent-forwards-remote-config-errors-1e939be195e6db67.yaml

Co-authored-by: Ahmed Mezghani <[email protected]>

* Added Unit Test

* Close body

* Remove returned error handling

---------

Co-authored-by: Ahmed Mezghani <[email protected]>

cmd/agent/subcommands/run/command.go

@@ -481,8 +481,8 @@ func startAgent(
 		configService, err = remoteconfig.NewService()
 		if err != nil {
 			log.Errorf("Failed to initialize config management service: %s", err)
-		} else if err := configService.Start(context.Background()); err != nil {
-			log.Errorf("Failed to start config management service: %s", err)
+		} else {
+			configService.Start(context.Background())
 		}
 
 		if err := rcclient.Start("core-agent"); err != nil {

cmd/cluster-agent/subcommands/start/command.go

@@ -444,9 +444,7 @@ func initializeRemoteConfig(ctx context.Context) (*remote.Client, error) {
 		return nil, fmt.Errorf("unable to create remote-config service: %w", err)
 	}
 
-	if err := configService.Start(ctx); err != nil {
-		return nil, fmt.Errorf("unable to start remote-config service: %w", err)
-	}
+	configService.Start(ctx)
 
 	rcClient, err := remote.NewClient("cluster-agent", configService, version.AgentVersion, []data.Product{data.ProductAPMTracing}, time.Second*5)
 	if err != nil {

cmd/trace-agent/config/remote/config.go

@@ -22,6 +22,8 @@ import (
 	"github.com/DataDog/datadog-agent/pkg/trace/metrics/timing"
 	"github.com/DataDog/datadog-agent/pkg/trace/traceutil"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 var bufferPool = sync.Pool{
@@ -74,7 +76,14 @@ func ConfigHandler(r *api.HTTPReceiver, client remote.ConfigUpdater, cfg *config
 		}
 		cfg, err := client.ClientGetConfigs(req.Context(), &configsRequest)
 		if err != nil {
-			statusCode = http.StatusInternalServerError
+			if e, ok := status.FromError(err); ok {
+				switch e.Code() {
+				case codes.Unimplemented, codes.NotFound:
+					statusCode = http.StatusNotFound
+				}
+			} else {
+				statusCode = http.StatusInternalServerError
+			}
 			http.Error(w, err.Error(), statusCode)
 			return
 		}

cmd/trace-agent/config/remote/config_test.go

@@ -22,6 +22,9 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestConfigEndpoint(t *testing.T) {
@@ -153,12 +156,36 @@ func TestUpstreamRequest(t *testing.T) {
 	}
 }
 
+func TestForwardErrors(t *testing.T) {
+	assert := assert.New(t)
+	grpc := agentGRPCConfigFetcher{}
+	rcv := api.NewHTTPReceiver(config.New(), sampler.NewDynamicConfig(), make(chan *api.Payload, 5000), nil, telemetry.NewNoopCollector())
+
+	grpc.On("ClientGetConfigs", mock.Anything, mock.Anything, mock.Anything).
+		Return(nil, status.Error(codes.Unimplemented, "not implemented"))
+
+	mux := http.NewServeMux()
+	mux.Handle("/v0.7/config", ConfigHandler(rcv, &grpc, &config.AgentConfig{}))
+	server := httptest.NewServer(mux)
+
+	req, _ := http.NewRequest("POST", server.URL+"/v0.7/config", strings.NewReader(`{"client":{"id":"test_client","is_tracer":true,"client_tracer":{"service":"test","tags":["foo:bar"]}}}`))
+	req.Header.Set("Datadog-Container-ID", "cid")
+	r, err := http.DefaultClient.Do(req)
+	assert.NoError(err)
+	assert.Equal(404, r.StatusCode)
+	r.Body.Close()
+}
+
 type agentGRPCConfigFetcher struct {
 	pbgo.AgentSecureClient
 	mock.Mock
 }
 
 func (a *agentGRPCConfigFetcher) ClientGetConfigs(ctx context.Context, in *pbgo.ClientGetConfigsRequest) (*pbgo.ClientGetConfigsResponse, error) {
 	args := a.Called(ctx, in)
-	return args.Get(0).(*pbgo.ClientGetConfigsResponse), args.Error(1)
+	var maybeResponse *pbgo.ClientGetConfigsResponse
+	if args.Get(0) != nil {
+		maybeResponse = args.Get(0).(*pbgo.ClientGetConfigsResponse)
+	}
+	return maybeResponse, args.Error(1)
 }

pkg/config/remote/service/service.go

@@ -270,7 +270,7 @@ func newRCBackendOrgUUIDProvider(http api.API) uptane.OrgUUIDProvider {
 }
 
 // Start the remote configuration management service
-func (s *Service) Start(ctx context.Context) error {
+func (s *Service) Start(ctx context.Context) {
 	ctx, cancel := context.WithCancel(ctx)
 	s.cancel = cancel
 	go func() {
@@ -324,7 +324,6 @@ func (s *Service) Start(ctx context.Context) error {
 			}
 		}
 	}()
-	return nil
 }
 
 func (s *Service) Stop() error {

releasenotes/notes/fix-tracer-agent-forwards-remote-config-errors-1e939be195e6db67.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    APM: Fixed trace-agent not forwarding errors from remote configuration and reporting them all as 500s
+