refactor: normalize workspace agent session metadata (#28124)

Phase 1 of the Scalable Approach for Adding New IDE Session Types RFC,
tracked in #27410.

Gives agent session accounting one canonical vocabulary for app names, so
later work can report a count per app without the agent and the server
disagreeing about what an app is called.

- Adds codersdk/appname.go: NormalizeAppName for storage and lookup, and
  AppFamilyName for the places that need a bounded value, such as a
  Prometheus label or the Connection_Type enum.
- agentssh.Server keeps session counters in a map keyed by normalized name
  instead of three fixed atomics, dropping entries at zero.
- Drops MagicSessionType. Sessions carry plain app names, and unrecognized
  ones flow through as themselves rather than collapsing to unknown.

Wire behavior is unchanged: the agent still reports the four session_count_*
fields over Agent API 2.10.
This commit is contained in:
Ehab Younes
2026-08-21 02:22:18 +03:00
committed by GitHub
parent 3ffb266af6
commit c767cc5198
9 changed files with 391 additions and 137 deletions
+12 -13
View File
@@ -424,19 +424,18 @@ func (a *agent) init() {
BlockFileTransfer: a.blockFileTransfer,
BlockReversePortForwarding: a.blockReversePortForwarding,
BlockLocalPortForwarding: a.blockLocalPortForwarding,
ReportConnection: func(id uuid.UUID, magicType agentssh.MagicSessionType, ip string) func(code int, reason string) {
ReportConnection: func(id uuid.UUID, appName string, ip string) func(code int, reason string) {
var connectionType proto.Connection_Type
switch magicType {
case agentssh.MagicSessionTypeSSH:
// Connection_Type is a fixed enum, stored as a database enum in
// the connection log, so it can only hold a family.
switch codersdk.AppNameFamily(appName) {
case codersdk.AppFamilySSH:
connectionType = proto.Connection_SSH
case agentssh.MagicSessionTypeVSCode:
case codersdk.AppFamilyVSCode:
connectionType = proto.Connection_VSCODE
case agentssh.MagicSessionTypeJetBrains:
case codersdk.AppFamilyJetBrains:
connectionType = proto.Connection_JETBRAINS
case agentssh.MagicSessionTypeUnknown:
connectionType = proto.Connection_TYPE_UNSPECIFIED
default:
a.logger.Error(a.hardCtx, "unhandled magic session type when reporting connection", slog.F("magic_type", magicType))
connectionType = proto.Connection_TYPE_UNSPECIFIED
}
@@ -2162,11 +2161,11 @@ func (a *agent) Collect(ctx context.Context, networkStats map[netlogtype.Connect
stats.TxPackets += int64(counts.TxPackets)
}
// The count of active sessions.
sshStats := a.sshServer.ConnStats()
stats.SessionCountSsh = sshStats.Sessions
stats.SessionCountVscode = sshStats.VSCode
stats.SessionCountJetbrains = sshStats.JetBrains
// The count of active sessions; types without a protocol field are dropped.
sessionCounts := a.sshServer.SessionCounts()
stats.SessionCountSsh = sessionCounts[string(codersdk.AppFamilySSH)]
stats.SessionCountVscode = sessionCounts[string(codersdk.AppFamilyVSCode)]
stats.SessionCountJetbrains = sessionCounts[string(codersdk.AppFamilyJetBrains)]
stats.SessionCountReconnectingPty = a.reconnectingPTYServer.ConnCount()
+4 -4
View File
@@ -310,13 +310,13 @@ func TestAgent_Stats_Magic(t *testing.T) {
defer sshClient.Close()
session, err := sshClient.NewSession()
require.NoError(t, err)
session.Setenv(agentssh.MagicSessionTypeEnvironmentVariable, string(agentssh.MagicSessionTypeVSCode))
session.Setenv(agentssh.AppNameEnvironmentVariable, string(codersdk.AppFamilyVSCode))
defer session.Close()
command := "sh -c 'echo $" + agentssh.MagicSessionTypeEnvironmentVariable + "'"
command := "sh -c 'echo $" + agentssh.AppNameEnvironmentVariable + "'"
expected := ""
if runtime.GOOS == "windows" {
expected = "%" + agentssh.MagicSessionTypeEnvironmentVariable + "%"
expected = "%" + agentssh.AppNameEnvironmentVariable + "%"
command = "cmd.exe /c echo " + expected
}
output, err := session.Output(command)
@@ -338,7 +338,7 @@ func TestAgent_Stats_Magic(t *testing.T) {
defer sshClient.Close()
session, err := sshClient.NewSession()
require.NoError(t, err)
session.Setenv(agentssh.MagicSessionTypeEnvironmentVariable, string(agentssh.MagicSessionTypeVSCode))
session.Setenv(agentssh.AppNameEnvironmentVariable, string(codersdk.AppFamilyVSCode))
defer session.Close()
stdin, err := session.StdinPipe()
require.NoError(t, err)
+97 -100
View File
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"maps"
"net"
"os"
"os/exec"
@@ -53,14 +54,12 @@ const (
BlockedFileTransferErrorMessage = "File transfer has been disabled."
)
// MagicSessionType is a type that represents the type of session that is being
// established.
type MagicSessionType string
const (
// MagicSessionTypeEnvironmentVariable is used to track the purpose behind an SSH connection.
// This is stripped from any commands being executed, and is counted towards connection stats.
MagicSessionTypeEnvironmentVariable = "CODER_SSH_SESSION_TYPE"
// AppNameEnvironmentVariable carries the name of the app opening an SSH
// connection. This is stripped from any commands being executed, and is
// counted towards connection stats. The value keeps its original spelling
// so existing clients keep working.
AppNameEnvironmentVariable = "CODER_SSH_SESSION_TYPE"
// ContainerEnvironmentVariable is used to specify the target container for an SSH connection.
// This is stripped from any commands being executed.
// Only available if CODER_AGENT_DEVCONTAINERS_ENABLE=true.
@@ -71,23 +70,14 @@ const (
ContainerUserEnvironmentVariable = "CODER_CONTAINER_USER"
)
// MagicSessionType enums.
const (
// MagicSessionTypeUnknown means the session type could not be determined.
MagicSessionTypeUnknown MagicSessionType = "unknown"
// MagicSessionTypeSSH is the default session type.
MagicSessionTypeSSH MagicSessionType = "ssh"
// MagicSessionTypeVSCode is set in the SSH config by the VS Code extension to identify itself.
MagicSessionTypeVSCode MagicSessionType = "vscode"
// MagicSessionTypeJetBrains is set in the SSH config by the JetBrains
// extension to identify itself.
MagicSessionTypeJetBrains MagicSessionType = "jetbrains"
)
// BlockedFileTransferCommands contains a list of restricted file transfer commands.
var BlockedFileTransferCommands = []string{"nc", "rsync", "scp", "sftp"}
type reportConnectionFunc func(id uuid.UUID, sessionType MagicSessionType, ip string) (disconnected func(code int, reason string))
type reportConnectionFunc func(id uuid.UUID, sessionType string, ip string) (disconnected func(code int, reason string))
// startSessionFunc counts a session until endSession is called, which must
// happen exactly once.
type startSessionFunc func(sessionType string) (endSession func())
// Config sets configuration parameters for the agent SSH server.
type Config struct {
@@ -148,6 +138,10 @@ type Server struct {
// a lock on mu but protected by closing.
wg sync.WaitGroup
// Kept off mu, which Close holds while closing sessions.
sessionCountsMu sync.Mutex
sessionCounts map[string]int64
Execer agentexec.Execer
logger slog.Logger
srv *ssh.Server
@@ -155,10 +149,6 @@ type Server struct {
config *Config
connCountVSCode atomic.Int64
connCountJetBrains atomic.Int64
connCountSSHSession atomic.Int64
metrics *sshServerMetrics
}
@@ -192,7 +182,7 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
config.EnvInfo = &usershell.SystemEnvInfo{}
}
if config.ReportConnection == nil {
config.ReportConnection = func(uuid.UUID, MagicSessionType, string) func(int, string) { return func(int, string) {} }
config.ReportConnection = func(uuid.UUID, string, string) func(int, string) { return func(int, string) {} }
}
forwardHandler := &ssh.ForwardedTCPHandler{}
@@ -200,13 +190,14 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
metrics := newSSHServerMetrics(prometheusRegistry)
s := &Server{
Execer: execer,
listeners: make(map[net.Listener]struct{}),
fs: fs,
conns: make(map[net.Conn]struct{}),
sessions: make(map[ssh.Session]struct{}),
processes: make(map[*os.Process]struct{}),
logger: logger,
Execer: execer,
listeners: make(map[net.Listener]struct{}),
fs: fs,
conns: make(map[net.Conn]struct{}),
sessions: make(map[ssh.Session]struct{}),
processes: make(map[*os.Process]struct{}),
sessionCounts: make(map[string]int64),
logger: logger,
config: config,
@@ -232,7 +223,7 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
ChannelHandlers: map[string]ssh.ChannelHandler{
"direct-tcpip": func(srv *ssh.Server, conn *gossh.ServerConn, newChan gossh.NewChannel, ctx ssh.Context) {
// Wrapper is designed to find and track JetBrains Gateway connections.
wrapped := NewJetbrainsChannelWatcher(ctx, s.logger, s.config.ReportConnection, newChan, &s.connCountJetBrains)
wrapped := NewJetbrainsChannelWatcher(ctx, s.logger, s.config.ReportConnection, newChan, s.startSession)
ssh.DirectTCPIPHandler(srv, conn, wrapped, ctx)
},
"direct-streamlocal@openssh.com": func(srv *ssh.Server, conn *gossh.ServerConn, newChan gossh.NewChannel, ctx ssh.Context) {
@@ -324,44 +315,49 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
return s, nil
}
type ConnStats struct {
Sessions int64
VSCode int64
JetBrains int64
}
func (s *Server) ConnStats() ConnStats {
return ConnStats{
Sessions: s.connCountSSHSession.Load(),
VSCode: s.connCountVSCode.Load(),
JetBrains: s.connCountJetBrains.Load(),
// startSession counts a session until the returned function is called.
func (s *Server) startSession(appName string) (endSession func()) {
key := codersdk.NormalizeAppName(appName)
s.sessionCountsMu.Lock()
defer s.sessionCountsMu.Unlock()
s.sessionCounts[key]++
return func() {
s.sessionCountsMu.Lock()
defer s.sessionCountsMu.Unlock()
s.sessionCounts[key]--
if s.sessionCounts[key] <= 0 {
delete(s.sessionCounts, key)
}
}
}
func extractMagicSessionType(env []string) (magicType MagicSessionType, rawType string, filteredEnv []string) {
// SessionCounts returns active sessions per app name, omitting zeroes. Never
// nil, so callers can merge in other sources.
func (s *Server) SessionCounts() map[string]int64 {
s.sessionCountsMu.Lock()
defer s.sessionCountsMu.Unlock()
return maps.Clone(s.sessionCounts)
}
func extractAppName(env []string) (appName, rawAppName string, filteredEnv []string) {
for _, kv := range env {
if !strings.HasPrefix(kv, MagicSessionTypeEnvironmentVariable) {
if !strings.HasPrefix(kv, AppNameEnvironmentVariable) {
continue
}
rawType = strings.TrimPrefix(kv, MagicSessionTypeEnvironmentVariable+"=")
rawAppName = strings.TrimPrefix(kv, AppNameEnvironmentVariable+"=")
// Keep going, we'll use the last instance of the env.
}
// Always force lowercase checking to be case-insensitive.
switch MagicSessionType(strings.ToLower(rawType)) {
case MagicSessionTypeVSCode:
magicType = MagicSessionTypeVSCode
case MagicSessionTypeJetBrains:
magicType = MagicSessionTypeJetBrains
case "", MagicSessionTypeSSH:
magicType = MagicSessionTypeSSH
default:
magicType = MagicSessionTypeUnknown
if rawAppName == "" {
appName = string(codersdk.AppFamilySSH)
} else {
// Normalize, don't classify: unknown names flow through.
appName = codersdk.NormalizeAppName(rawAppName)
}
return magicType, rawType, slices.DeleteFunc(env, func(kv string) bool {
return strings.HasPrefix(kv, MagicSessionTypeEnvironmentVariable+"=")
return appName, rawAppName, slices.DeleteFunc(env, func(kv string) bool {
return strings.HasPrefix(kv, AppNameEnvironmentVariable+"=")
})
}
@@ -423,7 +419,14 @@ func (s *Server) sessionHandler(session ssh.Session) {
logger.Info(ctx, "handling ssh session")
env := session.Environ()
magicType, magicTypeRaw, env := extractMagicSessionType(env)
appName, rawAppName, env := extractAppName(env)
family := codersdk.AppNameFamily(appName)
if family == codersdk.AppFamilyUnknown {
logger.Debug(ctx, "unrecognized ssh session type",
slog.F("app_name", appName),
slog.F("raw_app_name", rawAppName),
)
}
// It's not safe to assume RemoteAddr() returns a non-nil value. slog.F usage is fine because it correctly
// handles nil.
@@ -437,7 +440,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
if !s.trackSession(session, true) {
reason := "unable to accept new session, server is closing"
// Report connection attempt even if we couldn't accept it.
disconnected := s.config.ReportConnection(id, magicType, remoteAddrString)
disconnected := s.config.ReportConnection(id, appName, remoteAddrString)
defer disconnected(1, reason)
logger.Info(ctx, reason)
@@ -449,19 +452,13 @@ func (s *Server) sessionHandler(session ssh.Session) {
reportSession := true
switch magicType {
case MagicSessionTypeVSCode:
s.connCountVSCode.Add(1)
defer s.connCountVSCode.Add(-1)
case MagicSessionTypeJetBrains:
if family == codersdk.AppFamilyJetBrains {
// Do nothing here because JetBrains launches hundreds of ssh sessions.
// We instead track JetBrains in the single persistent tcp forwarding channel.
reportSession = false
case MagicSessionTypeSSH:
s.connCountSSHSession.Add(1)
defer s.connCountSSHSession.Add(-1)
case MagicSessionTypeUnknown:
logger.Warn(ctx, "invalid magic ssh session type specified", slog.F("raw_type", magicTypeRaw))
} else {
endSession := s.startSession(appName)
defer endSession()
}
closeCause := func(_ string) {}
@@ -472,7 +469,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
scr := &sessionCloseTracker{Session: session}
session = scr
disconnected := s.config.ReportConnection(id, magicType, remoteAddrString)
disconnected := s.config.ReportConnection(id, appName, remoteAddrString)
defer func() {
logger.Info(ctx, "ssh session closed",
codersdk.ConnectionDirectionAgentToClient.SlogField(),
@@ -537,7 +534,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
env = append(env, fmt.Sprintf("DISPLAY=localhost:%d.%d", display, x11.ScreenNumber))
}
err := s.sessionStart(logger, session, env, magicType, container, containerUser)
err := s.sessionStart(logger, session, env, appName, container, containerUser)
var exitError *exec.ExitError
if xerrors.As(err, &exitError) {
code := exitError.ExitCode()
@@ -611,10 +608,10 @@ func (s *Server) fileTransferBlocked(session ssh.Session) bool {
return false
}
func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []string, magicType MagicSessionType, container, containerUser string) (retErr error) {
func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []string, appName, container, containerUser string) (retErr error) {
ctx := session.Context()
magicTypeLabel := magicTypeMetricLabel(magicType)
appLabel := appNameMetricLabel(appName)
sshPty, windowSize, isPty := session.Pty()
ptyLabel := "no"
if isPty {
@@ -626,20 +623,20 @@ func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []str
if s.config.ExperimentalContainers && container != "" {
ei, err = agentcontainers.EnvInfo(ctx, s.Execer, container, containerUser)
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, ptyLabel, "container_env_info").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, ptyLabel, "container_env_info").Add(1)
return err
}
}
cmd, err := s.CreateCommand(ctx, session.RawCommand(), env, ei)
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, ptyLabel, "create_command").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, ptyLabel, "create_command").Add(1)
return err
}
if ssh.AgentRequested(session) {
l, err := ssh.NewAgentListener()
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, ptyLabel, "listener").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, ptyLabel, "listener").Add(1)
return xerrors.Errorf("new agent listener: %w", err)
}
defer l.Close()
@@ -648,13 +645,13 @@ func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []str
}
if isPty {
return s.startPTYSession(logger, session, magicTypeLabel, cmd, sshPty, windowSize)
return s.startPTYSession(logger, session, appLabel, cmd, sshPty, windowSize)
}
return s.startNonPTYSession(logger, session, magicTypeLabel, cmd.AsExec())
return s.startNonPTYSession(logger, session, appLabel, cmd.AsExec())
}
func (s *Server) startNonPTYSession(logger slog.Logger, session ssh.Session, magicTypeLabel string, cmd *exec.Cmd) error {
s.metrics.sessionsTotal.WithLabelValues(magicTypeLabel, "no").Add(1)
func (s *Server) startNonPTYSession(logger slog.Logger, session ssh.Session, appLabel string, cmd *exec.Cmd) error {
s.metrics.sessionsTotal.WithLabelValues(appLabel, "no").Add(1)
// Create a process group and send SIGHUP to child processes,
// otherwise context cancellation will not propagate properly
@@ -673,19 +670,19 @@ func (s *Server) startNonPTYSession(logger slog.Logger, session ssh.Session, mag
// use StdinPipe. It's unknown what causes this.
stdinPipe, err := cmd.StdinPipe()
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "no", "stdin_pipe").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "no", "stdin_pipe").Add(1)
return xerrors.Errorf("create stdin pipe: %w", err)
}
go func() {
_, err := io.Copy(stdinPipe, session)
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "no", "stdin_io_copy").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "no", "stdin_io_copy").Add(1)
}
_ = stdinPipe.Close()
}()
err = cmd.Start()
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "no", "start_command").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "no", "start_command").Add(1)
return xerrors.Errorf("start: %w", err)
}
@@ -706,7 +703,7 @@ func (s *Server) startNonPTYSession(logger slog.Logger, session ssh.Session, mag
}()
go func() {
for sig := range sigs {
handleSignal(logger, sig, cmd.Process, s.metrics, magicTypeLabel)
handleSignal(logger, sig, cmd.Process, s.metrics, appLabel)
}
}()
return cmd.Wait()
@@ -722,8 +719,8 @@ type ptySession interface {
Signals(chan<- ssh.Signal)
}
func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTypeLabel string, cmd *pty.Cmd, sshPty ssh.Pty, windowSize <-chan ssh.Window) (retErr error) {
s.metrics.sessionsTotal.WithLabelValues(magicTypeLabel, "yes").Add(1)
func (s *Server) startPTYSession(logger slog.Logger, session ptySession, appLabel string, cmd *pty.Cmd, sshPty ssh.Pty, windowSize <-chan ssh.Window) (retErr error) {
s.metrics.sessionsTotal.WithLabelValues(appLabel, "yes").Add(1)
ctx := session.Context()
// Disable minimal PTY emulation set by gliderlabs/ssh (NL-to-CRNL).
@@ -737,7 +734,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
err := showAnnouncementBanner(session, banner)
if err != nil {
logger.Error(ctx, "agent failed to show announcement banner", slog.Error(err))
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "announcement_banner").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "announcement_banner").Add(1)
break
}
}
@@ -748,7 +745,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
err := showMOTD(s.fs, session, s.config.MOTDFile())
if err != nil {
logger.Error(ctx, "agent failed to show MOTD", slog.Error(err))
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "motd").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "motd").Add(1)
}
}
@@ -760,14 +757,14 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
pty.WithLogger(slog.Stdlib(ctx, logger, slog.LevelInfo)),
))
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "start_command").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "start_command").Add(1)
return xerrors.Errorf("start command: %w", err)
}
defer func() {
closeErr := ptty.Close()
if closeErr != nil {
logger.Warn(ctx, "failed to close tty", slog.Error(closeErr))
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "close").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "close").Add(1)
if retErr == nil {
retErr = closeErr
}
@@ -791,7 +788,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
sigs = nil
continue
}
handleSignal(logger, sig, process, s.metrics, magicTypeLabel)
handleSignal(logger, sig, process, s.metrics, appLabel)
case win, ok := <-windowSize:
if !ok {
windowSize = nil
@@ -802,7 +799,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
// If the pty is closed, then command has exited, no need to log.
if resizeErr != nil && !errors.Is(resizeErr, pty.ErrClosed) {
logger.Warn(ctx, "failed to resize tty", slog.Error(resizeErr))
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "resize").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "resize").Add(1)
}
}
}
@@ -811,7 +808,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
go func() {
_, err := io.Copy(ptty.InputWriter(), session)
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "input_io_copy").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "input_io_copy").Add(1)
}
}()
@@ -826,7 +823,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
n, err := io.Copy(session, ptty.OutputReader())
logger.Debug(ctx, "copy output done", slog.F("bytes", n), slog.Error(err))
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "output_io_copy").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "output_io_copy").Add(1)
return xerrors.Errorf("copy error: %w", err)
}
// We've gotten all the output, but we need to wait for the process to
@@ -838,7 +835,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
// and not something to be concerned about. But, if it's something else, we should log it.
if err != nil && !xerrors.As(err, &exitErr) {
logger.Warn(ctx, "process wait exited with error", slog.Error(err))
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "wait").Add(1)
s.metrics.sessionErrors.WithLabelValues(appLabel, "yes", "wait").Add(1)
}
if err != nil {
return xerrors.Errorf("process wait: %w", err)
@@ -846,7 +843,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy
return nil
}
func handleSignal(logger slog.Logger, ssig ssh.Signal, signaler interface{ Signal(os.Signal) error }, metrics *sshServerMetrics, magicTypeLabel string) {
func handleSignal(logger slog.Logger, ssig ssh.Signal, signaler interface{ Signal(os.Signal) error }, metrics *sshServerMetrics, appLabel string) {
ctx := context.Background()
sig := osSignalFrom(ssig)
logger = logger.With(slog.F("ssh_signal", ssig), slog.F("signal", sig.String()))
@@ -854,7 +851,7 @@ func handleSignal(logger slog.Logger, ssig ssh.Signal, signaler interface{ Signa
err := signaler.Signal(sig)
if err != nil {
logger.Warn(ctx, "signaling the process failed", slog.Error(err))
metrics.sessionErrors.WithLabelValues(magicTypeLabel, "yes", "signal").Add(1)
metrics.sessionErrors.WithLabelValues(appLabel, "yes", "signal").Add(1)
}
}
+88
View File
@@ -0,0 +1,88 @@
package agentssh
import (
"fmt"
"testing"
"github.com/stretchr/testify/require"
)
func TestExtractAppName(t *testing.T) {
t.Parallel()
envWith := func(value string) []string {
return []string{
"FOO=bar",
fmt.Sprintf("%s=%s", AppNameEnvironmentVariable, value),
"BAZ=qux",
}
}
for _, tc := range []struct {
name string
env []string
want string
}{
{"NoEnvDefaultsToSSH", []string{"FOO=bar"}, "ssh"},
{"EmptyValueDefaultsToSSH", envWith(""), "ssh"},
{"VSCode", envWith("vscode"), "vscode"},
{"JetBrainsLegacyCasing", envWith("JetBrains"), "jetbrains"},
{"UnknownTypeNormalized", envWith("Cursor-Nightly"), "cursor_nightly"},
{"LastInstanceWins", append(envWith("vscode"), AppNameEnvironmentVariable+"=cursor"), "cursor"},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
appName, _, filteredEnv := extractAppName(tc.env)
require.Equal(t, tc.want, appName)
for _, kv := range filteredEnv {
require.NotContains(t, kv, AppNameEnvironmentVariable+"=")
}
})
}
}
func TestAppNameMetricLabel(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
appName string
want string
}{
{"SSH", "ssh", "ssh"},
{"ForkUsesFamily", "cursor", "vscode"},
// No alias for the nightly variant, so it does not get a label.
{"UnlistedVariant", "cursor_nightly", "unknown"},
{"Unknown", "some_new_ide", "unknown"},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tc.want, appNameMetricLabel(tc.appName))
})
}
}
func TestSessionCounts(t *testing.T) {
t.Parallel()
s := &Server{sessionCounts: make(map[string]int64)}
// Counters are created on demand and names are normalized.
_ = s.startSession("ssh")
_ = s.startSession("vscode")
endCursor1 := s.startSession("Cursor")
endCursor2 := s.startSession("cursor")
require.Equal(t, map[string]int64{
"ssh": 1,
"vscode": 1,
"cursor": 2,
}, s.SessionCounts())
// Zero-count entries are dropped, not reported as idle apps.
endCursor1()
endCursor2()
require.Equal(t, map[string]int64{
"ssh": 1,
"vscode": 1,
}, s.SessionCounts())
}
+9 -7
View File
@@ -7,7 +7,6 @@ import (
"github.com/gliderlabs/ssh"
"github.com/google/uuid"
"go.uber.org/atomic"
gossh "golang.org/x/crypto/ssh"
"cdr.dev/slog/v3"
@@ -26,15 +25,18 @@ type localForwardChannelData struct {
// JetbrainsChannelWatcher is used to track JetBrains port forwarded (Gateway)
// channels. If the port forward is something other than JetBrains, this struct
// is a noop.
//
// Sessions are counted here, not in sessionHandler: JetBrains opens hundreds
// of ssh sessions but only one persistent forwarded channel.
type JetbrainsChannelWatcher struct {
gossh.NewChannel
jetbrainsCounter *atomic.Int64
startSession startSessionFunc
logger slog.Logger
originAddr string
reportConnection reportConnectionFunc
}
func NewJetbrainsChannelWatcher(ctx ssh.Context, logger slog.Logger, reportConnection reportConnectionFunc, newChannel gossh.NewChannel, counter *atomic.Int64) gossh.NewChannel {
func NewJetbrainsChannelWatcher(ctx ssh.Context, logger slog.Logger, reportConnection reportConnectionFunc, newChannel gossh.NewChannel, startSession startSessionFunc) gossh.NewChannel {
d := localForwardChannelData{}
if err := gossh.Unmarshal(newChannel.ExtraData(), &d); err != nil {
// If the data fails to unmarshal, do nothing.
@@ -63,7 +65,7 @@ func NewJetbrainsChannelWatcher(ctx ssh.Context, logger slog.Logger, reportConne
return &JetbrainsChannelWatcher{
NewChannel: newChannel,
jetbrainsCounter: counter,
startSession: startSession,
logger: logger.With(slog.F("destination_port", d.DestPort)),
originAddr: d.OriginAddr,
reportConnection: reportConnection,
@@ -71,21 +73,21 @@ func NewJetbrainsChannelWatcher(ctx ssh.Context, logger slog.Logger, reportConne
}
func (w *JetbrainsChannelWatcher) Accept() (gossh.Channel, <-chan *gossh.Request, error) {
disconnected := w.reportConnection(uuid.New(), MagicSessionTypeJetBrains, w.originAddr)
disconnected := w.reportConnection(uuid.New(), string(codersdk.AppFamilyJetBrains), w.originAddr)
c, r, err := w.NewChannel.Accept()
if err != nil {
disconnected(1, err.Error())
return c, r, err
}
w.jetbrainsCounter.Add(1)
endSession := w.startSession(string(codersdk.AppFamilyJetBrains))
// nolint: gocritic // JetBrains is a proper noun and should be capitalized
w.logger.Debug(context.Background(), "JetBrains watcher accepted channel")
return &ChannelOnClose{
Channel: c,
done: func() {
w.jetbrainsCounter.Add(-1)
endSession()
disconnected(0, "normal close")
// nolint: gocritic // JetBrains is a proper noun and should be capitalized
w.logger.Debug(context.Background(), "JetBrains channel closed",
+5 -13
View File
@@ -1,9 +1,9 @@
package agentssh
import (
"strings"
"github.com/prometheus/client_golang/prometheus"
"github.com/coder/coder/v2/codersdk"
)
type sshServerMetrics struct {
@@ -71,15 +71,7 @@ func newSSHServerMetrics(registerer prometheus.Registerer) *sshServerMetrics {
}
}
func magicTypeMetricLabel(magicType MagicSessionType) string {
switch magicType {
case MagicSessionTypeVSCode:
case MagicSessionTypeJetBrains:
case MagicSessionTypeSSH:
case MagicSessionTypeUnknown:
default:
magicType = MagicSessionTypeUnknown
}
// Always be case insensitive
return strings.ToLower(string(magicType))
func appNameMetricLabel(appName string) string {
// Family, not raw name, so cardinality stays bounded.
return string(codersdk.AppNameFamily(appName))
}
+75
View File
@@ -0,0 +1,75 @@
package codersdk
import (
"strings"
utilstrings "github.com/coder/coder/v2/coderd/util/strings"
)
// maxAppNameLength caps an app name in runes.
const maxAppNameLength = 64
// AppFamilyName is the bounded set of names that arbitrary app names group
// into, for callers that need a fixed value such as a metric label or the
// Connection_Type enum. Server-side only, not part of the HTTP API.
//
// @typescript-ignore AppFamilyName
type AppFamilyName string
const (
AppFamilyVSCode AppFamilyName = "vscode"
AppFamilyJetBrains AppFamilyName = "jetbrains"
AppFamilySSH AppFamilyName = "ssh"
AppFamilyReconnectingPTY AppFamilyName = "reconnecting_pty"
AppFamilyUnknown AppFamilyName = "unknown"
)
// appNameFamilies never gates storage, so a missing alias only costs an
// AppFamilyUnknown label. Keys are the IDs Coder's registry modules use.
var appNameFamilies = map[string]AppFamilyName{
"vscode": AppFamilyVSCode,
"vscode_insiders": AppFamilyVSCode,
"vscode_web": AppFamilyVSCode,
"code_server": AppFamilyVSCode,
"cursor": AppFamilyVSCode,
"windsurf": AppFamilyVSCode,
"positron": AppFamilyVSCode,
"vscodium": AppFamilyVSCode,
"codium": AppFamilyVSCode,
"antigravity": AppFamilyVSCode,
"trae": AppFamilyVSCode,
"kiro": AppFamilyVSCode,
"devin": AppFamilyVSCode,
"jetbrains": AppFamilyJetBrains,
// Zed has no Connection_Type or session count field of its own, so it
// rolls up under SSH. The raw name still reaches storage.
"zed": AppFamilySSH,
"ssh": AppFamilySSH,
"reconnecting_pty": AppFamilyReconnectingPTY,
}
// AppNameFamily normalizes an app name and returns its family, or
// AppFamilyUnknown.
func AppNameFamily(appName string) AppFamilyName {
if family, ok := appNameFamilies[NormalizeAppName(appName)]; ok {
return family
}
return AppFamilyUnknown
}
// NormalizeAppName prepares a client-supplied app name for storage and
// lookup: it strips the null bytes Postgres TEXT rejects, trims, truncates,
// lowercases, and folds hyphens to underscores. Empty becomes
// AppFamilyUnknown.
func NormalizeAppName(appName string) string {
appName = strings.ReplaceAll(appName, "\x00", "")
// Trim before truncating so padding does not spend the budget, and after
// in case the cut lands in whitespace.
appName = strings.TrimSpace(appName)
appName = strings.TrimSpace(utilstrings.Truncate(appName, maxAppNameLength))
if appName == "" {
return string(AppFamilyUnknown)
}
return strings.ReplaceAll(strings.ToLower(appName), "-", "_")
}
+16
View File
@@ -0,0 +1,16 @@
package codersdk
import (
"testing"
"github.com/stretchr/testify/require"
)
// Lookups fold input through NormalizeAppName before probing, so map keys
// that are not already normalized can never match.
func TestAppNameFamilyKeysAreNormalized(t *testing.T) {
t.Parallel()
for name := range appNameFamilies {
require.Equal(t, NormalizeAppName(name), name)
}
}
+85
View File
@@ -0,0 +1,85 @@
package codersdk_test
import (
"strings"
"testing"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/codersdk"
)
// Duplicated from the package so the limit stays unexported.
const maxAppNameLength = 64
func TestNormalizeAppName(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
input string
want string
}{
{"NormalizedPassthrough", "vscode", "vscode"},
{"KnownNameCaseInsensitive", "JetBrains", "jetbrains"},
{"LegacyAlias", "reconnecting-pty", "reconnecting_pty"},
{"HyphenatedKnownName", "vscode-insiders", "vscode_insiders"},
{"UnknownHyphensFolded", "my-future-ide", "my_future_ide"},
{"UnknownLowercased", "Cursor Nightly", "cursor nightly"},
{"UnknownPreservesUnicode", "エディタ", "エディタ"},
{"StripsNullBytes", "cur\x00sor", "cursor"},
{"TrimsWhitespace", " vscode\t", "vscode"},
{"Empty", "", "unknown"},
{"OnlyNullBytes", "\x00\x00", "unknown"},
{"OnlyWhitespace", " ", "unknown"},
{"TruncatesToMaxRunes", strings.Repeat("a", maxAppNameLength+10), strings.Repeat("a", maxAppNameLength)},
{"TruncatesMultibyteSafely", strings.Repeat("あ", maxAppNameLength+1), strings.Repeat("あ", maxAppNameLength)},
// Padding is trimmed before truncating, so it does not spend the budget.
{"TrimsPaddingBeforeTruncating", " " + strings.Repeat("a", maxAppNameLength) + " ", strings.Repeat("a", maxAppNameLength)},
{"TrimsWhitespaceLeftByTruncation", strings.Repeat("a", 60) + " tail", strings.Repeat("a", 60)},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tc.want, codersdk.NormalizeAppName(tc.input))
})
}
}
// The usage-app vocabulary is declared independently of the normalized app
// names because its values are part of the HTTP API. This pins the two
// together so they cannot drift.
func TestUsageAppNamesAreNormalized(t *testing.T) {
t.Parallel()
for sdkName, want := range map[codersdk.UsageAppName]string{
codersdk.UsageAppNameVscode: "vscode",
codersdk.UsageAppNameJetbrains: "jetbrains",
codersdk.UsageAppNameReconnectingPty: "reconnecting_pty",
codersdk.UsageAppNameSSH: "ssh",
} {
require.Equal(t, want, codersdk.NormalizeAppName(string(sdkName)))
}
}
func TestAppNameFamily(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
input string
want codersdk.AppFamilyName
}{
{"VSCode", "vscode", codersdk.AppFamilyVSCode},
{"VSCodeFork", "cursor", codersdk.AppFamilyVSCode},
{"VSCodeInsidersHyphenated", "vscode-insiders", codersdk.AppFamilyVSCode},
{"CaseInsensitive", "JetBrains", codersdk.AppFamilyJetBrains},
{"SSHClientJoinsSSHFamily", "zed", codersdk.AppFamilySSH},
{"Alias", "reconnecting-pty", codersdk.AppFamilyReconnectingPTY},
{"Unknown", "SomeFutureIDE", codersdk.AppFamilyUnknown},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tc.want, codersdk.AppNameFamily(tc.input))
})
}
}