Skip to content

Commit

Permalink
uploader: modify interface to not return error
Browse files Browse the repository at this point in the history
logging an error when the uploader fails to handle an executable turned
out to be relatively noisy, so we modify the interface to no longer
return an error, and we log.Debugf errors we encounter in the uploader
  • Loading branch information
Gandem committed Jun 25, 2024
1 parent ba75918 commit b1f005f
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 15 deletions.
5 changes: 1 addition & 4 deletions processmanager/execinfomanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,7 @@ func (mgr *ExecutableInfoManager) AddOrIncRef(hostFileID host.FileID, fileID lib

// Processing symbols for upload can take a while, so we release the lock
// before doing this.
err = mgr.uploader.HandleExecutable(elfRef, fileID)
if err != nil {
log.Errorf("Failed to handle executable %v: %v", elfRef.FileName(), err)
}
mgr.uploader.HandleExecutable(elfRef, fileID)

return info.ExecutableInfo, nil
}
Expand Down
15 changes: 8 additions & 7 deletions symbolication/datadog_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ func NewDatadogUploader() (Uploader, error) {
}, nil
}

func (d *DatadogUploader) HandleExecutable(elfRef *pfelf.Reference, fileID libpf.FileID) error {
func (d *DatadogUploader) HandleExecutable(elfRef *pfelf.Reference, fileID libpf.FileID) {
_, ok := d.uploadCache.Peek(fileID)
if ok {
log.Debugf("Skipping symbol upload for executable %s: already uploaded",
elfRef.FileName())
return nil
return
}
fileName := elfRef.FileName()

Expand All @@ -93,19 +93,20 @@ func (d *DatadogUploader) HandleExecutable(elfRef *pfelf.Reference, fileID libpf
if err != nil {
log.Debugf("Skipping symbol upload for executable %s: %v",
fileName, err)
return nil
return
}

// This needs to be done synchronously before the process manager closes the elfRef
inputFilePath := localDebugSymbolsPath(ef, elfRef)
if inputFilePath == "" {
log.Debugf("Skipping symbol upload for executable %s: no debug symbols found", fileName)
return nil
return
}

e, err := newExecutableMetadata(fileName, ef, fileID)
if err != nil {
return err
log.Debugf("Skipping symbol upload for executable %s: %v", fileName, err)
return
}

d.uploadCache.Add(fileID, struct{}{})
Expand All @@ -132,7 +133,7 @@ func (d *DatadogUploader) HandleExecutable(elfRef *pfelf.Reference, fileID libpf
}
}()

return nil
return

Check failure on line 136 in symbolication/datadog_uploader.go

View workflow job for this annotation

GitHub Actions / Lint (stable)

S1023: redundant `return` statement (gosimple)
}

type executableMetadata struct {
Expand All @@ -151,7 +152,7 @@ func newExecutableMetadata(fileName string, elf *pfelf.File,
isGolang := elf.IsGolang()

buildID, err := elf.GetBuildID()
// Some Go executables don't have a GNU build ID, so we don't want to fail
// 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)
Expand Down
2 changes: 1 addition & 1 deletion symbolication/iface.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ import (
)

type Uploader interface {
HandleExecutable(elfRef *pfelf.Reference, fileID libpf.FileID) error
HandleExecutable(elfRef *pfelf.Reference, fileID libpf.FileID)
}
4 changes: 1 addition & 3 deletions symbolication/uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ var _ Uploader = (*NoopUploader)(nil)

type NoopUploader struct{}

func (n *NoopUploader) HandleExecutable(_ *pfelf.Reference, _ libpf.FileID) error {
return nil
}
func (n *NoopUploader) HandleExecutable(_ *pfelf.Reference, _ libpf.FileID) {}

func NewNoopUploader() Uploader {
return &NoopUploader{}
Expand Down

0 comments on commit b1f005f

Please sign in to comment.