From 64b5659943222549d55936dd81fff7ce7f0aee6f Mon Sep 17 00:00:00 2001
From: Eliott Bouhana <47679741+eliottness@users.noreply.github.com>
Date: Tue, 26 Nov 2024 14:10:46 +0100
Subject: [PATCH] fix(injector: builtin: net-http-client): treat 4XX as errors,
 not 5XXs (#426)

Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Co-authored-by: Romain Marcadier <romain@datadoghq.com>
---
 .../tests/net_http/client_error.go            | 67 +++++++++++++++++++
 _integration-tests/tests/net_http/gen_test.go |  4 ++
 .../builtin/yaml/stdlib/net-http.client.yml   |  4 +-
 3 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 _integration-tests/tests/net_http/client_error.go

diff --git a/_integration-tests/tests/net_http/client_error.go b/_integration-tests/tests/net_http/client_error.go
new file mode 100644
index 00000000..f3ded49c
--- /dev/null
+++ b/_integration-tests/tests/net_http/client_error.go
@@ -0,0 +1,67 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2023-present Datadog, Inc.
+
+package nethttp
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"testing"
+	"time"
+
+	"datadoghq.dev/orchestrion/_integration-tests/utils"
+	"datadoghq.dev/orchestrion/_integration-tests/validator/trace"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestCaseClientError checks of the net/http client instrumentation handles creates error if the returned status code is 4xx.
+type TestCaseClientError struct {
+	srv     *http.Server
+	handler http.Handler
+}
+
+func (b *TestCaseClientError) Setup(t *testing.T) {
+	b.srv = &http.Server{
+		Addr:         "127.0.0.1:" + utils.GetFreePort(t),
+		ReadTimeout:  5 * time.Second,
+		WriteTimeout: 10 * time.Second,
+	}
+	b.srv.Handler = http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusTeapot)
+	})
+
+	go func() { assert.ErrorIs(t, b.srv.ListenAndServe(), http.ErrServerClosed) }()
+	t.Cleanup(func() {
+		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+		defer cancel()
+		assert.NoError(t, b.srv.Shutdown(ctx))
+	})
+}
+
+func (b *TestCaseClientError) Run(t *testing.T) {
+	resp, err := http.Get(fmt.Sprintf("http://%s/", b.srv.Addr))
+	require.NoError(t, err)
+	require.Equal(t, http.StatusTeapot, resp.StatusCode)
+}
+
+func (*TestCaseClientError) ExpectedTraces() trace.Traces {
+	return trace.Traces{
+		{
+			Tags: map[string]any{
+				"name":     "http.request",
+				"resource": "GET /",
+				"type":     "http",
+			},
+			Meta: map[string]string{
+				"component":        "net/http",
+				"span.kind":        "client",
+				"http.errors":      "418 I'm a teapot",
+				"http.status_code": "418",
+			},
+		},
+	}
+}
diff --git a/_integration-tests/tests/net_http/gen_test.go b/_integration-tests/tests/net_http/gen_test.go
index 005cca62..35715705 100644
--- a/_integration-tests/tests/net_http/gen_test.go
+++ b/_integration-tests/tests/net_http/gen_test.go
@@ -14,6 +14,10 @@ import (
 	"testing"
 )
 
+func TestIntegration_nethttp_ClientError(t *testing.T) {
+	utils.RunTest(t, new(TestCaseClientError))
+}
+
 func TestIntegration_nethttp_FuncHandler(t *testing.T) {
 	utils.RunTest(t, new(TestCaseFuncHandler))
 }
diff --git a/internal/injector/builtin/yaml/stdlib/net-http.client.yml b/internal/injector/builtin/yaml/stdlib/net-http.client.yml
index 219d8741..6d55c96c 100644
--- a/internal/injector/builtin/yaml/stdlib/net-http.client.yml
+++ b/internal/injector/builtin/yaml/stdlib/net-http.client.yml
@@ -150,8 +150,8 @@ aspects:
                   span.SetTag(ext.Error, {{ $err }})
                 } else {
                   span.SetTag(ext.HTTPCode, strconv.Itoa({{ $res }}.StatusCode))
-                  if {{ $res }}.StatusCode >= 500 && {{ $res}}.StatusCode < 600 {
-                    // Treat HTTP 5XX as errors
+                  if {{ $res }}.StatusCode >= 400 && {{ $res}}.StatusCode < 500 {
+                    // Treat HTTP 4XX as errors
                     span.SetTag("http.errors", {{ $res }}.Status)
                     span.SetTag(ext.Error, fmt.Errorf("%d: %s", {{ $res }}.StatusCode, StatusText({{ $res }}.StatusCode)))
                   }