Skip to content

Commit

Permalink
fix(injector: builtin: net-http-client): treat 4XX as errors, not 5XXs (
Browse files Browse the repository at this point in the history
#426)

Signed-off-by: Eliott Bouhana <[email protected]>
Co-authored-by: Romain Marcadier <[email protected]>
  • Loading branch information
eliottness and RomainMuller authored Nov 26, 2024
1 parent f0c5d95 commit 64b5659
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 2 deletions.
67 changes: 67 additions & 0 deletions _integration-tests/tests/net_http/client_error.go
Original file line number Diff line number Diff line change
@@ -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",
},
},
}
}
4 changes: 4 additions & 0 deletions _integration-tests/tests/net_http/gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/injector/builtin/yaml/stdlib/net-http.client.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
Expand Down

0 comments on commit 64b5659

Please sign in to comment.