From eeff2226839ec1f2de353b842146dc7c2de7e635 Mon Sep 17 00:00:00 2001
From: Nayef Ghattas <nayef.ghattas@datadoghq.com>
Date: Tue, 3 Sep 2024 17:43:20 +0000
Subject: [PATCH] symbolication: allow symbol upload for Bazel-built Go
 binairies

Bazel built Go binairies have their GNU build ID stripped, and
their Go build ID set to the string "redacted"
See https://github.com/bazelbuild/rules_go/blob/199d8e4827f87d382a85febd0148c1b42fa949cc/go/private/actions/link.bzl#L174

In that case, we want to able to rely on the file hash, for this
reason we:
- modify the function to get go build ID to consider that "redacted"
is equivalent to not having a build ID
- still upload debug symbols even if they don't contain a GNU build
id or a Go build ID (they'll always contain a file hash, that
we will be using for symbolication in that case).

The underlying goal is to allow remote symbolication with local
symbol upload to work seamless for Go binaries built with Bazel
---
 libpf/pfelf/pfelf.go              |  7 ++++++-
 symbolication/datadog_uploader.go | 20 ++++++++------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/libpf/pfelf/pfelf.go b/libpf/pfelf/pfelf.go
index 46b95e5..39f1498 100644
--- a/libpf/pfelf/pfelf.go
+++ b/libpf/pfelf/pfelf.go
@@ -237,13 +237,18 @@ func GetGoBuildID(elfFile *elf.File) (string, error) {
 }
 
 // getGoBuildIDFromNotes returns the Go build ID from an ELF notes section data.
+//
+//nolint:lll
 func getGoBuildIDFromNotes(notes []byte) (string, error) {
 	// 0x4 is the "Go Build ID" type. Not sure where this is standardized.
 	buildID, found, err := getNoteString(notes, "Go", 0x4)
 	if err != nil {
 		return "", fmt.Errorf("could not determine BuildID: %v", err)
 	}
-	if !found {
+	// When building Go binaries, Bazel explicitly sets their build ID to "redacted"
+	// see https://github.com/bazelbuild/rules_go/blob/199d8e4827f87d382a85febd0148c1b42fa949cc/go/private/actions/link.bzl#L174.
+	// In that case, we don't want to associate the build ID with the binary in the mapping.
+	if !found || buildID == "redacted" {
 		return "", ErrNoBuildID
 	}
 	return buildID, nil
diff --git a/symbolication/datadog_uploader.go b/symbolication/datadog_uploader.go
index cbcfc58..1dce8a1 100644
--- a/symbolication/datadog_uploader.go
+++ b/symbolication/datadog_uploader.go
@@ -111,11 +111,7 @@ func (d *DatadogUploader) HandleExecutable(elfRef *pfelf.Reference, fileID libpf
 		return
 	}
 
-	e, err := newExecutableMetadata(fileName, ef, fileID)
-	if err != nil {
-		log.Debugf("Skipping symbol upload for executable %s: %v", fileName, err)
-		return
-	}
+	e := newExecutableMetadata(fileName, ef, fileID)
 
 	d.uploadCache.Add(fileID, struct{}{})
 	// TODO:
@@ -162,21 +158,21 @@ type executableMetadata struct {
 }
 
 func newExecutableMetadata(fileName string, elf *pfelf.File,
-	fileID libpf.FileID) (*executableMetadata, error) {
+	fileID libpf.FileID) *executableMetadata {
 	isGolang := elf.IsGolang()
 
 	buildID, err := elf.GetBuildID()
-	// Some Go executables don't have a GNU build ID, so we don't want to fail
-	// if we can't get it
-	if err != nil && !isGolang {
-		return nil, fmt.Errorf("failed to get build id: %w", err)
+	if err != nil {
+		log.Debugf(
+			"Unable to get GNU build ID for executable %s: %s", fileName, err)
 	}
 
 	goBuildID := ""
 	if isGolang {
 		goBuildID, err = elf.GetGoBuildID()
 		if err != nil {
-			return nil, fmt.Errorf("failed to get go build id: %w", err)
+			log.Debugf(
+				"Unable to get Go build ID for executable %s: %s", fileName, err)
 		}
 	}
 
@@ -191,7 +187,7 @@ func newExecutableMetadata(fileName string, elf *pfelf.File,
 		SymbolSource:  "debug_info",
 		FileName:      filepath.Base(fileName),
 		filePath:      fileName,
-	}, nil
+	}
 }
 
 func (e *executableMetadata) String() string {