refactor(pprof): remove redundant exports

conf/example.yaml

@@ -39,6 +39,13 @@ instrumentation:
             host: "0.0.0.0"
             port: 9090
 
+##################### PProf #####################
+pprof:
+  # Whether to enable the pprof server.
+  enabled: true
+  # The address on which the pprof server listens.
+  address: 0.0.0.0:6060
+
 ##################### Web #####################
 web:
   # Whether to enable the web frontend

ee/query-service/app/server.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	_ "net/http/pprof" // http profiler
 	"slices"
 
 	"go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux"
@@ -313,15 +312,6 @@ func (s *Server) Start(ctx context.Context) error {
 		s.unavailableChannel <- healthcheck.Unavailable
 	}()
 
-	go func() {
-		slog.Info("Starting pprof server", "addr", baseconst.DebugHttpPort)
-
-		err = http.ListenAndServe(baseconst.DebugHttpPort, nil)
-		if err != nil {
-			slog.Error("Could not start pprof server", errors.Attr(err))
-		}
-	}()
-
 	go func() {
 		slog.Info("Starting OpAmp Websocket server", "addr", baseconst.OpAmpWsEndpoint)
 		err := s.opampServer.Start(baseconst.OpAmpWsEndpoint)

pkg/instrumentation/logger.go

@@ -13,7 +13,13 @@ type zapToSlogConverter struct{}
 func NewLogger(config Config) *slog.Logger {
 	logger := slog.New(
 		loghandler.New(
-			slog.NewJSONHandler(os.Stderr, &slog.HandlerOptions{Level: config.Logs.Level, ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr {
+			slog.NewJSONHandler(os.Stderr, &slog.HandlerOptions{Level: config.Logs.Level, AddSource: true, ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr {
+				// This is more in line with OpenTelemetry semantic conventions
+				if a.Key == slog.SourceKey {
+					a.Key = "code"
+					return a
+				}
+
 				if a.Key == slog.TimeKey {
 					a.Key = "timestamp"
 					return a
@@ -21,7 +27,6 @@ func NewLogger(config Config) *slog.Logger {
 
 				return a
 			}}),
-			loghandler.NewSource(),
 			loghandler.NewCorrelation(),
 			loghandler.NewFiltering(),
 			loghandler.NewException(),

pkg/instrumentation/loghandler/source.go

@@ -1,28 +0,0 @@
-package loghandler
-
-import (
-	"context"
-	"log/slog"
-	"runtime"
-)
-
-type source struct{}
-
-func NewSource() *source {
-	return &source{}
-}
-
-func (h *source) Wrap(next LogHandler) LogHandler {
-	return LogHandlerFunc(func(ctx context.Context, record slog.Record) error {
-		if record.PC != 0 {
-			frame, _ := runtime.CallersFrames([]uintptr{record.PC}).Next()
-			record.AddAttrs(
-				slog.String("code.filepath", frame.File),
-				slog.String("code.function", frame.Function),
-				slog.Int("code.lineno", frame.Line),
-			)
-		}
-
-		return next.Handle(ctx, record)
-	})
-}

pkg/instrumentation/loghandler/source_test.go

@@ -1,37 +0,0 @@
-package loghandler
-
-import (
-	"bytes"
-	"context"
-	"encoding/json"
-	"log/slog"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func TestSource(t *testing.T) {
-	src := NewSource()
-
-	buf := bytes.NewBuffer(nil)
-	logger := slog.New(&handler{base: slog.NewJSONHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug}), wrappers: []Wrapper{src}})
-
-	logger.InfoContext(context.Background(), "test")
-
-	m := make(map[string]any)
-	err := json.Unmarshal(buf.Bytes(), &m)
-	require.NoError(t, err)
-
-	assert.Contains(t, m, "code.filepath")
-	assert.Contains(t, m, "code.function")
-	assert.Contains(t, m, "code.lineno")
-
-	assert.Contains(t, m["code.filepath"], "source_test.go")
-	assert.Contains(t, m["code.function"], "TestSource")
-	assert.NotZero(t, m["code.lineno"])
-
-	// Ensure the nested "source" key is not present.
-	assert.NotContains(t, m, "source")
-	assert.NotContains(t, m, "code")
-}

pkg/pprof/config.go

@@ -0,0 +1,32 @@
+package pprof
+
+import "github.com/SigNoz/signoz/pkg/factory"
+
+// Config holds the configuration for the pprof server.
+type Config struct {
+	Enabled bool   `mapstructure:"enabled"`
+	Address string `mapstructure:"address"`
+}
+
+func NewConfigFactory() factory.ConfigFactory {
+	return factory.NewConfigFactory(factory.MustNewName("pprof"), newConfig)
+}
+
+func newConfig() factory.Config {
+	return Config{
+		Enabled: true,
+		Address: "0.0.0.0:6060",
+	}
+}
+
+func (c Config) Validate() error {
+	return nil
+}
+
+func (c Config) Provider() string {
+	if c.Enabled {
+		return "http"
+	}
+
+	return "noop"
+}

pkg/pprof/config_test.go

@@ -0,0 +1,42 @@
+package pprof
+
+import (
+	"context"
+	"testing"
+
+	"github.com/SigNoz/signoz/pkg/config"
+	"github.com/SigNoz/signoz/pkg/config/envprovider"
+	"github.com/SigNoz/signoz/pkg/factory"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewWithEnvProvider(t *testing.T) {
+	t.Setenv("SIGNOZ_PPROF_ENABLED", "false")
+	t.Setenv("SIGNOZ_PPROF_ADDRESS", "127.0.0.1:6061")
+
+	conf, err := config.New(
+		context.Background(),
+		config.ResolverConfig{
+			Uris: []string{"env:"},
+			ProviderFactories: []config.ProviderFactory{
+				envprovider.NewFactory(),
+			},
+		},
+		[]factory.ConfigFactory{
+			NewConfigFactory(),
+		},
+	)
+	require.NoError(t, err)
+
+	actual := Config{}
+	err = conf.Unmarshal("pprof", &actual)
+	require.NoError(t, err)
+
+	expected := Config{
+		Enabled: false,
+		Address: "127.0.0.1:6061",
+	}
+
+	assert.Equal(t, expected, actual)
+}

pkg/pprof/httppprof/provider.go

@@ -0,0 +1,58 @@
+package httppprof
+
+import (
+	"context"
+	"log/slog"
+	"net/http"
+	httppprof "net/http/pprof"
+	runtimepprof "runtime/pprof"
+
+	"github.com/SigNoz/signoz/pkg/factory"
+	httpserver "github.com/SigNoz/signoz/pkg/http/server"
+	"github.com/SigNoz/signoz/pkg/pprof"
+)
+
+type provider struct {
+	server *httpserver.Server
+}
+
+func NewFactory() factory.ProviderFactory[pprof.PProf, pprof.Config] {
+	return factory.NewProviderFactory(factory.MustNewName("http"), New)
+}
+
+func New(_ context.Context, settings factory.ProviderSettings, config pprof.Config) (pprof.PProf, error) {
+	server, err := httpserver.New(
+		settings.Logger.With(slog.String("pkg", "github.com/SigNoz/signoz/pkg/pprof/httppprof")),
+		httpserver.Config{Address: config.Address},
+		newHandler(),
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return &provider{server: server}, nil
+}
+
+func (provider *provider) Start(ctx context.Context) error {
+	return provider.server.Start(ctx)
+}
+
+func (provider *provider) Stop(ctx context.Context) error {
+	return provider.server.Stop(ctx)
+}
+
+func newHandler() http.Handler {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/debug/pprof/", httppprof.Index)
+	mux.HandleFunc("/debug/pprof/cmdline", httppprof.Cmdline)
+	mux.HandleFunc("/debug/pprof/profile", httppprof.Profile)
+	mux.HandleFunc("/debug/pprof/symbol", httppprof.Symbol)
+	mux.HandleFunc("/debug/pprof/trace", httppprof.Trace)
+
+	// Register the runtime profiles in the same order returned by runtime/pprof.Profiles().
+	for _, profile := range runtimepprof.Profiles() {
+		mux.Handle("/debug/pprof/"+profile.Name(), httppprof.Handler(profile.Name()))
+	}
+
+	return mux
+}

pkg/pprof/nooppprof/provider.go

@@ -0,0 +1,35 @@
+package nooppprof
+
+import (
+	"context"
+	"sync"
+
+	"github.com/SigNoz/signoz/pkg/factory"
+	"github.com/SigNoz/signoz/pkg/pprof"
+)
+
+type provider struct {
+	stopC    chan struct{}
+	stopOnce sync.Once
+}
+
+func NewFactory() factory.ProviderFactory[pprof.PProf, pprof.Config] {
+	return factory.NewProviderFactory(factory.MustNewName("noop"), New)
+}
+
+func New(_ context.Context, _ factory.ProviderSettings, _ pprof.Config) (pprof.PProf, error) {
+	return &provider{stopC: make(chan struct{})}, nil
+}
+
+func (provider *provider) Start(context.Context) error {
+	<-provider.stopC
+	return nil
+}
+
+func (provider *provider) Stop(context.Context) error {
+	provider.stopOnce.Do(func() {
+		close(provider.stopC)
+	})
+
+	return nil
+}

pkg/pprof/pprof.go

@@ -0,0 +1,8 @@
+package pprof
+
+import "github.com/SigNoz/signoz/pkg/factory"
+
+// PProf is the interface that wraps the pprof service lifecycle.
+type PProf interface {
+	factory.Service
+}

pkg/query-service/app/server.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	_ "net/http/pprof" // http profiler
 	"slices"
 
 	"github.com/SigNoz/signoz/pkg/cache/memorycache"
@@ -294,15 +293,6 @@ func (s *Server) Start(ctx context.Context) error {
 		s.unavailableChannel <- healthcheck.Unavailable
 	}()
 
-	go func() {
-		slog.Info("Starting pprof server", "addr", constants.DebugHttpPort)
-
-		err = http.ListenAndServe(constants.DebugHttpPort, nil)
-		if err != nil {
-			slog.Error("Could not start pprof server", errors.Attr(err))
-		}
-	}()
-
 	go func() {
 		slog.Info("Starting OpAmp Websocket server", "addr", constants.OpAmpWsEndpoint)
 		err := s.opampServer.Start(constants.OpAmpWsEndpoint)

pkg/query-service/constants/constants.go

@@ -14,7 +14,6 @@ import (
 const (
 	HTTPHostPort    = "0.0.0.0:8080" // Address to serve http (query service)
 	PrivateHostPort = "0.0.0.0:8085" // Address to server internal services like alert manager
-	DebugHttpPort   = "0.0.0.0:6060" // Address to serve http (pprof)
 	OpAmpWsEndpoint = "0.0.0.0:4320" // address for opamp websocket
 )
 

pkg/signoz/config.go

@@ -23,6 +23,7 @@ import (
 	"github.com/SigNoz/signoz/pkg/instrumentation"
 	"github.com/SigNoz/signoz/pkg/modules/metricsexplorer"
 	"github.com/SigNoz/signoz/pkg/modules/user"
+	"github.com/SigNoz/signoz/pkg/pprof"
 	"github.com/SigNoz/signoz/pkg/prometheus"
 	"github.com/SigNoz/signoz/pkg/querier"
 	"github.com/SigNoz/signoz/pkg/ruler"
@@ -50,6 +51,9 @@ type Config struct {
 	// Instrumentation config
 	Instrumentation instrumentation.Config `mapstructure:"instrumentation"`
 
+	// PProf config
+	PProf pprof.Config `mapstructure:"pprof"`
+
 	// Analytics config
 	Analytics analytics.Config `mapstructure:"analytics"`
 
@@ -122,6 +126,7 @@ func NewConfig(ctx context.Context, logger *slog.Logger, resolverConfig config.R
 		global.NewConfigFactory(),
 		version.NewConfigFactory(),
 		instrumentation.NewConfigFactory(),
+		pprof.NewConfigFactory(),
 		analytics.NewConfigFactory(),
 		web.NewConfigFactory(),
 		cache.NewConfigFactory(),

pkg/signoz/provider.go

@@ -34,6 +34,9 @@ import (
 	"github.com/SigNoz/signoz/pkg/modules/session/implsession"
 	"github.com/SigNoz/signoz/pkg/modules/user"
 	"github.com/SigNoz/signoz/pkg/modules/user/impluser"
+	"github.com/SigNoz/signoz/pkg/pprof"
+	"github.com/SigNoz/signoz/pkg/pprof/httppprof"
+	"github.com/SigNoz/signoz/pkg/pprof/nooppprof"
 	"github.com/SigNoz/signoz/pkg/prometheus"
 	"github.com/SigNoz/signoz/pkg/prometheus/clickhouseprometheus"
 	"github.com/SigNoz/signoz/pkg/querier"
@@ -89,6 +92,13 @@ func NewWebProviderFactories() factory.NamedMap[factory.ProviderFactory[web.Web,
 	)
 }
 
+func NewPProfProviderFactories() factory.NamedMap[factory.ProviderFactory[pprof.PProf, pprof.Config]] {
+	return factory.MustNewNamedMap(
+		httppprof.NewFactory(),
+		nooppprof.NewFactory(),
+	)
+}
+
 func NewSQLStoreProviderFactories() factory.NamedMap[factory.ProviderFactory[sqlstore.SQLStore, sqlstore.Config]] {
 	return factory.MustNewNamedMap(
 		sqlitesqlstore.NewFactory(sqlstorehook.NewLoggingFactory(), sqlstorehook.NewInstrumentationFactory()),

pkg/signoz/signoz.go

@@ -107,6 +107,17 @@ func New(
 	// Get the provider settings from instrumentation
 	providerSettings := instrumentation.ToProviderSettings()
 
+	pprofService, err := factory.NewProviderFromNamedMap(
+		ctx,
+		providerSettings,
+		config.PProf,
+		NewPProfProviderFactories(),
+		config.PProf.Provider(),
+	)
+	if err != nil {
+		return nil, err
+	}
+
 	// Initialize analytics just after instrumentation, as providers might require it
 	analytics, err := factory.NewProviderFromNamedMap(
 		ctx,
@@ -448,6 +459,7 @@ func New(
 	registry, err := factory.NewRegistry(
 		instrumentation.Logger(),
 		factory.NewNamedService(factory.MustNewName("instrumentation"), instrumentation),
+		factory.NewNamedService(factory.MustNewName("pprof"), pprofService),
 		factory.NewNamedService(factory.MustNewName("analytics"), analytics),
 		factory.NewNamedService(factory.MustNewName("alertmanager"), alertmanager),
 		factory.NewNamedService(factory.MustNewName("licensing"), licensing),