feat: emit tool-call duration histogram metric

README.md

@@ -21,8 +21,6 @@ tools by default; write and DDL tools opt in via env var.
 
 Requires Go 1.25+ and a reachable CockroachDB cluster.
 
-### `go install`
-
 ```bash
 go install github.com/cockroachdb/cockroachdb-mcp-server@latest
 ```
@@ -197,6 +195,32 @@ Precedence for picking the value:
 | `CRDB_MCP_LOG_LEVEL` | `debug`, `info`, `warn`, `error` | `info` |
 | `CRDB_MCP_LOG_PATH` | Log file path, or `-` for stderr. No rotation; use logrotate or your orchestrator | - |
 
+### Tracing (OpenTelemetry)
+
+Tracing is opt-in: with neither variable below set, no exporter is installed.
+When enabled, tool calls and their SQL statements are exported as spans,
+tool-call latency as a histogram metric (`mcp.tool.call.duration`), and server
+logs as OTel log records. Query text and errors are redacted before export so
+literals and user data never leave the server.
+
+| Variable | Purpose | Default |
+| --- | --- | --- |
+| `OTEL_EXPORTER_OTLP_ENDPOINT` | OTLP gRPC endpoint; the standard `OTEL_*` env vars are honored | - |
+| `CRDB_MCP_OTEL_FILE` | Write traces and logs as JSON lines to this file instead (takes precedence) | - |
+
+In stdio mode, add the variable to the `env` block of your
+[MCP client config](#setup---mcp-client-config); in HTTP mode, export it in
+the server's environment:
+
+```json
+{
+  "env": {
+    "CRDB_DATABASE_URL": "postgresql://...",
+    "OTEL_EXPORTER_OTLP_ENDPOINT": "http://localhost:4317"
+  }
+}
+```
+
 ## Tools
 
 Grant the connecting SQL role only the privileges the registered tools need.

config/config.go

@@ -38,6 +38,8 @@ const (
 	envTxnQoS             = "CRDB_MCP_TXN_QOS"
 	envLogLevel           = "CRDB_MCP_LOG_LEVEL"
 	envLogPath            = "CRDB_MCP_LOG_PATH"
+	envOTelFile           = "CRDB_MCP_OTEL_FILE"
+	envOTLPEndpoint       = "OTEL_EXPORTER_OTLP_ENDPOINT"
 
 	defaultPort               = 26257
 	defaultSSLMode            = "verify-full"
@@ -91,6 +93,8 @@ type Config struct {
 	// AllowNoBearer lets HTTP mode start without a bearer token. Auth must
 	// then be provided upstream (reverse proxy, gateway, mTLS).
 	AllowNoBearer bool
+	OTelFile      string
+	OTLPEndpoint  string
 }
 
 // Load reads configuration from environment variables.
@@ -112,6 +116,8 @@ func Load() (*Config, error) {
 		TLSKey:         os.Getenv(envTLSKey),
 		LogLevel:       defaultLogLevel,
 		LogPath:        os.Getenv(envLogPath),
+		OTelFile:       os.Getenv(envOTelFile),
+		OTLPEndpoint:   os.Getenv(envOTLPEndpoint),
 	}
 	if raw := os.Getenv(envTransport); raw != "" {
 		cfg.Transport = raw
@@ -354,6 +360,11 @@ func (c *Config) TLSEnabled() bool {
 	return c.TLSCert != "" && c.TLSKey != ""
 }
 
+// OTelEnabled reports whether an OpenTelemetry exporter is configured.
+func (c *Config) OTelEnabled() bool {
+	return c.OTelFile != "" || c.OTLPEndpoint != ""
+}
+
 // validateHTTPTLS enforces the SECSERV-422 default-secure policy: HTTP mode
 // must serve TLS unless the operator explicitly opts into cleartext via
 // CRDB_MCP_ALLOW_INSECURE_HTTP=true. Cert and key are required together, and

config/config_test.go

@@ -37,6 +37,27 @@ func TestLoad(t *testing.T) {
 		require.Empty(t, cfg.TxnQoS, "txn qos is empty unless env var is explicit; adapter applies the fallback")
 		require.Equal(t, defaultLogLevel, cfg.LogLevel)
 		require.Empty(t, cfg.LogPath, "log path defaults to empty (stderr)")
+		require.Empty(t, cfg.OTelFile, "otel file is opt-in")
+		require.Empty(t, cfg.OTLPEndpoint, "otlp endpoint is opt-in")
+		require.False(t, cfg.OTelEnabled(), "OTel is off when neither env var is set")
+	})
+
+	t.Run("otel file path is captured", func(t *testing.T) {
+		env := mergeEnv(baseEnv, map[string]string{envOTelFile: "/var/log/otel.jsonl"})
+		setEnv(t, env)
+		cfg, err := Load()
+		require.NoError(t, err)
+		require.Equal(t, "/var/log/otel.jsonl", cfg.OTelFile)
+		require.True(t, cfg.OTelEnabled())
+	})
+
+	t.Run("otlp endpoint is captured", func(t *testing.T) {
+		env := mergeEnv(baseEnv, map[string]string{envOTLPEndpoint: "otel-collector:4317"})
+		setEnv(t, env)
+		cfg, err := Load()
+		require.NoError(t, err)
+		require.Equal(t, "otel-collector:4317", cfg.OTLPEndpoint)
+		require.True(t, cfg.OTelEnabled())
 	})
 
 	t.Run("log path is captured and validated as writable", func(t *testing.T) {
@@ -411,7 +432,7 @@ func clearEnv(t *testing.T) {
 		envCAPath, envCertFile, envKeyFile, envEnableWriteQueries, envQueryTimeout,
 		envMaxRowsCount, envTransport, envHTTPListenAddr, envBearerToken,
 		envTLSCert, envTLSKey, envAllowInsecureHTTP, envAllowPasswordAuth,
-		envLogLevel, envLogPath,
+		envLogLevel, envLogPath, envOTelFile, envOTLPEndpoint,
 	} {
 		t.Setenv(k, "")
 	}

db/adapter.go

@@ -2,13 +2,28 @@ package db
 
 import (
 	"context"
+	"strings"
 	"time"
 
+	mcpotel "github.com/cockroachdb/cockroachdb-mcp-server/otel"
+	crdbparser "github.com/cockroachdb/cockroachdb-parser/pkg/sql/parser"
+	"github.com/cockroachdb/cockroachdb-parser/pkg/sql/sem/tree"
 	"github.com/cockroachdb/errors"
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgxpool"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
+	semconv "go.opentelemetry.io/otel/semconv/v1.30.0"
+	"go.opentelemetry.io/otel/trace"
 )
 
+const redactedSQLFallback = "<redacted: unparseable SQL>"
+
+// tracerScope names the instrumentation scope for spans created by this
+// package.
+const tracerScope = "github.com/cockroachdb/cockroachdb-mcp-server/db"
+
 const (
 	defaultApplicationName = "cockroachdb-mcp-server"
 	// defaultTxnQoS is applied when neither cfg.TxnQoS nor the DSN specifies
@@ -134,10 +149,12 @@ func (a *Adapter) Close() {
 }
 
 // Query executes a query and returns columns and rows.
-func (a *Adapter) Query(ctx context.Context, sql string) (*QueryResult, error) {
+func (a *Adapter) Query(ctx context.Context, sql string) (_ *QueryResult, err error) {
 	if sql == "" {
 		return nil, errors.New("SQL statement cannot be empty")
 	}
+	ctx, span := startSQLSpan(ctx, sql)
+	defer func() { endSpan(span, err) }()
 	if a.queryTimeout > 0 {
 		var cancel context.CancelFunc
 		ctx, cancel = context.WithTimeout(ctx, a.queryTimeout)
@@ -148,15 +165,22 @@ func (a *Adapter) Query(ctx context.Context, sql string) (*QueryResult, error) {
 		return nil, errors.Wrap(err, "exec query")
 	}
 	defer rows.Close()
-	return scanRows(rows)
+	result, err := scanRows(rows)
+	if err != nil {
+		return nil, err
+	}
+	span.SetAttributes(semconv.DBResponseReturnedRows(len(result.Rows)))
+	return result, nil
 }
 
 // Exec runs a non-result-returning statement (DDL/DML) and returns the
 // number of rows affected.
-func (a *Adapter) Exec(ctx context.Context, sql string) (int64, error) {
+func (a *Adapter) Exec(ctx context.Context, sql string) (_ int64, err error) {
 	if sql == "" {
 		return 0, errors.New("SQL statement cannot be empty")
 	}
+	ctx, span := startSQLSpan(ctx, sql)
+	defer func() { endSpan(span, err) }()
 	if a.queryTimeout > 0 {
 		var cancel context.CancelFunc
 		ctx, cancel = context.WithTimeout(ctx, a.queryTimeout)
@@ -166,9 +190,71 @@ func (a *Adapter) Exec(ctx context.Context, sql string) (int64, error) {
 	if err != nil {
 		return 0, errors.Wrap(err, "exec statement")
 	}
+	span.SetAttributes(attribute.Int64("db.response.affected_rows", tag.RowsAffected()))
 	return tag.RowsAffected(), nil
 }
 
+// startSQLSpan is a no-op until otel.Setup installs an exporter. When no
+// exporter is configured, redactSQL is not called so the parser does not run
+// on the hot path.
+func startSQLSpan(ctx context.Context, sql string) (context.Context, trace.Span) {
+	op := sqlOperation(sql)
+	name := "sql.statement"
+	if op != "" {
+		name = "sql." + op
+	}
+	ctx, span := otel.Tracer(tracerScope).Start(ctx, name,
+		trace.WithSpanKind(trace.SpanKindClient))
+	if !span.IsRecording() {
+		return ctx, span
+	}
+	attrs := []attribute.KeyValue{
+		semconv.DBSystemNameCockroachdb,
+		semconv.DBQueryText(redactSQL(sql)),
+	}
+	if op != "" {
+		attrs = append(attrs, semconv.DBOperationName(op))
+	}
+	span.SetAttributes(attrs...)
+	return ctx, span
+}
+
+// redactSQL parses sql and re-formats it with table/column names anonymized
+// and constants hidden so no user data lands in span attributes.
+func redactSQL(sql string) string {
+	stmts, err := crdbparser.Parse(sql)
+	if err != nil {
+		return redactedSQLFallback
+	}
+	return stmts.StringWithFlags(tree.FmtAnonymize | tree.FmtHideConstants)
+}
+
+func sqlOperation(sql string) string {
+	fs := strings.Fields(sql)
+	if len(fs) == 0 {
+		return ""
+	}
+	op := strings.ToUpper(fs[0])
+	for _, r := range op {
+		if r < 'A' || r > 'Z' {
+			return ""
+		}
+	}
+	return op
+}
+
+// endSpan must run inside a closure so it captures err at defer-time:
+//
+//	defer func() { endSpan(span, retErr) }()
+func endSpan(span trace.Span, err error) {
+	if err != nil {
+		safe := mcpotel.SafeSpanError(err)
+		span.RecordError(safe)
+		span.SetStatus(codes.Error, safe.Error())
+	}
+	span.End()
+}
+
 func scanRows(rows pgx.Rows) (*QueryResult, error) {
 	fds := rows.FieldDescriptions()
 	columns := make([]string, len(fds))

db/adapter_test.go

@@ -120,6 +120,63 @@ func TestBuildPoolConfig(t *testing.T) {
 	})
 }
 
+func TestRedactSQL(t *testing.T) {
+	cases := []struct {
+		name, sql, want string
+	}{
+		{
+			"SELECT hides literals and identifiers",
+			"SELECT * FROM system.descriptor WHERE id = 1",
+			"SELECT * FROM _._ WHERE _ = _",
+		},
+		{
+			"literal PII in WHERE is scrubbed",
+			"SELECT email FROM users WHERE email = 'alice@example.com'",
+			"SELECT _ FROM _ WHERE _ = '_'",
+		},
+		{
+			"INSERT VALUES payload is scrubbed",
+			"INSERT INTO mcp_smoke.trace_notes (id, body) VALUES (1, 'reconnect ok')",
+			"INSERT INTO _._(_, _) VALUES (_, '_')",
+		},
+		{
+			"CREATE TABLE hides names but preserves types",
+			"CREATE TABLE t (id INT PRIMARY KEY, name STRING)",
+			"CREATE TABLE _ (_ INT8 PRIMARY KEY, _ STRING)",
+		},
+		{
+			"unparseable SQL returns fallback without leaking source",
+			"not sql at all",
+			redactedSQLFallback,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.want, redactSQL(tc.sql))
+		})
+	}
+}
+
+func TestSQLOperation(t *testing.T) {
+	cases := []struct {
+		name, sql, want string
+	}{
+		{"select lowercased", "select 1", "SELECT"},
+		{"select uppercased", "SELECT 1", "SELECT"},
+		{"leading whitespace", "  \n\tSELECT 1", "SELECT"},
+		{"multiword DML", "INSERT INTO t VALUES (1)", "INSERT"},
+		{"comment-prefixed SQL falls back to empty so span name becomes sql.statement", "-- hello\nSELECT 1", ""},
+		{"block-comment-prefixed SQL falls back to empty", "/* c */ SELECT 1", ""},
+		{"empty string", "", ""},
+		{"whitespace only", "   \n\t", ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.want, sqlOperation(tc.sql))
+		})
+	}
+}
+
 // clearPGEnv unsets libpq fallback env vars so a developer's shell
 // (e.g. an exported PGPASSWORD) cannot leak into the test connection
 // and mask or flip the password-auth assertions.