Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

symbolication: reduce error logs for local symbol upload [PROF-10143] #39

Merged
merged 2 commits into from
Jul 10, 2024
Merged

symbolication: reduce error logs for local symbol upload [PROF-10143] #39

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86bd4fe
symbolication: separate extraction from upload timeouts
Gandem Jul 9, 2024
d0d80b6
symbolication: reduce error logs for short-lived executables
Gandem Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 41 additions & 13 deletions symbolication/datadog_uploader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"os"
"os/exec"
"runtime"
"strings"
"time"

lru "github.com/elastic/go-freelru"
Expand All @@ -29,14 +30,16 @@ const binaryCacheSize = 1000

const sourceMapEndpoint = "/api/v2/srcmap"

const uploadTimeout = 10 * time.Second
const symbolCopyTimeout = 10 * time.Second
const uploadTimeout = 15 * time.Second

type DatadogUploader struct {
ddAPIKey string
intakeURL string
dryRun bool

uploadCache *lru.SyncedLRU[libpf.FileID, struct{}]
client *http.Client
}

var _ Uploader = (*DatadogUploader)(nil)
Expand Down Expand Up @@ -75,6 +78,9 @@ func NewDatadogUploader() (Uploader, error) {
dryRun: dryRun,

uploadCache: uploadCache,
client: &http.Client{
Timeout: uploadTimeout,
},
}, nil
}

Expand Down Expand Up @@ -117,15 +123,20 @@ func (d *DatadogUploader) HandleExecutable(elfRef *pfelf.Reference, fileID libpf
// if there are many executables.
// Ideally, we should limit the number of concurrent uploads
go func() {
ctx, cancel := context.WithTimeout(context.Background(), uploadTimeout)
defer cancel()
_, err = os.Stat(inputFilePath)
if err != nil {
d.uploadCache.Remove(fileID)
log.Debugf("Skipping symbol extraction for short-lived executable %s: %v", fileName,
err)
return
}

if d.dryRun {
log.Infof("Dry run: would upload symbols %s for executable: %s", inputFilePath, e)
return
}

err = d.handleSymbols(ctx, inputFilePath, e)
err = d.handleSymbols(inputFilePath, e)
if err != nil {
d.uploadCache.Remove(fileID)
log.Errorf("Failed to handle symbols: %v for executable: %s", err, e)
Expand Down Expand Up @@ -184,7 +195,7 @@ func (e *executableMetadata) String() string {
)
}

func (d *DatadogUploader) handleSymbols(ctx context.Context, symbolPath string,
func (d *DatadogUploader) handleSymbols(symbolPath string,
e *executableMetadata) error {
symbolFile, err := os.CreateTemp("", "objcopy-debug")
if err != nil {
Expand All @@ -193,12 +204,14 @@ func (d *DatadogUploader) handleSymbols(ctx context.Context, symbolPath string,
defer os.Remove(symbolFile.Name())
defer symbolFile.Close()

ctx, cancel := context.WithTimeout(context.Background(), symbolCopyTimeout)
defer cancel()
err = d.copySymbols(ctx, symbolPath, symbolFile.Name())
if err != nil {
return fmt.Errorf("failed to copy symbols: %w", err)
}

err = d.uploadSymbols(ctx, symbolFile, e)
err = d.uploadSymbols(symbolFile, e)
if err != nil {
return fmt.Errorf("failed to upload symbols: %w", err)
}
Expand All @@ -213,21 +226,21 @@ func (d *DatadogUploader) copySymbols(ctx context.Context, inputPath, outputPath
inputPath,
outputPath,
}
err := exec.CommandContext(ctx, "objcopy", args...).Run()
_, err := exec.CommandContext(ctx, "objcopy", args...).Output()
if err != nil {
return fmt.Errorf("failed to extract debug symbols: %w", err)
return fmt.Errorf("failed to extract debug symbols: %w", cleanCmdError(err))
}
return nil
}

func (d *DatadogUploader) uploadSymbols(ctx context.Context, symbolFile *os.File,
func (d *DatadogUploader) uploadSymbols(symbolFile *os.File,
e *executableMetadata) error {
req, err := d.buildSymbolUploadRequest(ctx, symbolFile, e)
req, err := d.buildSymbolUploadRequest(symbolFile, e)
if err != nil {
return fmt.Errorf("failed to build symbol upload request: %w", err)
}

resp, err := http.DefaultClient.Do(req)
resp, err := d.client.Do(req)
if err != nil {
return err
}
Expand All @@ -243,7 +256,7 @@ func (d *DatadogUploader) uploadSymbols(ctx context.Context, symbolFile *os.File
return nil
}

func (d *DatadogUploader) buildSymbolUploadRequest(ctx context.Context, symbolFile *os.File,
func (d *DatadogUploader) buildSymbolUploadRequest(symbolFile *os.File,
e *executableMetadata) (*http.Request, error) {
b := new(bytes.Buffer)

Expand Down Expand Up @@ -287,7 +300,7 @@ func (d *DatadogUploader) buildSymbolUploadRequest(ctx context.Context, symbolFi
return nil, fmt.Errorf("failed to close gzip writer: %w", err)
}

r, err := http.NewRequestWithContext(ctx, http.MethodPost, d.intakeURL, b)
r, err := http.NewRequest(http.MethodPost, d.intakeURL, b)
if err != nil {
return nil, fmt.Errorf("failed to create request: %w", err)
}
Expand Down Expand Up @@ -351,3 +364,18 @@ func debugSymbolsPathForElf(ef *pfelf.File, fileName string) (string, error) {
}
return filePath, nil
}

// cleanCmdError simplifies error messages from os/exec.Cmd.Run.
// For ExitErrors, it trims and returns stderr. By default, ExitError prints the exit
// status but not stderr.
//
// cleanCmdError returns other errors unmodified.
func cleanCmdError(err error) error {
var xerr *exec.ExitError
if errors.As(err, &xerr) {
if stderr := strings.TrimSpace(string(xerr.Stderr)); stderr != "" {
return fmt.Errorf("%w: %s", err, stderr)
}
}
return err
}
Loading