From 739be2528b7ad5765f309e1412b59bd6cb1fd195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 5 Feb 2025 16:36:06 +0100 Subject: [PATCH 1/4] feat: implement origin detection (#3124) --- ddtrace/tracer/transport.go | 3 +++ ddtrace/tracer/transport_test.go | 27 +++++++++++++++++++++++++++ internal/container_linux.go | 6 +++--- internal/container_linux_test.go | 2 +- internal/container_stub.go | 2 +- internal/env.go | 5 +++++ internal/telemetry/client_test.go | 9 +++++++++ 7 files changed, 49 insertions(+), 5 deletions(-) diff --git a/ddtrace/tracer/transport.go b/ddtrace/tracer/transport.go index 222f6c8b70..1cf84254ad 100644 --- a/ddtrace/tracer/transport.go +++ b/ddtrace/tracer/transport.go @@ -104,6 +104,9 @@ func newHTTPTransport(url string, client *http.Client) *httpTransport { if eid := internal.EntityID(); eid != "" { defaultHeaders["Datadog-Entity-ID"] = eid } + if extEnv := internal.ExternalEnvironment(); extEnv != "" { + defaultHeaders["Datadog-External-Env"] = extEnv + } return &httpTransport{ traceURL: fmt.Sprintf("%s/v0.4/traces", url), statsURL: fmt.Sprintf("%s/v0.6/stats", url), diff --git a/ddtrace/tracer/transport_test.go b/ddtrace/tracer/transport_test.go index b8967713cf..efcafde18f 100644 --- a/ddtrace/tracer/transport_test.go +++ b/ddtrace/tracer/transport_test.go @@ -396,3 +396,30 @@ func TestWithUDS(t *testing.T) { assert.Len(rt.reqs, 1) assert.Equal(hits, 2) } + +func TestExternalEnvironment(t *testing.T) { + t.Setenv("DD_EXTERNAL_ENV", "it-false,cn-nginx-webserver,pu-75a2b6d5-3949-4afb-ad0d-92ff0674e759") + assert := assert.New(t) + found := false + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + extEnv := r.Header.Get("Datadog-External-Env") + if extEnv == "" { + return + } + assert.Equal("it-false,cn-nginx-webserver,pu-75a2b6d5-3949-4afb-ad0d-92ff0674e759", extEnv) + found = true + })) + defer srv.Close() + + u, err := url.Parse(srv.URL) + assert.NoError(err) + c := &http.Client{} + trc := newTracer(WithAgentTimeout(2), WithAgentAddr(u.Host), WithHTTPClient(c)) + defer trc.Stop() + + p, err := encode(getTestTrace(1, 1)) + assert.NoError(err) + _, err = trc.config.transport.send(p) + assert.NoError(err) + assert.True(found) +} diff --git a/internal/container_linux.go b/internal/container_linux.go index 237c293e21..6ac57029f9 100644 --- a/internal/container_linux.go +++ b/internal/container_linux.go @@ -48,7 +48,7 @@ var ( // containerID is the containerID read at init from /proc/self/cgroup containerID string - // entityID is the entityID to use for the container. It is the `cid-` if the container id available, + // entityID is the entityID to use for the container. It is the `ci-` if the container id available, // otherwise the cgroup node controller's inode prefixed with `in-` or an empty string on incompatible OS. // We use the memory controller on cgroupv1 and the root cgroup on cgroupv2. entityID string @@ -151,7 +151,7 @@ func readEntityID(mountPath, cgroupPath string, isHostCgroupNamespace bool) stri // First try to emit the containerID if available. It will be retrieved if the container is // running in the host cgroup namespace, independently of the cgroup version. if containerID != "" { - return "cid-" + containerID + return "ci-" + containerID } // Rely on the inode if we're not running in the host cgroup namespace. if isHostCgroupNamespace { @@ -161,7 +161,7 @@ func readEntityID(mountPath, cgroupPath string, isHostCgroupNamespace bool) stri } // EntityID attempts to return the container ID or the cgroup node controller's inode if the container ID -// is not available. The cid is prefixed with `cid-` and the inode with `in-`. +// is not available. The cid is prefixed with `ci-` and the inode with `in-`. func EntityID() string { return entityID } diff --git a/internal/container_linux_test.go b/internal/container_linux_test.go index 029ff647c4..83796086f7 100644 --- a/internal/container_linux_test.go +++ b/internal/container_linux_test.go @@ -93,7 +93,7 @@ func TestReadEntityIDPrioritizeCID(t *testing.T) { containerID = "fakeContainerID" eid := readEntityID("", "", true) - assert.Equal(t, "cid-fakeContainerID", eid) + assert.Equal(t, "ci-fakeContainerID", eid) } func TestReadEntityIDFallbackOnInode(t *testing.T) { diff --git a/internal/container_stub.go b/internal/container_stub.go index c6c2487406..38f4e5ce21 100644 --- a/internal/container_stub.go +++ b/internal/container_stub.go @@ -13,7 +13,7 @@ func ContainerID() string { } // EntityID attempts to return the container ID or the cgroup v2 node inode if the container ID is not available. -// The cid is prefixed with `cid-` and the inode with `in-`. +// The cid is prefixed with `ci-` and the inode with `in-`. func EntityID() string { return "" } diff --git a/internal/env.go b/internal/env.go index 2e760526ac..ace2bba238 100644 --- a/internal/env.go +++ b/internal/env.go @@ -133,3 +133,8 @@ func BoolVal(val string, def bool) bool { } return v } + +// ExternalEnvironment returns the value of the DD_EXTERNAL_ENV environment variable. +func ExternalEnvironment() string { + return os.Getenv("DD_EXTERNAL_ENV") +} diff --git a/internal/telemetry/client_test.go b/internal/telemetry/client_test.go index 033b2e277a..45cd4172a3 100644 --- a/internal/telemetry/client_test.go +++ b/internal/telemetry/client_test.go @@ -11,6 +11,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "runtime" "sort" "sync" "testing" @@ -18,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" + "gopkg.in/DataDog/dd-trace-go.v1/internal" logging "gopkg.in/DataDog/dd-trace-go.v1/internal/log" ) @@ -439,11 +441,18 @@ func Test_heartbeatInterval(t *testing.T) { } func TestNoEmptyHeaders(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("skipping test on non-linux OS") + } + if internal.EntityID() == "" || internal.ContainerID() == "" { + t.Skip("skipping test when entity ID and container ID are not available") + } c := &client{} req := c.newRequest(RequestTypeAppStarted) assertNotEmpty := func(header string) { headers := *req.Header vals := headers[header] + assert.Greater(t, len(vals), 0, "header %s should not be empty", header) for _, v := range vals { assert.NotEmpty(t, v, "%s header should not be empty", header) } From d5d974f7802579e4b7e351e01cbaa623a761a1da Mon Sep 17 00:00:00 2001 From: Nayef Ghattas Date: Thu, 6 Feb 2025 08:38:31 +0100 Subject: [PATCH 2/4] ddtrace/tracer/slog: fix slog handler (#3131) --- ddtrace/tracer/slog.go | 58 ++++++++++++++++++++++++++++--------- ddtrace/tracer/slog_test.go | 10 ++++++- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/ddtrace/tracer/slog.go b/ddtrace/tracer/slog.go index a4c9b24bd0..4c67445b73 100644 --- a/ddtrace/tracer/slog.go +++ b/ddtrace/tracer/slog.go @@ -13,11 +13,16 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/log" ) +// groupOrAttrs holds either a group name or a list of slog.Attrs. +type groupOrAttrs struct { + group string // group name if non-empty + attrs []slog.Attr // attrs if non-empty +} + // slogHandler implements the slog.Handler interface to dispatch messages to our // internal logger. type slogHandler struct { - attrs []string - groups []string + goas []groupOrAttrs } func (h slogHandler) Enabled(ctx context.Context, lvl slog.Level) bool { @@ -30,10 +35,30 @@ func (h slogHandler) Enabled(ctx context.Context, lvl slog.Level) bool { } func (h slogHandler) Handle(ctx context.Context, r slog.Record) error { - parts := make([]string, 0, len(h.attrs)+r.NumAttrs()) - parts = append(parts, h.attrs...) + goas := h.goas + + if r.NumAttrs() == 0 { + // If the record has no Attrs, remove groups at the end of the list; they are empty. + for len(goas) > 0 && goas[len(goas)-1].group != "" { + goas = goas[:len(goas)-1] + } + } + + parts := make([]string, 0, len(goas)+r.NumAttrs()) + formatGroup := "" + + for _, goa := range goas { + if goa.group != "" { + formatGroup += goa.group + "." + } else { + for _, a := range goa.attrs { + parts = append(parts, formatGroup+a.String()) + } + } + } + r.Attrs(func(a slog.Attr) bool { - parts = append(parts, formatAttr(a, h.groups)) + parts = append(parts, formatGroup+a.String()) return true }) @@ -51,18 +76,25 @@ func (h slogHandler) Handle(ctx context.Context, r slog.Record) error { return nil } -func (h slogHandler) WithAttrs(attrs []slog.Attr) slog.Handler { - for _, a := range attrs { - h.attrs = append(h.attrs, formatAttr(a, h.groups)) - } +func (h slogHandler) withGroupOrAttrs(goa groupOrAttrs) slogHandler { + h.goas = append(h.goas, goa) return h } +// WithGroup returns a new Handler whose group consist of +// both the receiver's groups and the arguments. func (h slogHandler) WithGroup(name string) slog.Handler { - h.groups = append(h.groups, name) - return h + if name == "" { + return h + } + return h.withGroupOrAttrs(groupOrAttrs{group: name}) } -func formatAttr(a slog.Attr, groups []string) string { - return strings.Join(append(groups, a.String()), ".") +// WithAttrs returns a new Handler whose attributes consist of +// both the receiver's attributes and the arguments. +func (h slogHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + if len(attrs) == 0 { + return h + } + return h.withGroupOrAttrs(groupOrAttrs{attrs: attrs}) } diff --git a/ddtrace/tracer/slog_test.go b/ddtrace/tracer/slog_test.go index 352cac0183..047d350458 100644 --- a/ddtrace/tracer/slog_test.go +++ b/ddtrace/tracer/slog_test.go @@ -37,9 +37,17 @@ func Test_slogHandler(t *testing.T) { l.Error("error test", "n", 3) log.Flush() // needed to get the error log flushed + // Check that chaining works as expected. + l = l.With("baz", "qux") + l = l.WithGroup("c").WithGroup("d") + l.Info("info test", "n", 1) + + log.Flush() + // Check that the logs were written correctly. - require.Len(t, rl.Logs(), 3) + require.Len(t, rl.Logs(), 4) require.Contains(t, rl.Logs()[0], "info test foo=bar a.b.n=1") require.Contains(t, rl.Logs()[1], "warn test foo=bar a.b.n=2") require.Contains(t, rl.Logs()[2], "error test foo=bar a.b.n=3") + require.Contains(t, rl.Logs()[3], "info test foo=bar a.b.baz=qux a.b.c.d.n=1") } From 59968219d7342adf9b0bef106af22e1a3f5d07f9 Mon Sep 17 00:00:00 2001 From: Flavien Darche <11708575+e-n-0@users.noreply.github.com> Date: Thu, 6 Feb 2025 11:55:34 +0100 Subject: [PATCH 3/4] fix: manual publish of Service Extension image (#3151) --- .../workflows/service-extensions-publish.yml | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/.github/workflows/service-extensions-publish.yml b/.github/workflows/service-extensions-publish.yml index 8813618ac9..3ee2d0572d 100644 --- a/.github/workflows/service-extensions-publish.yml +++ b/.github/workflows/service-extensions-publish.yml @@ -7,12 +7,11 @@ on: workflow_dispatch: inputs: tag_name: - description: 'Docker image tag to use for the package' - required: true - default: 'dev' + description: 'Docker image tag to use for the package (default to selected branch name)' + required: false commit_sha: - description: 'Commit SHA to checkout' - required: true + description: 'Commit SHA to checkout (default to latest commit on selected branch)' + required: false set_as_latest: description: 'Set the tag as latest' required: false @@ -23,9 +22,8 @@ permissions: packages: write env: - TAG_NAME: ${{ github.ref_name || github.event.inputs.tag_name }} - REF_NAME: ${{ github.ref || github.event.inputs.commit_sha }} - COMMIT_SHA: ${{ github.sha || github.event.inputs.commit_sha }} + TAG_NAME: ${{ github.event.inputs.tag_name || github.ref_name }} + COMMIT_SHA: ${{ github.event.inputs.commit_sha || github.sha }} PUSH_LATEST: ${{ github.event.inputs.set_as_latest || 'true' }} REGISTRY_IMAGE: ghcr.io/datadog/dd-trace-go/service-extensions-callout @@ -45,7 +43,7 @@ jobs: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ env.REF_NAME }} + ref: ${{ env.COMMIT_SHA }} - name: Install Docker (only arm64) if: matrix.platform == 'linux/arm64' @@ -119,9 +117,13 @@ jobs: id: tags run: | tagname=${TAG_NAME//\//-} # remove slashes from tag name - echo "tags=-t ghcr.io/datadog/dd-trace-go/service-extensions-callout:${tagname} \ - -t ghcr.io/datadog/dd-trace-go/service-extensions-callout:${{ env.COMMIT_SHA }} \ - ${{ env.PUSH_LATEST == 'true' && '-t ghcr.io/datadog/dd-trace-go/service-extensions-callout:latest' }}" >> $GITHUB_OUTPUT + tags="tags=-t ghcr.io/datadog/dd-trace-go/service-extensions-callout:${tagname} \ + -t ghcr.io/datadog/dd-trace-go/service-extensions-callout:${{ env.COMMIT_SHA }}" + if [ "${PUSH_LATEST}" == "true" ]; then + tags="$tags -t ghcr.io/datadog/dd-trace-go/service-extensions-callout:latest" + fi + + echo $tags >> $GITHUB_OUTPUT - name: Create manifest list and push working-directory: /tmp/digests From d642159fa31e57f0c5b6993e6f5444d2f10d695c Mon Sep 17 00:00:00 2001 From: Flavien Darche <11708575+e-n-0@users.noreply.github.com> Date: Thu, 6 Feb 2025 16:29:44 +0100 Subject: [PATCH 4/4] contrib/envoyproxy: fix context propagation from envoy tracing (#3144) --- .../envoyproxy/go-control-plane/envoy_test.go | 40 +++++++++++++++++++ .../envoyproxy/go-control-plane/fakehttp.go | 35 +++++++++++++--- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/contrib/envoyproxy/go-control-plane/envoy_test.go b/contrib/envoyproxy/go-control-plane/envoy_test.go index 7a7ac6129f..36afe49c80 100644 --- a/contrib/envoyproxy/go-control-plane/envoy_test.go +++ b/contrib/envoyproxy/go-control-plane/envoy_test.go @@ -23,6 +23,7 @@ import ( v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "google.golang.org/grpc/metadata" ) func TestAppSec(t *testing.T) { @@ -273,6 +274,45 @@ func TestGeneratedSpan(t *testing.T) { require.Equal(t, "server", span.Tag("span.kind")) require.Equal(t, "Mistake Not...", span.Tag("http.useragent")) }) + + t.Run("span-with-injected-context", func(t *testing.T) { + client, mt, cleanup := setup() + defer cleanup() + + ctx := context.Background() + + // add metadata to the context + ctx = metadata.AppendToOutgoingContext(ctx, + "x-datadog-trace-id", "12345", + "x-datadog-parent-id", "67890", + ) + + stream, err := client.Process(ctx) + require.NoError(t, err) + + end2EndStreamRequest(t, stream, "/../../../resource-span/.?id=test", "GET", map[string]string{"user-agent": "Mistake Not...", "test-key": "test-value"}, map[string]string{"response-test-key": "response-test-value"}, false) + + err = stream.CloseSend() + require.NoError(t, err) + stream.Recv() // to flush the spans + + finished := mt.FinishedSpans() + require.Len(t, finished, 1) + + // Check for tags + span := finished[0] + require.Equal(t, "http.request", span.OperationName()) + require.Equal(t, "https://datadoghq.com/../../../resource-span/.?id=test", span.Tag("http.url")) + require.Equal(t, "GET", span.Tag("http.method")) + require.Equal(t, "datadoghq.com", span.Tag("http.host")) + require.Equal(t, "GET /resource-span", span.Tag("resource.name")) + require.Equal(t, "server", span.Tag("span.kind")) + require.Equal(t, "Mistake Not...", span.Tag("http.useragent")) + + // Check for trace context + require.Equal(t, uint64(12345), span.Context().TraceID()) + require.Equal(t, uint64(67890), span.ParentID()) + }) } func TestXForwardedForHeaderClientIp(t *testing.T) { diff --git a/contrib/envoyproxy/go-control-plane/fakehttp.go b/contrib/envoyproxy/go-control-plane/fakehttp.go index 3f20725e1b..62f2243be9 100644 --- a/contrib/envoyproxy/go-control-plane/fakehttp.go +++ b/contrib/envoyproxy/go-control-plane/fakehttp.go @@ -76,6 +76,28 @@ func splitPseudoHeaders(receivedHeaders []*corev3.HeaderValue) (headers map[stri return headers, pseudoHeaders } +// mergeMetadataHeaders merges the metadata headers of the grpc connection into the http headers of the request +// - Skip pseudo headers and headers that are already set +// - Set headers keys to be canonical +func mergeMetadataHeaders(md metadata.MD, headers http.Header) { + for k, v := range md { + if strings.HasPrefix(k, ":") { + continue + } + + // Skip the content-type header of the grpc request + // Note: all envoy set headers are lower-case + if k == "content-type" { + continue + } + + k = http.CanonicalHeaderKey(k) + if _, ok := headers[k]; !ok { + headers[k] = v + } + } +} + func createFakeResponseWriter(w http.ResponseWriter, res *extproc.ProcessingRequest_ResponseHeaders) error { headers, pseudoHeaders := splitPseudoHeaders(res.ResponseHeaders.GetHeaders().GetHeaders()) @@ -103,6 +125,13 @@ func newRequest(ctx context.Context, req *extproc.ProcessingRequest_RequestHeade return nil, err } + var remoteAddr string + md, ok := metadata.FromIncomingContext(ctx) + if ok { + mergeMetadataHeaders(md, headers) + remoteAddr = getRemoteAddr(md) + } + parsedURL, err := url.Parse(fmt.Sprintf("%s://%s%s", pseudoHeaders[":scheme"], pseudoHeaders[":authority"], pseudoHeaders[":path"])) if err != nil { return nil, fmt.Errorf( @@ -113,12 +142,6 @@ func newRequest(ctx context.Context, req *extproc.ProcessingRequest_RequestHeade err) } - var remoteAddr string - md, ok := metadata.FromIncomingContext(ctx) - if ok { - remoteAddr = getRemoteAddr(md) - } - var tlsState *tls.ConnectionState if pseudoHeaders[":scheme"] == "https" { tlsState = &tls.ConnectionState{}