diff --git a/cmd/sourcegraph-oss/osscmd/osscmd.go b/cmd/sourcegraph-oss/osscmd/osscmd.go index 50a76ab35d7..8b45309a59d 100644 --- a/cmd/sourcegraph-oss/osscmd/osscmd.go +++ b/cmd/sourcegraph-oss/osscmd/osscmd.go @@ -24,5 +24,5 @@ func MainOSS(services []service.Service, args []string) { // SingleServiceMainOSS is called from the `main` function of a command in the OSS build // to start a single service (such as frontend or gitserver). func SingleServiceMainOSS(service service.Service) { - svcmain.SingleServiceMain(service, config, true, true) + svcmain.SingleServiceMain(service, config) } diff --git a/enterprise/cmd/llm-proxy/BUILD.bazel b/enterprise/cmd/llm-proxy/BUILD.bazel index 78b94b20396..4d8eaf5b6b1 100644 --- a/enterprise/cmd/llm-proxy/BUILD.bazel +++ b/enterprise/cmd/llm-proxy/BUILD.bazel @@ -7,7 +7,11 @@ go_library( visibility = ["//visibility:private"], deps = [ "//enterprise/cmd/llm-proxy/shared", + "//internal/conf", + "//internal/env", "//internal/service/svcmain", + "@com_github_getsentry_sentry_go//:sentry-go", + "@com_github_sourcegraph_log//:log", ], ) diff --git a/enterprise/cmd/llm-proxy/main.go b/enterprise/cmd/llm-proxy/main.go index ab36af7f8ee..a1793e08266 100644 --- a/enterprise/cmd/llm-proxy/main.go +++ b/enterprise/cmd/llm-proxy/main.go @@ -1,10 +1,32 @@ package main import ( + "github.com/getsentry/sentry-go" + "github.com/sourcegraph/log" + "github.com/sourcegraph/sourcegraph/enterprise/cmd/llm-proxy/shared" + "github.com/sourcegraph/sourcegraph/internal/conf" + "github.com/sourcegraph/sourcegraph/internal/env" "github.com/sourcegraph/sourcegraph/internal/service/svcmain" ) +var sentryDSN = env.Get("LLM_PROXY_SENTRY_DSN", "", "Sentry DSN") + func main() { - svcmain.SingleServiceMain(shared.Service, svcmain.Config{}, true, false) + svcmain.SingleServiceMainWithoutConf(shared.Service, svcmain.Config{}, svcmain.OutOfBandConfiguration{ + Logging: func() conf.LogSinksSource { + if sentryDSN == "" { + return nil + } + + return conf.NewStaticLogsSinksSource(log.SinksConfig{ + Sentry: &log.SentrySink{ + ClientOptions: sentry.ClientOptions{ + Dsn: sentryDSN, + }, + }, + }) + }(), + Tracing: nil, + }) } diff --git a/enterprise/cmd/sourcegraph/enterprisecmd/enterprisecmd.go b/enterprise/cmd/sourcegraph/enterprisecmd/enterprisecmd.go index e2b3bf3292a..3cbe82769fa 100644 --- a/enterprise/cmd/sourcegraph/enterprisecmd/enterprisecmd.go +++ b/enterprise/cmd/sourcegraph/enterprisecmd/enterprisecmd.go @@ -19,7 +19,7 @@ func MainEnterprise(services []service.Service, args []string) { // SingleServiceMainEnterprise is called from the `main` function of a command in the // enterprise (non-OSS) build to start a single service (such as frontend or gitserver). func SingleServiceMainEnterprise(service service.Service) { - svcmain.SingleServiceMain(service, config, true, true) + svcmain.SingleServiceMain(service, config) } func init() { diff --git a/enterprise/cmd/sourcegraph/enterprisecmd/executorcmd/executorcmd.go b/enterprise/cmd/sourcegraph/enterprisecmd/executorcmd/executorcmd.go index 47a94614875..52d6e9535c5 100644 --- a/enterprise/cmd/sourcegraph/enterprisecmd/executorcmd/executorcmd.go +++ b/enterprise/cmd/sourcegraph/enterprisecmd/executorcmd/executorcmd.go @@ -10,10 +10,12 @@ import ( "github.com/sourcegraph/sourcegraph/internal/service/svcmain" ) -var config = svcmain.Config{} +var config = svcmain.Config{ + SkipValidate: true, +} // SingleServiceMainEnterprise is called from the `main` function of a command in the // enterprise (non-OSS) build to start a single service (such as frontend or gitserver). func SingleServiceMainEnterprise(service service.Service) { - svcmain.SingleServiceMain(service, config, false, false) + svcmain.SingleServiceMainWithoutConf(service, config, svcmain.OutOfBandConfiguration{}) } diff --git a/internal/conf/log_sinks.go b/internal/conf/log_sinks.go index faac97416c0..3018b0377d6 100644 --- a/internal/conf/log_sinks.go +++ b/internal/conf/log_sinks.go @@ -3,10 +3,27 @@ package conf import ( "github.com/getsentry/sentry-go" "github.com/sourcegraph/log" + + "github.com/sourcegraph/sourcegraph/internal/conf/conftypes" ) -func GetLogSinks() log.SinksConfig { - cfg := Get() +type LogSinksSource interface { + SinksConfig() log.SinksConfig + + // Watchable allows the caller to be notified when the configuration changes. + conftypes.Watchable +} + +// NewLogsSinksSource wraps WatchableSiteConfig with a method that generates a valid +// sinks configuration for sourcegraph/log. +func NewLogsSinksSource(c conftypes.WatchableSiteConfig) LogSinksSource { + return logSinksSource{c} +} + +type logSinksSource struct{ conftypes.WatchableSiteConfig } + +func (s logSinksSource) SinksConfig() log.SinksConfig { + cfg := s.SiteConfig() var sentrySink *log.SentrySink if cfg.Log != nil { @@ -23,3 +40,15 @@ func GetLogSinks() log.SinksConfig { Sentry: sentrySink, } } + +// NewStaticLogsSinksSource procs immediately and only once when Watch is called, +// and returns a static config. +func NewStaticLogsSinksSource(cfg log.SinksConfig) LogSinksSource { + return staticLogSinksSource{cfg: cfg} +} + +type staticLogSinksSource struct{ cfg log.SinksConfig } + +func (s staticLogSinksSource) SinksConfig() log.SinksConfig { return s.cfg } + +func (s staticLogSinksSource) Watch(fn func()) { fn() } // proc immediately diff --git a/internal/service/svcmain/svcmain.go b/internal/service/svcmain/svcmain.go index 1d9ab592542..5d62e04a2ec 100644 --- a/internal/service/svcmain/svcmain.go +++ b/internal/service/svcmain/svcmain.go @@ -29,7 +29,10 @@ import ( ) type Config struct { - AfterConfigure func() // run after all services' Configure hooks are called + // SkipValidate, if true, will skip validation of service configuration. + SkipValidate bool + // AfterConfigure, if provided, is run after all services' Configure hooks are called + AfterConfigure func() } // Main is called from the `main` function of the `sourcegraph-oss` and @@ -86,7 +89,7 @@ func Main(services []sgservice.Service, config Config, args []string) { app.Action = func(_ *cli.Context) error { logger := log.Scoped("sourcegraph", "Sourcegraph") singleprogram.Init(logger) - run(liblog, logger, services, config, true, true) + run(liblog, logger, services, config, nil) return nil } @@ -97,8 +100,13 @@ func Main(services []sgservice.Service, config Config, args []string) { } // SingleServiceMain is called from the `main` function of a command to start a single -// service (such as frontend or gitserver). -func SingleServiceMain(svc sgservice.Service, config Config, validateConfig, useConfPackage bool) { +// service (such as frontend or gitserver). It assumes the service can access site +// configuration and initializes the conf package, and sets up some default hooks for +// watching site configuration for instrumentation services like tracing and logging. +// +// If your service cannot access site configuration, use SingleServiceMainWithoutConf +// instead. +func SingleServiceMain(svc sgservice.Service, config Config) { liblog := log.Init(log.Resource{ Name: env.MyName, Version: version.Version(), @@ -112,7 +120,39 @@ func SingleServiceMain(svc sgservice.Service, config Config, validateConfig, use ), ) logger := log.Scoped("sourcegraph", "Sourcegraph") - run(liblog, logger, []sgservice.Service{svc}, config, validateConfig, useConfPackage) + run(liblog, logger, []sgservice.Service{svc}, config, nil) +} + +// OutOfBandConfiguration declares additional configuration that happens continuously, +// separate from service startup. In most cases this is configuration based on site config +// (the conf package). +type OutOfBandConfiguration struct { + // Logging is used to configure logging. + Logging conf.LogSinksSource + + // Tracing is used to configure tracing. + Tracing tracer.WatchableConfigurationSource +} + +// SingleServiceMainWithConf is called from the `main` function of a command to start a single +// service WITHOUT site configuration enabled by default. This is only useful for services +// that are not part of the core Sourcegraph deployment, such as executors and managed +// services. Use with care! +func SingleServiceMainWithoutConf(svc sgservice.Service, config Config, oobConfig OutOfBandConfiguration) { + liblog := log.Init(log.Resource{ + Name: env.MyName, + Version: version.Version(), + InstanceID: hostname.Get(), + }, + // Experimental: DevX is observing how sampling affects the errors signal. + log.NewSentrySinkWith( + log.SentrySink{ + ClientOptions: sentry.ClientOptions{SampleRate: 0.2}, + }, + ), + ) + logger := log.Scoped("sourcegraph", "Sourcegraph") + run(liblog, logger, []sgservice.Service{svc}, config, &oobConfig) } func run( @@ -120,19 +160,32 @@ func run( logger log.Logger, services []sgservice.Service, config Config, - validateConfig bool, - useConfPackage bool, + // If nil, will use site config + oobConfig *OutOfBandConfiguration, ) { defer liblog.Sync() // Initialize log15. Even though it's deprecated, it's still fairly widely used. logging.Init() //nolint:staticcheck // Deprecated, but logs unmigrated to sourcegraph/log look really bad without this. - if useConfPackage { + // If no oobConfig is provided, we're in conf mode + if oobConfig == nil { conf.Init() - go conf.Watch(liblog.Update(conf.GetLogSinks)) - tracer.Init(log.Scoped("tracer", "internal tracer package"), conf.DefaultClient()) + oobConfig = &OutOfBandConfiguration{ + Logging: conf.NewLogsSinksSource(conf.DefaultClient()), + Tracing: tracer.ConfConfigurationSource{WatchableSiteConfig: conf.DefaultClient()}, + } } + + if oobConfig.Logging != nil { + go oobConfig.Logging.Watch(func() { + liblog.Update(oobConfig.Logging.SinksConfig) + }) + } + if oobConfig.Tracing != nil { + tracer.Init(log.Scoped("tracer", "internal tracer package"), oobConfig.Tracing) + } + profiler.Init() obctx := observation.NewContext(logger) @@ -154,7 +207,7 @@ func run( // Validate each service's configuration. // // This cannot be done for executor, see the executorcmd package for details. - if validateConfig { + if !config.SkipValidate { for i, c := range serviceConfigs { if c == nil { continue diff --git a/internal/tracer/BUILD.bazel b/internal/tracer/BUILD.bazel index 27b8b9b3028..0b05d58809c 100644 --- a/internal/tracer/BUILD.bazel +++ b/internal/tracer/BUILD.bazel @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "tracer", srcs = [ + "conf.go", "doc.go", "logged_ot.go", "logged_otel.go", @@ -23,6 +24,7 @@ go_library( "//internal/tracer/oteldefaults/exporters", "//internal/version", "//lib/errors", + "//schema", "@com_github_opentracing_opentracing_go//:opentracing-go", "@com_github_sourcegraph_log//:log", "@io_opentelemetry_go_otel//:otel", @@ -42,7 +44,6 @@ go_test( srcs = ["watch_test.go"], embed = [":tracer"], deps = [ - "//internal/conf/conftypes", "//internal/trace/policy", "//schema", "@com_github_sourcegraph_log//:log", diff --git a/internal/tracer/conf.go b/internal/tracer/conf.go new file mode 100644 index 00000000000..f3cf3899479 --- /dev/null +++ b/internal/tracer/conf.go @@ -0,0 +1,17 @@ +package tracer + +import "github.com/sourcegraph/sourcegraph/internal/conf/conftypes" + +// ConfConfigurationSource wraps a basic conf-based configuration source in +// a tracing-specific configuration source. +type ConfConfigurationSource struct{ conftypes.WatchableSiteConfig } + +var _ WatchableConfigurationSource = ConfConfigurationSource{} + +func (c ConfConfigurationSource) Config() Configuration { + s := c.SiteConfig() + return Configuration{ + ExternalURL: s.ExternalURL, + ObservabilityTracing: s.ObservabilityTracing, + } +} diff --git a/internal/tracer/tracer.go b/internal/tracer/tracer.go index c9f38c0345f..5e7f87d10d4 100644 --- a/internal/tracer/tracer.go +++ b/internal/tracer/tracer.go @@ -19,6 +19,7 @@ import ( "github.com/sourcegraph/sourcegraph/internal/hostname" "github.com/sourcegraph/sourcegraph/internal/tracer/oteldefaults" "github.com/sourcegraph/sourcegraph/internal/version" + "github.com/sourcegraph/sourcegraph/schema" ) // options control the behavior of a TracerType @@ -55,8 +56,24 @@ func (t TracerType) isSetByUser() bool { return false } +type Configuration struct { + ExternalURL string + *schema.ObservabilityTracing +} + +type ConfigurationSource interface { + Config() Configuration +} + +type WatchableConfigurationSource interface { + ConfigurationSource + + // Watchable allows the caller to be notified when the configuration changes. + conftypes.Watchable +} + // Init should be called from the main function of service -func Init(logger log.Logger, c conftypes.WatchableSiteConfig) { +func Init(logger log.Logger, c WatchableConfigurationSource) { // Tune GOMAXPROCS for kubernetes. All our binaries import this package, // so we tune for all of them. // diff --git a/internal/tracer/watch.go b/internal/tracer/watch.go index 35bf5372296..bf917f5dd58 100644 --- a/internal/tracer/watch.go +++ b/internal/tracer/watch.go @@ -6,7 +6,6 @@ import ( "github.com/sourcegraph/log" oteltracesdk "go.opentelemetry.io/otel/sdk/trace" - "github.com/sourcegraph/sourcegraph/internal/conf/conftypes" "github.com/sourcegraph/sourcegraph/internal/trace/policy" ) @@ -16,7 +15,7 @@ import ( // provider and debugMode references. func newConfWatcher( logger log.Logger, - c conftypes.SiteConfigQuerier, + c ConfigurationSource, // provider will be updated with the appropriate span processors. provider *oteltracesdk.TracerProvider, // spanProcessorBuilder is used to create span processors to configure on the provider @@ -36,7 +35,7 @@ func newConfWatcher( // return function to be called on every conf update return func() { var ( - siteConfig = c.SiteConfig() + siteConfig = c.Config() tracingConfig = siteConfig.ObservabilityTracing previousPolicy = policy.GetTracePolicy() setTracerType = None diff --git a/internal/tracer/watch_test.go b/internal/tracer/watch_test.go index 5af64253be2..b18343b42e7 100644 --- a/internal/tracer/watch_test.go +++ b/internal/tracer/watch_test.go @@ -11,18 +11,17 @@ import ( oteltracesdk "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" - "github.com/sourcegraph/sourcegraph/internal/conf/conftypes" "github.com/sourcegraph/sourcegraph/internal/trace/policy" "github.com/sourcegraph/sourcegraph/schema" ) type mockConfig struct { - get func() schema.SiteConfiguration + get func() Configuration } -var _ conftypes.SiteConfigQuerier = &mockConfig{} +var _ ConfigurationSource = &mockConfig{} -func (m *mockConfig) SiteConfig() schema.SiteConfiguration { return m.get() } +func (m *mockConfig) Config() Configuration { return m.get() } func TestConfigWatcher(t *testing.T) { var ( @@ -43,8 +42,8 @@ func TestConfigWatcher(t *testing.T) { doUpdate := newConfWatcher( logger, &mockConfig{ - get: func() schema.SiteConfiguration { - return schema.SiteConfiguration{ + get: func() Configuration { + return Configuration{ ObservabilityTracing: nil, } }, @@ -67,8 +66,8 @@ func TestConfigWatcher(t *testing.T) { t.Run("enable tracing with 'observability.tracing: {}'", func(t *testing.T) { mockConfig := &mockConfig{ - get: func() schema.SiteConfiguration { - return schema.SiteConfiguration{ + get: func() Configuration { + return Configuration{ ObservabilityTracing: &schema.ObservabilityTracing{}, } }, @@ -123,8 +122,8 @@ func TestConfigWatcher(t *testing.T) { }) t.Run("disable tracing after enabling it", func(t *testing.T) { - mockConfig.get = func() schema.SiteConfiguration { - return schema.SiteConfiguration{ + mockConfig.get = func() Configuration { + return Configuration{ ObservabilityTracing: &schema.ObservabilityTracing{Sampling: "none"}, } } @@ -155,8 +154,8 @@ func TestConfigWatcher(t *testing.T) { t.Run("update tracing with debug and sampling all", func(t *testing.T) { mockConf := &mockConfig{ - get: func() schema.SiteConfiguration { - return schema.SiteConfiguration{ + get: func() Configuration { + return Configuration{ ObservabilityTracing: &schema.ObservabilityTracing{ Debug: true, Sampling: "all", @@ -204,8 +203,8 @@ func TestConfigWatcher(t *testing.T) { t.Run("sanity check - swap existing processor with another", func(t *testing.T) { spansRecorder2 := tracetest.NewSpanRecorder() updatedSpanProcessor = spansRecorder2 - mockConf.get = func() schema.SiteConfiguration { - return schema.SiteConfiguration{ + mockConf.get = func() Configuration { + return Configuration{ ObservabilityTracing: &schema.ObservabilityTracing{ Debug: true, Sampling: "all",