Enable Python and Ruby Unwinding by default (#2281)

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>

depends:  
- #2305 
- #2286
- #2290

README.md

@@ -163,9 +163,6 @@ Flags:
       --debuginfo-disable-caching
                                    Disable caching of debuginfo.
       --symbolizer-jit-disable     Disable JIT symbolization.
-      --dwarf-unwinding-disable    Do not unwind using .eh_frame information.
-      --dwarf-unwinding-mixed      Unwind using .eh_frame information and frame
-                                   pointers
       --otlp-address=STRING        The endpoint to send OTLP traces to.
       --otlp-exporter="grpc"       The OTLP exporter to use.
       --object-file-pool-eviction-policy="lru"
@@ -177,6 +174,12 @@ Flags:
                                    object files from disk. It keeps FDs open,
                                    so it should be kept in sync with ulimits.
                                    0 means no limit.
+      --dwarf-unwinding-disable    Do not unwind using .eh_frame information.
+      --dwarf-unwinding-mixed      Unwind using .eh_frame information and frame
+                                   pointers.
+      --python-unwinding-disable
+                                   Disable Python unwinder.
+      --ruby-unwinding-disable     Disable Ruby unwinder.
       --analytics-opt-out          Opt out of sending anonymous usage
                                    statistics.
       --telemetry-disable-panic-reporting

cmd/parca-agent/main.go

@@ -139,10 +139,13 @@ type flags struct {
 	RemoteStore    FlagsRemoteStore    `embed:"" prefix:"remote-store-"`
 	Debuginfo      FlagsDebuginfo      `embed:"" prefix:"debuginfo-"`
 	Symbolizer     FlagsSymbolizer     `embed:"" prefix:"symbolizer-"`
-	DWARFUnwinding FlagsDWARFUnwinding `embed:"" prefix:"dwarf-unwinding-"`
 	OTLP           FlagsOTLP           `embed:"" prefix:"otlp-"`
 	ObjectFilePool FlagsObjectFilePool `embed:"" prefix:"object-file-pool-"`
 
+	DWARFUnwinding         FlagsDWARFUnwinding `embed:""        prefix:"dwarf-unwinding-"`
+	PythonUnwindingDisable bool                `default:"false" help:"Disable Python unwinder."`
+	RubyUnwindingDisable   bool                `default:"false" help:"Disable Ruby unwinder."`
+
 	AnalyticsOptOut bool `default:"false" help:"Opt out of sending anonymous usage statistics."`
 
 	Telemetry FlagsTelemetry `embed:"" prefix:"telemetry-"`
@@ -222,7 +225,7 @@ type FlagsSymbolizer struct {
 // FlagsDWARFUnwinding contains flags to configure DWARF unwinding.
 type FlagsDWARFUnwinding struct {
 	Disable bool `help:"Do not unwind using .eh_frame information."`
-	Mixed   bool `default:"true"                                    help:"Unwind using .eh_frame information and frame pointers"`
+	Mixed   bool `default:"true"                                    help:"Unwind using .eh_frame information and frame pointers."`
 }
 
 type FlagsTelemetry struct {
@@ -242,8 +245,8 @@ type FlagsHidden struct {
 	AllowRunningAsNonRoot             bool `help:"Force running the Agent even if the user is not root. This will break a lot of the assumptions and result in the Agent malfunctioning."  hidden:""`
 	AllowRunningInNonRootPIDNamespace bool `help:"Force running the Agent in a non 'root' PID namespace. This will break a lot of the assumptions and result in the Agent malfunctioning." hidden:""`
 
-	EnablePythonUnwinding bool `default:"false" help:"Enable Python unwinding." hidden:""`
-	EnableRubyUnwinding   bool `default:"false" help:"Enable Ruby unwinding."   hidden:""`
+	EnablePythonUnwinding bool `default:"false" help:"[deprecated] Python unwinder enabled by default. Use --python-unwinding-disable to disable it." hidden:""`
+	EnableRubyUnwinding   bool `default:"false" help:"[deprecated] Ruby unwinder enabled by default. Use --ruby-unwinding-disable to disable it."     hidden:""`
 
 	ForcePanic bool `default:"false" help:"Panics the agent in a goroutine to test that telemetry works." hidden:""`
 
@@ -471,6 +474,13 @@ func main() {
 	intro := figure.NewColorFigure("Parca Agent ", "roman", "yellow", true)
 	intro.Print()
 
+	if flags.Hidden.EnablePythonUnwinding {
+		level.Warn(logger).Log("msg", "python unwinder is enabled by default, use --python-unwinding-disable to disable it")
+	}
+	if flags.Hidden.EnableRubyUnwinding {
+		level.Warn(logger).Log("msg", "ruby unwinder is enabled by default, use --ruby-unwinding-disable to disable it")
+	}
+
 	// TODO(sylfrena): Entirely remove once full support for DWARF Unwinding Arm64 is added and production tested for a few days
 	if goruntime.GOARCH == "arm64" {
 		flags.DWARFUnwinding.Disable = false
@@ -912,7 +922,6 @@ func run(logger log.Logger, reg *prometheus.Registry, flags flags) error {
 		labelsManager,
 		flags.Profiling.Duration,
 		flags.Debuginfo.UploadCacheDuration,
-		flags.Hidden.EnablePythonUnwinding || flags.Hidden.EnableRubyUnwinding,
 	)
 	{
 		logger := log.With(logger, "group", "process_info_manager")
@@ -964,8 +973,8 @@ func run(logger log.Logger, reg *prometheus.Registry, flags flags) error {
 				DWARFUnwindingMixedModeEnabled:    flags.DWARFUnwinding.Mixed,
 				BPFVerboseLoggingEnabled:          flags.BPF.VerboseLogging,
 				BPFEventsBufferSize:               flags.BPF.EventsBufferSize,
-				PythonUnwindingEnabled:            flags.Hidden.EnablePythonUnwinding,
-				RubyUnwindingEnabled:              flags.Hidden.EnableRubyUnwinding,
+				PythonUnwindingEnabled:            !flags.PythonUnwindingDisable,
+				RubyUnwindingEnabled:              !flags.RubyUnwindingDisable,
 				EventRateLimitsEnabled:            flags.BPF.EventRateLimitsEnabled,
 			},
 			bpfProgramLoaded,

pkg/process/info.go

@@ -135,8 +135,6 @@ type InfoManager struct {
 
 	uploadJobQueue chan *uploadJob
 	uploadJobPool  *sync.Pool
-
-	shouldFetchInterpreterInfo bool
 }
 
 func NewInfoManager(
@@ -150,7 +148,6 @@ func NewInfoManager(
 	lm LabelManager,
 	profilingDuration time.Duration,
 	cacheTTL time.Duration,
-	fetchInterpreterInfo bool,
 ) *InfoManager {
 	im := &InfoManager{
 		logger:  logger,
@@ -187,7 +184,6 @@ func NewInfoManager(
 				return &uploadJob{}
 			},
 		},
-		shouldFetchInterpreterInfo: fetchInterpreterInfo,
 	}
 	return im
 }
@@ -290,19 +286,15 @@ func (im *InfoManager) fetch(ctx context.Context, pid int, checkMappings bool) (
 	// Upload debug information of the discovered object files.
 	im.ensureDebuginfoUploaded(ctx, mappings)
 
-	var interp *runtime.Interpreter
-	if im.shouldFetchInterpreterInfo {
-		// Fetch interpreter information.
-		// At this point we cannot tell if a process is a Python or Ruby interpreter so,
-		// we will pay the cost for the excluded one if only one of them enabled.
-		var err error
-		interp, err = interpreter.Fetch(proc)
-		if err != nil {
-			level.Debug(im.logger).Log("msg", "failed to fetch interpreter information", "err", err, "pid", pid)
-		}
-		if interp != nil {
-			level.Debug(im.logger).Log("msg", "interpreter information fetched", "interpreter", interp.Type, "version", interp.Version, "pid", pid)
-		}
+	// Fetch interpreter information.
+	// At this point we cannot tell if a process is a Python or Ruby interpreter so,
+	// we will pay the cost for the excluded one if only one of them enabled.
+	interp, err := interpreter.Fetch(proc)
+	if err != nil {
+		level.Debug(im.logger).Log("msg", "failed to fetch interpreter information", "err", err, "pid", pid)
+	}
+	if interp != nil {
+		level.Debug(im.logger).Log("msg", "interpreter information fetched", "interpreter", interp.Type, "version", interp.Version, "pid", pid)
 	}
 
 	// No matter what happens with the debug information, we should continue.

test/integration/profiler_test.go

@@ -311,7 +311,6 @@ func prepareProfiler(t *testing.T, profileStore profiler.ProfileStore, logger lo
 			labelsManager,
 			loopDuration,
 			loopDuration,
-			false, // interpreter unwinding enabled
 		),
 		cim,
 		parcapprof.NewManager(