Refactor code for readability

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
parca-dev · Mar 22, 2022 · dd829a6 · dd829a6
1 parent 4653f7a
commit dd829a6
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 41 deletions.
diff --git a/pkg/debuginfo/debuginfo.go b/pkg/debuginfo/debuginfo.go
@@ -80,6 +80,12 @@ func New(logger log.Logger, client Client, tmp string) *DebugInfo {
 
 // EnsureUploaded ensures that the extracted or the found debuginfo for the given buildID is uploaded.
 func (di *DebugInfo) EnsureUploaded(ctx context.Context, objFiles []*objectfile.MappedObjectFile) {
+	type cleanup struct {
+		buildID string
+		path    string
+	}
+
+	var filesToCleanup []cleanup
 	for _, objFile := range objFiles {
 		buildID := objFile.BuildID
 		logger := log.With(di.logger, "buildid", buildID, "path", objFile.Path)
@@ -90,9 +96,13 @@ func (di *DebugInfo) EnsureUploaded(ctx context.Context, objFiles []*objectfile.
 
 		// Finds the debuginfo file. Interim files can be clean up.
 		dbgInfoPath, shouldCleanup := di.debugInfoFilePath(ctx, buildID, objFile)
+		// Cleanup the extracted debug information file.
+		if shouldCleanup {
+			filesToCleanup = append(filesToCleanup, cleanup{buildID: buildID, path: dbgInfoPath})
+		}
 		// If debuginfo file is still not found, we don't need to upload anything.
 		if dbgInfoPath == "" {
-			level.Warn(logger).Log("msg", "failed to find debug info")
+			level.Warn(logger).Log("msg", "failed to find debug information")
 			continue
 		}
 
@@ -101,47 +111,46 @@ func (di *DebugInfo) EnsureUploaded(ctx context.Context, objFiles []*objectfile.
 			if errors.Is(err, debuginfo.ErrDebugInfoAlreadyExists) {
 				// If already exists, we should mark as exists in cache!
 				di.existsCache.Put(buildID, struct{}{})
-				level.Debug(logger).Log("msg", "debug info already exists")
+				level.Debug(logger).Log("msg", "debug information has already been uploaded or exists in server")
 				continue
 			}
-			level.Error(logger).Log("msg", "failed to upload debug info", "err", err)
+			level.Error(logger).Log("msg", "failed to upload debug information", "err", err)
 			continue
 		}
-		level.Debug(logger).Log("msg", "debug info uploaded successfully")
+		level.Debug(logger).Log("msg", "debug information successfully uploaded successfully")
+	}
 
-		// Successfully uploaded, we can clean up.
-		// Cleanup the extracted debug info file.
-		if shouldCleanup {
-			if err := os.Remove(dbgInfoPath); err != nil {
-				if os.IsNotExist(err) {
-					di.debugInfoFileCache.Invalidate(buildID)
-					continue
-				}
-				level.Debug(logger).Log("msg", "failed to cleanup debug info", "err", err)
+	for _, c := range filesToCleanup {
+		if err := os.Remove(c.path); err != nil {
+			if os.IsNotExist(err) {
+				di.debugInfoFileCache.Invalidate(c.buildID)
 				continue
 			}
-			di.debugInfoFileCache.Invalidate(buildID)
+
+			level.Debug(di.logger).Log(
+				"msg", "failed to cleanup debug information",
+				"buildid", c.buildID, "path", c.path, "err", err,
+			)
+			continue
 		}
+		di.debugInfoFileCache.Invalidate(c.buildID)
 	}
 }
 
 func (di *DebugInfo) exists(ctx context.Context, buildID, filePath string) bool {
 	logger := log.With(di.logger, "buildid", buildID, "path", filePath)
 	if _, ok := di.existsCache.GetIfPresent(buildID); ok {
-		level.Debug(logger).Log("msg", "debug info already uploaded to server")
+		level.Debug(logger).Log("msg", "debug information already exists in the server", "source", "cache")
 		return true
 	}
 
 	exists, err := di.client.Exists(ctx, buildID)
 	if err != nil {
-		level.Debug(logger).Log(
-			"msg", "failed to check whether build ID symbol exists",
-			"buildid", buildID, "err", err,
-		)
+		level.Debug(logger).Log("msg", "failed to check whether build ID symbol exists", "err", err)
 	}
 
 	if exists {
-		level.Debug(logger).Log("msg", "debug information already exist in server")
+		level.Debug(logger).Log("msg", "debug information already exists in the server", "source", "server")
 		di.existsCache.Put(buildID, struct{}{})
 		return true
 	}

diff --git a/pkg/maps/maps.go b/pkg/maps/maps.go
@@ -92,7 +92,8 @@ func (c *PIDMappingFileCache) mappingForPID(pid uint32) ([]*profile.Mapping, err
 		// Try our best to have the BuildID.
 		if m.BuildID == "" {
 			// TODO(brancz): These need special cases. See pseudo-paths in proc's man page
-			if m.File == "[vdso]" || m.File == "[vsyscall]" || m.File == "[stack]" || m.File == "[heap]" || m.File == "" {
+			//  m.File == "[vdso]" || m.File == "[vsyscall]" || m.File == "[stack]" || m.File == "[heap]"
+			if m.Unsymbolizable() || m.File == "" {
 				continue
 			}
 

diff --git a/pkg/profiler/profiler.go b/pkg/profiler/profiler.go
@@ -446,30 +446,15 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, captureTime time.Time)
 
 					m, err := mapping.PIDAddrMapping(pid, addr)
 					if err != nil {
-						level.Debug(p.logger).Log("msg", "failed to get process mapping", "err", err)
-					}
-
-					var objFile *objectfile.MappedObjectFile
-					if m != nil {
-						objFile, err = p.objCache.ObjectFileForProcess(pid, m)
-						if err != nil {
-							level.Debug(p.logger).Log("msg", "failed to open object file", "err", err)
+						if !errors.Is(err, maps.ErrNotFound) {
+							level.Warn(p.logger).Log("msg", "failed to get process mapping", "err", err)
 						}
 					}
 
-					// Transform the address by normalizing OS offsets.
-					normalizedAddr := addr
-					if objFile != nil {
-						nAddr, err := objFile.ObjAddr(addr)
-						if err != nil {
-							level.Debug(p.logger).Log("msg", "failed to get normalized address from object file", "err", err)
-						} else {
-							normalizedAddr = nAddr
-						}
-					}
 					l := &profile.Location{
-						ID:      uint64(locationIndex + 1),
-						Address: normalizedAddr,
+						ID: uint64(locationIndex + 1),
+						// Try to normalize the address for a symbol for position independent code.
+						Address: p.normalizeAddress(m, pid, addr),
 						Mapping: m,
 					}
 
@@ -572,6 +557,33 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, captureTime time.Time)
 	return nil
 }
 
+func (p *CgroupProfiler) normalizeAddress(m *profile.Mapping, pid uint32, addr uint64) uint64 {
+	if m == nil {
+		return addr
+	}
+
+	logger := log.With(p.logger, "pid", pid, "buildID", m.BuildID)
+	if m.Unsymbolizable() {
+		level.Debug(logger).Log("msg", "mapping is unsymbolizable")
+		return addr
+	}
+
+	objFile, err := p.objCache.ObjectFileForProcess(pid, m)
+	if err != nil {
+		level.Debug(logger).Log("msg", "failed to open object file", "err", err)
+		return addr
+	}
+
+	// Transform the address by normalizing Kernel memory offsets.
+	normalizedAddr, err := objFile.ObjAddr(addr)
+	if err != nil {
+		level.Debug(logger).Log("msg", "failed to get normalized address from object file", "err", err)
+		return addr
+	}
+
+	return normalizedAddr
+}
+
 func (p *CgroupProfiler) sendProfile(ctx context.Context, prof *profile.Profile) error {
 	buf := bytes.NewBuffer(nil)
 	if err := prof.Write(buf); err != nil {