From 363aa356df5fc99c8499b78fdcbb5a96c16ca881 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Wed, 26 Jul 2023 21:31:52 +0000 Subject: [PATCH] feat(cli): support fine-grained server log filtering --- agent/agent.go | 2 +- cli/server.go | 67 +++++++++++++++++-- cli/server_test.go | 10 +-- cli/testdata/coder_server_--help.golden | 10 +-- cli/testdata/server-config.yaml.golden | 7 +- coderd/apidoc/docs.go | 6 ++ coderd/apidoc/swagger.json | 6 ++ coderd/workspaceagents.go | 2 +- codersdk/deployment.go | 31 ++++++--- docs/api/general.md | 1 + docs/api/schemas.md | 14 ++-- docs/cli/server.md | 22 +++--- .../cli/testdata/coder_server_--help.golden | 10 +-- enterprise/wsproxy/wsproxy.go | 6 +- site/src/api/typesGenerated.ts | 2 + tailnet/conn.go | 12 ++-- 16 files changed, 148 insertions(+), 60 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index e3cac00663c71..d736564056791 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -755,7 +755,7 @@ func (a *agent) createTailnet(ctx context.Context, agentID uuid.UUID, derpMap *t network, err := tailnet.NewConn(&tailnet.Options{ Addresses: a.wireguardAddresses(agentID), DERPMap: derpMap, - Logger: a.logger.Named("tailnet"), + Logger: a.logger.Named("net.tailnet"), ListenPort: a.tailnetListenPort, BlockEndpoints: disableDirectConnections, }) diff --git a/cli/server.go b/cli/server.go index 29de70def1466..51cac38471ab2 100644 --- a/cli/server.go +++ b/cli/server.go @@ -334,7 +334,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. ) if cfg.AccessURL.String() == "" { cliui.Infof(inv.Stderr, "Opening tunnel so workspaces can connect to your deployment. For production scenarios, specify an external access URL") - tunnel, err = devtunnel.New(ctx, logger.Named("devtunnel"), cfg.WgtunnelHost.String()) + tunnel, err = devtunnel.New(ctx, logger.Named("net.devtunnel"), cfg.WgtunnelHost.String()) if err != nil { return xerrors.Errorf("create tunnel: %w", err) } @@ -1751,6 +1751,50 @@ func IsLocalhost(host string) bool { return host == "localhost" || host == "127.0.0.1" || host == "::1" } +var _ slog.Sink = &filterSink{} + +type filterSink struct { + next []slog.Sink + re *regexp.Regexp +} + +func (f *filterSink) compile(res []string) error { + if len(res) == 0 { + return nil + } + + var reb strings.Builder + for i, re := range res { + _, _ = fmt.Fprintf(&reb, "(%s)", re) + if i != len(res)-1 { + _, _ = reb.WriteRune('|') + } + } + + re, err := regexp.Compile(reb.String()) + if err != nil { + return xerrors.Errorf("compile regex: %w", err) + } + f.re = re + return nil +} + +func (f *filterSink) LogEntry(ctx context.Context, ent slog.SinkEntry) { + logName := strings.Join(ent.LoggerNames, ".") + if f.re != nil && !f.re.MatchString(logName) { + return + } + for _, sink := range f.next { + sink.LogEntry(ctx, ent) + } +} + +func (f *filterSink) Sync() { + for _, sink := range f.next { + sink.Sync() + } +} + func BuildLogger(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (slog.Logger, func(), error) { var ( sinks = []slog.Sink{} @@ -1795,16 +1839,25 @@ func BuildLogger(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (slog. sinks = append(sinks, tracing.SlogSink{}) } - level := slog.LevelInfo - if cfg.Verbose { - level = slog.LevelDebug - } - + // User should log to null device if they don't want logs. if len(sinks) == 0 { return slog.Logger{}, nil, xerrors.New("no loggers provided") } - return slog.Make(sinks...).Leveled(level), func() { + filter := &filterSink{next: sinks} + + err = filter.compile(cfg.Logging.Filter.Value()) + if err != nil { + return slog.Logger{}, nil, xerrors.Errorf("compile filters: %w", err) + } + + level := slog.LevelInfo + // Debug logging is always enabled if a filter is present. + if cfg.Verbose || filter.re != nil { + level = slog.LevelDebug + } + + return slog.Make(filter).Leveled(level), func() { for _, closer := range closers { _ = closer() } diff --git a/cli/server_test.go b/cli/server_test.go index a6f33be695672..ee00499c4d2a6 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -1305,7 +1305,7 @@ func TestServer(t *testing.T) { root, _ := clitest.New(t, "server", - "--verbose", + "--log-filter=.*", "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", @@ -1322,7 +1322,7 @@ func TestServer(t *testing.T) { root, _ := clitest.New(t, "server", - "--verbose", + "--log-filter=.*", "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", @@ -1339,7 +1339,7 @@ func TestServer(t *testing.T) { root, _ := clitest.New(t, "server", - "--verbose", + "--log-filter=.*", "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", @@ -1359,7 +1359,7 @@ func TestServer(t *testing.T) { inv, _ := clitest.New(t, "server", - "--verbose", + "--log-filter=.*", "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", @@ -1393,7 +1393,7 @@ func TestServer(t *testing.T) { // HTTP. inv, _ := clitest.New(t, "server", - "--verbose", + "--log-filter=.*", "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 35d68b84a0772..cb7ca61b4913a 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -83,12 +83,13 @@ Use a YAML configuration file when your server launch become unwieldy. --log-json string, $CODER_LOGGING_JSON Output JSON logs to a given file. + -l, --log-filter string-array, $CODER_LOG_FILTER + Filter debug logs by matching against a given regex. Use .* to match + all debug logs. + --log-stackdriver string, $CODER_LOGGING_STACKDRIVER Output Stackdriver compatible logs to a given file. - -v, --verbose bool, $CODER_VERBOSE - Output debug-level logs. - Introspection / Prometheus Options --prometheus-address host:port, $CODER_PROMETHEUS_ADDRESS (default: 127.0.0.1:2112) The bind address to serve prometheus metrics. @@ -106,8 +107,7 @@ Use a YAML configuration file when your server launch become unwieldy. --trace-logs bool, $CODER_TRACE_LOGS Enables capturing of logs as events in traces. This is useful for debugging, but may result in a very large amount of events being sent - to the tracing backend which may incur significant costs. If the - verbose flag was supplied, debug-level logs will be included. + to the tracing backend which may incur significant costs. --trace bool, $CODER_TRACE_ENABLE Whether application tracing data is collected. It exports to a backend diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 5d4171a8888df..c7a8df03414e6 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -183,14 +183,17 @@ introspection: enable: false # Enables capturing of logs as events in traces. This is useful for debugging, but # may result in a very large amount of events being sent to the tracing backend - # which may incur significant costs. If the verbose flag was supplied, debug-level - # logs will be included. + # which may incur significant costs. # (default: , type: bool) captureLogs: false logging: # Output debug-level logs. # (default: , type: bool) verbose: false + # Filter debug logs by matching against a given regex. Use .* to match all debug + # logs. + # (default: , type: string-array) + filter: [] # Output human-readable logs to a given file. # (default: /dev/stderr, type: string) humanPath: /dev/stderr diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 5c8a81385eb4d..19012d4110193 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -8304,6 +8304,12 @@ const docTemplate = `{ "json": { "type": "string" }, + "log_filter": { + "type": "array", + "items": { + "type": "string" + } + }, "stackdriver": { "type": "string" } diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index de119075e17ec..f5899353f00a6 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7448,6 +7448,12 @@ "json": { "type": "string" }, + "log_filter": { + "type": "array", + "items": { + "type": "string" + } + }, "stackdriver": { "type": "string" } diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 2fd5aa49aeb44..c0a4f064cb392 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -691,7 +691,7 @@ func (api *API) _dialWorkspaceAgentTailnet(agentID uuid.UUID) (*codersdk.Workspa conn, err := tailnet.NewConn(&tailnet.Options{ Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, DERPMap: api.DERPMap(), - Logger: api.Logger.Named("tailnet"), + Logger: api.Logger.Named("net.tailnet"), BlockEndpoints: api.DeploymentValues.DERP.Config.BlockDirect.Value(), }) if err != nil { diff --git a/codersdk/deployment.go b/codersdk/deployment.go index f9350f2cf0875..23536cae55b4a 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -340,9 +340,10 @@ type SwaggerConfig struct { } type LoggingConfig struct { - Human clibase.String `json:"human" typescript:",notnull"` - JSON clibase.String `json:"json" typescript:",notnull"` - Stackdriver clibase.String `json:"stackdriver" typescript:",notnull"` + Filter clibase.StringArray `json:"log_filter" typescript:",notnull"` + Human clibase.String `json:"human" typescript:",notnull"` + JSON clibase.String `json:"json" typescript:",notnull"` + Stackdriver clibase.String `json:"stackdriver" typescript:",notnull"` } type DangerousConfig struct { @@ -533,6 +534,16 @@ when required by your organization's security policy.`, Group: &deploymentGroupNetworking, YAML: "redirectToAccessURL", } + logFilter := clibase.Option{ + Name: "Log Filter", + Description: "Filter debug logs by matching against a given regex. Use .* to match all debug logs.", + Flag: "log-filter", + FlagShorthand: "l", + Env: "CODER_LOG_FILTER", + Value: &c.Logging.Filter, + Group: &deploymentGroupIntrospectionLogging, + YAML: "filter", + } opts := clibase.OptionSet{ { Name: "Access URL", @@ -1159,7 +1170,7 @@ when required by your organization's security policy.`, }, { Name: "Capture Logs in Traces", - Description: "Enables capturing of logs as events in traces. This is useful for debugging, but may result in a very large amount of events being sent to the tracing backend which may incur significant costs. If the verbose flag was supplied, debug-level logs will be included.", + Description: "Enables capturing of logs as events in traces. This is useful for debugging, but may result in a very large amount of events being sent to the tracing backend which may incur significant costs.", Flag: "trace-logs", Env: "CODER_TRACE_LOGS", Value: &c.Trace.CaptureLogs, @@ -1249,12 +1260,14 @@ when required by your organization's security policy.`, Flag: "verbose", Env: "CODER_VERBOSE", FlagShorthand: "v", - - Value: &c.Verbose, - Group: &deploymentGroupIntrospectionLogging, - YAML: "verbose", - Annotations: clibase.Annotations{}.Mark(annotationExternalProxies, "true"), + Hidden: true, + UseInstead: []clibase.Option{logFilter}, + Value: &c.Verbose, + Group: &deploymentGroupIntrospectionLogging, + YAML: "verbose", + Annotations: clibase.Annotations{}.Mark(annotationExternalProxies, "true"), }, + logFilter, { Name: "Human Log Location", Description: "Output human-readable logs to a given file.", diff --git a/docs/api/general.md b/docs/api/general.md index a11c48c3f6be1..bb64823bd86c9 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -236,6 +236,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "logging": { "human": "string", "json": "string", + "log_filter": ["string"], "stackdriver": "string" }, "max_session_expiry": 0, diff --git a/docs/api/schemas.md b/docs/api/schemas.md index d2947563290df..9bf6b0eea09b0 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2026,6 +2026,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "logging": { "human": "string", "json": "string", + "log_filter": ["string"], "stackdriver": "string" }, "max_session_expiry": 0, @@ -2382,6 +2383,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "logging": { "human": "string", "json": "string", + "log_filter": ["string"], "stackdriver": "string" }, "max_session_expiry": 0, @@ -3119,17 +3121,19 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in { "human": "string", "json": "string", + "log_filter": ["string"], "stackdriver": "string" } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| ------------- | ------ | -------- | ------------ | ----------- | -| `human` | string | false | | | -| `json` | string | false | | | -| `stackdriver` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ------------- | --------------- | -------- | ------------ | ----------- | +| `human` | string | false | | | +| `json` | string | false | | | +| `log_filter` | array of string | false | | | +| `stackdriver` | string | false | | | ## codersdk.LoginType diff --git a/docs/cli/server.md b/docs/cli/server.md index 684c2a3f8ba46..90c60d7392f00 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -69,7 +69,7 @@ The directory to cache temporary files. If unspecified and $CACHE_DIRECTORY is s | Environment | $CODER_TRACE_LOGS | | YAML | introspection.tracing.captureLogs | -Enables capturing of logs as events in traces. This is useful for debugging, but may result in a very large amount of events being sent to the tracing backend which may incur significant costs. If the verbose flag was supplied, debug-level logs will be included. +Enables capturing of logs as events in traces. This is useful for debugging, but may result in a very large amount of events being sent to the tracing backend which may incur significant costs. ### -c, --config @@ -317,6 +317,16 @@ Output human-readable logs to a given file. Output JSON logs to a given file. +### -l, --log-filter + +| | | +| ----------- | ----------------------------------------- | +| Type | string-array | +| Environment | $CODER_LOG_FILTER | +| YAML | introspection.logging.filter | + +Filter debug logs by matching against a given regex. Use .\* to match all debug logs. + ### --max-token-lifetime | | | @@ -948,16 +958,6 @@ Enables trace exporting to Honeycomb.io using the provided API Key. Periodically check for new releases of Coder and inform the owner. The check is performed once per day. -### -v, --verbose - -| | | -| ----------- | ------------------------------------------ | -| Type | bool | -| Environment | $CODER_VERBOSE | -| YAML | introspection.logging.verbose | - -Output debug-level logs. - ### --wildcard-access-url | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 35d68b84a0772..cb7ca61b4913a 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -83,12 +83,13 @@ Use a YAML configuration file when your server launch become unwieldy. --log-json string, $CODER_LOGGING_JSON Output JSON logs to a given file. + -l, --log-filter string-array, $CODER_LOG_FILTER + Filter debug logs by matching against a given regex. Use .* to match + all debug logs. + --log-stackdriver string, $CODER_LOGGING_STACKDRIVER Output Stackdriver compatible logs to a given file. - -v, --verbose bool, $CODER_VERBOSE - Output debug-level logs. - Introspection / Prometheus Options --prometheus-address host:port, $CODER_PROMETHEUS_ADDRESS (default: 127.0.0.1:2112) The bind address to serve prometheus metrics. @@ -106,8 +107,7 @@ Use a YAML configuration file when your server launch become unwieldy. --trace-logs bool, $CODER_TRACE_LOGS Enables capturing of logs as events in traces. This is useful for debugging, but may result in a very large amount of events being sent - to the tracing backend which may incur significant costs. If the - verbose flag was supplied, debug-level logs will be included. + to the tracing backend which may incur significant costs. --trace bool, $CODER_TRACE_ENABLE Whether application tracing data is collected. It exports to a backend diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go index 4536fe2bab7dc..1532e38914ddb 100644 --- a/enterprise/wsproxy/wsproxy.go +++ b/enterprise/wsproxy/wsproxy.go @@ -178,7 +178,7 @@ func New(ctx context.Context, opts *Options) (*Server, error) { ServerName: opts.AccessURL.Hostname(), } - derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(opts.Logger.Named("derp"))) + derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(opts.Logger.Named("net.derp"))) ctx, cancel := context.WithCancel(context.Background()) r := chi.NewRouter() @@ -186,11 +186,11 @@ func New(ctx context.Context, opts *Options) (*Server, error) { Options: opts, Handler: r, DashboardURL: opts.DashboardURL, - Logger: opts.Logger.Named("workspace-proxy"), + Logger: opts.Logger.Named("net.workspace-proxy"), TracerProvider: opts.Tracing, PrometheusRegistry: opts.PrometheusRegistry, SDKClient: client, - derpMesh: derpmesh.New(opts.Logger.Named("derpmesh"), derpServer, meshTLSConfig), + derpMesh: derpmesh.New(opts.Logger.Named("net.derpmesh"), derpServer, meshTLSConfig), ctx: ctx, cancel: cancel, } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d5eeae2e93697..fcb71675e11e6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -550,6 +550,8 @@ export interface LinkConfig { // From codersdk/deployment.go export interface LoggingConfig { + // This is likely an enum in an external package ("github.com/coder/coder/cli/clibase.StringArray") + readonly log_filter: string[] readonly human: string readonly json: string readonly stackdriver: string diff --git a/tailnet/conn.go b/tailnet/conn.go index 0534cf961771a..089a83ff79ff9 100644 --- a/tailnet/conn.go +++ b/tailnet/conn.go @@ -129,7 +129,7 @@ func NewConn(options *Options) (conn *Conn, err error) { AllowedIPs: options.Addresses, } - wireguardMonitor, err := monitor.New(Logger(options.Logger.Named("wgmonitor"))) + wireguardMonitor, err := monitor.New(Logger(options.Logger.Named("net.wgmonitor"))) if err != nil { return nil, xerrors.Errorf("create wireguard link monitor: %w", err) } @@ -141,9 +141,9 @@ func NewConn(options *Options) (conn *Conn, err error) { IP() dialer := &tsdial.Dialer{ - Logf: Logger(options.Logger.Named("tsdial")), + Logf: Logger(options.Logger.Named("net.tsdial")), } - wireguardEngine, err := wgengine.NewUserspaceEngine(Logger(options.Logger.Named("wgengine")), wgengine.Config{ + wireguardEngine, err := wgengine.NewUserspaceEngine(Logger(options.Logger.Named("net.wgengine")), wgengine.Config{ LinkMonitor: wireguardMonitor, Dialer: dialer, ListenPort: options.ListenPort, @@ -183,7 +183,7 @@ func NewConn(options *Options) (conn *Conn, err error) { netMap.SelfNode.DiscoKey = magicConn.DiscoPublicKey() netStack, err := netstack.Create( - Logger(options.Logger.Named("netstack")), + Logger(options.Logger.Named("net.netstack")), tunDevice, wireguardEngine, magicConn, @@ -216,7 +216,7 @@ func NewConn(options *Options) (conn *Conn, err error) { localIPs, logIPs, nil, - Logger(options.Logger.Named("packet-filter")), + Logger(options.Logger.Named("net.packet-filter")), )) dialContext, dialCancel := context.WithCancel(context.Background()) @@ -495,7 +495,7 @@ func (c *Conn) UpdateNodes(nodes []*Node, replacePeers bool) error { } func (c *Conn) reconfig() error { - cfg, err := nmcfg.WGCfg(c.netMap, Logger(c.logger.Named("wgconfig")), netmap.AllowSingleHosts, "") + cfg, err := nmcfg.WGCfg(c.netMap, Logger(c.logger.Named("net.wgconfig")), netmap.AllowSingleHosts, "") if err != nil { return xerrors.Errorf("update wireguard config: %w", err) }