From 7ea80d04741180cab3f208c54a6e8ebe067ef387 Mon Sep 17 00:00:00 2001 From: Marcus Vorwaller Date: Fri, 20 Mar 2026 03:24:12 -0700 Subject: [PATCH] feat: add metrics-coverage analyzer for instrumentation gap detection Adds a `nightshift metrics-coverage` command that scans Go source files using go/ast to detect metrics instrumentation patterns (Prometheus, OpenTelemetry, StatsD, expvar, custom counters). Computes per-package coverage ratios, identifies uninstrumented HTTP handlers and error paths, and assigns risk levels. Results render as markdown or JSON, with optional SQLite persistence via migration v6. Nightshift-Task: metrics-coverage Nightshift-Ref: https://github.com/marcus/nightshift Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/nightshift/commands/metrics_coverage.go | 152 ++++++ internal/analysis/metrics_coverage.go | 381 ++++++++++++++ internal/analysis/metrics_coverage_db.go | 173 ++++++ internal/analysis/metrics_coverage_report.go | 177 +++++++ internal/analysis/metrics_coverage_test.go | 521 +++++++++++++++++++ internal/db/migrations.go | 20 + 6 files changed, 1424 insertions(+) create mode 100644 cmd/nightshift/commands/metrics_coverage.go create mode 100644 internal/analysis/metrics_coverage.go create mode 100644 internal/analysis/metrics_coverage_db.go create mode 100644 internal/analysis/metrics_coverage_report.go create mode 100644 internal/analysis/metrics_coverage_test.go diff --git a/cmd/nightshift/commands/metrics_coverage.go b/cmd/nightshift/commands/metrics_coverage.go new file mode 100644 index 0000000..9273c71 --- /dev/null +++ b/cmd/nightshift/commands/metrics_coverage.go @@ -0,0 +1,152 @@ +package commands + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/spf13/cobra" + + "github.com/marcus/nightshift/internal/analysis" + "github.com/marcus/nightshift/internal/config" + "github.com/marcus/nightshift/internal/db" + "github.com/marcus/nightshift/internal/logging" +) + +var metricsCoverageCmd = &cobra.Command{ + Use: "metrics-coverage [path]", + Short: "Analyze metrics instrumentation coverage in Go source code", + Long: `Analyze metrics instrumentation coverage across Go packages. + +Scans Go source files using go/ast to detect metrics instrumentation patterns +including Prometheus, OpenTelemetry, StatsD, expvar, and custom metric types. + +For each package, computes the ratio of instrumented exported functions to total +exported functions and identifies gaps (uninstrumented HTTP handlers, error paths, +and public API functions). + +Coverage levels: + - Low risk (>=80%): Well-instrumented codebase + - Medium risk (50-80%): Moderate coverage + - High risk (20-50%): Significant gaps + - Critical (<20%): Minimal instrumentation`, + RunE: func(cmd *cobra.Command, args []string) error { + path, err := cmd.Flags().GetString("path") + if err != nil { + return err + } + + if path == "" && len(args) > 0 { + path = args[0] + } + if path == "" { + path, err = os.Getwd() + if err != nil { + return fmt.Errorf("getting current directory: %w", err) + } + } + + jsonOutput, _ := cmd.Flags().GetBool("json") + saveReport, _ := cmd.Flags().GetBool("save") + dbPath, _ := cmd.Flags().GetString("db") + + return runMetricsCoverage(path, jsonOutput, saveReport, dbPath) + }, +} + +func init() { + metricsCoverageCmd.Flags().StringP("path", "p", "", "Directory path to scan") + metricsCoverageCmd.Flags().Bool("json", false, "Output as JSON") + metricsCoverageCmd.Flags().Bool("save", false, "Save results to database") + metricsCoverageCmd.Flags().String("db", "", "Database path (uses config if not set)") + rootCmd.AddCommand(metricsCoverageCmd) +} + +func runMetricsCoverage(path string, jsonOutput bool, saveReport bool, dbPath string) error { + logger := logging.Component("metrics-coverage") + + absPath, err := filepath.Abs(path) + if err != nil { + return fmt.Errorf("resolving path: %w", err) + } + + // Verify directory exists + info, err := os.Stat(absPath) + if err != nil { + return fmt.Errorf("accessing path: %w", err) + } + if !info.IsDir() { + return fmt.Errorf("not a directory: %s", absPath) + } + + // Scan packages + scanner := analysis.NewMetricsCoverageScanner(absPath) + packages, err := scanner.Scan() + if err != nil { + return fmt.Errorf("scanning packages: %w", err) + } + + if len(packages) == 0 { + logger.Warnf("no Go packages with exported functions found in %s", absPath) + return nil + } + + // Generate report + gen := analysis.NewMetricsCoverageReportGenerator() + component := filepath.Base(absPath) + report := gen.GenerateCoverageReport(component, packages) + + // Output results + if jsonOutput { + return outputMetricsCoverageJSON(report) + } + + markdown := gen.RenderCoverageMarkdown(report) + fmt.Println(markdown) + + // Save if requested + if saveReport { + if dbPath == "" { + cfg, err := config.Load() + if err != nil { + logger.Warnf("could not load config for db path: %v", err) + } else { + dbPath = cfg.ExpandedDBPath() + } + } + + if dbPath != "" { + database, err := db.Open(dbPath) + if err != nil { + logger.Errorf("opening database: %v", err) + } else { + defer func() { _ = database.Close() }() + + result := &analysis.MetricsCoverageResult{ + Component: component, + Timestamp: time.Now(), + Summary: report.Summary, + Packages: report.Packages, + CoveragePct: report.Summary.OverallCoveragePct, + GapCount: report.Summary.GapCount, + } + + if err := result.Store(database.SQL()); err != nil { + logger.Errorf("storing result: %v", err) + } else { + logger.Infof("results saved (ID: %d)", result.ID) + } + } + } + } + + return nil +} + +func outputMetricsCoverageJSON(report *analysis.MetricsCoverageReport) error { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + return enc.Encode(report) +} diff --git a/internal/analysis/metrics_coverage.go b/internal/analysis/metrics_coverage.go new file mode 100644 index 0000000..92289d8 --- /dev/null +++ b/internal/analysis/metrics_coverage.go @@ -0,0 +1,381 @@ +package analysis + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" +) + +// MetricPattern defines a pattern that indicates metrics instrumentation. +type MetricPattern struct { + Package string // import path substring to match + Keywords []string // identifiers/selectors that indicate instrumentation +} + +// knownMetricPatterns are recognized instrumentation patterns. +var knownMetricPatterns = []MetricPattern{ + {Package: "prometheus", Keywords: []string{"Counter", "Gauge", "Histogram", "Summary", "NewCounter", "NewGauge", "NewHistogram", "NewSummary", "CounterVec", "GaugeVec", "HistogramVec", "SummaryVec", "Inc", "Add", "Set", "Observe"}}, + {Package: "otel", Keywords: []string{"Meter", "Counter", "Histogram", "UpDownCounter", "NewMeter", "Float64Counter", "Int64Counter", "Float64Histogram", "Int64Histogram"}}, + {Package: "statsd", Keywords: []string{"Count", "Gauge", "Histogram", "Timing", "Incr", "Decr", "Distribution"}}, + {Package: "datadog", Keywords: []string{"Count", "Gauge", "Histogram", "Timing", "Incr", "Distribution"}}, + {Package: "expvar", Keywords: []string{"NewInt", "NewFloat", "NewMap", "NewString", "Int", "Float", "Map", "Add", "Set"}}, +} + +// structuralKeywords are identifier substrings that indicate metrics instrumentation +// even without a known import package (custom metric types, structured logging counters). +var structuralKeywords = []string{ + "counter", "gauge", "histogram", "metric", "metrics", + "measure", "instrument", "observe", "record", + "statsd", "prometheus", "telemetry", +} + +// PackageCoverage holds metrics instrumentation coverage for a single Go package. +type PackageCoverage struct { + Path string `json:"path"` + TotalExported int `json:"total_exported"` + InstrumentedCount int `json:"instrumented_count"` + CoveragePct float64 `json:"coverage_pct"` + DetectedMetricTypes []string `json:"detected_metric_types,omitempty"` + Gaps []string `json:"gaps,omitempty"` +} + +// CoverageSummary holds the overall summary of metrics instrumentation coverage. +type CoverageSummary struct { + OverallCoveragePct float64 `json:"overall_coverage_pct"` + TotalPackages int `json:"total_packages"` + TotalExported int `json:"total_exported"` + TotalInstrumented int `json:"total_instrumented"` + GapCount int `json:"gap_count"` + RiskLevel string `json:"risk_level"` +} + +// MetricsCoverageScanner walks Go source files and detects metrics instrumentation. +type MetricsCoverageScanner struct { + root string +} + +// NewMetricsCoverageScanner creates a scanner rooted at the given directory. +func NewMetricsCoverageScanner(root string) *MetricsCoverageScanner { + return &MetricsCoverageScanner{root: root} +} + +// Scan walks all Go packages under root and computes coverage. +func (s *MetricsCoverageScanner) Scan() ([]PackageCoverage, error) { + pkgDirs, err := s.findGoPackages() + if err != nil { + return nil, err + } + + var results []PackageCoverage + for _, dir := range pkgDirs { + cov, err := s.scanPackage(dir) + if err != nil { + continue // skip packages that fail to parse + } + if cov.TotalExported == 0 { + continue // skip packages with no exported functions + } + results = append(results, *cov) + } + + return results, nil +} + +// findGoPackages discovers directories containing .go files under root. +func (s *MetricsCoverageScanner) findGoPackages() ([]string, error) { + seen := make(map[string]bool) + err := filepath.Walk(s.root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil // skip inaccessible paths + } + if info.IsDir() { + base := info.Name() + if base == "vendor" || base == "testdata" || base == ".git" || base == "node_modules" { + return filepath.SkipDir + } + return nil + } + if strings.HasSuffix(path, ".go") && !strings.HasSuffix(path, "_test.go") { + dir := filepath.Dir(path) + seen[dir] = true + } + return nil + }) + if err != nil { + return nil, err + } + + dirs := make([]string, 0, len(seen)) + for d := range seen { + dirs = append(dirs, d) + } + return dirs, nil +} + +// scanPackage analyzes a single Go package directory. +func (s *MetricsCoverageScanner) scanPackage(dir string) (*PackageCoverage, error) { + fset := token.NewFileSet() + pkgs, err := parser.ParseDir(fset, dir, func(fi os.FileInfo) bool { + return !strings.HasSuffix(fi.Name(), "_test.go") + }, parser.ParseComments) + if err != nil { + return nil, err + } + + relPath, _ := filepath.Rel(s.root, dir) + if relPath == "" || relPath == "." { + relPath = filepath.Base(dir) + } + + cov := &PackageCoverage{Path: relPath} + metricTypes := make(map[string]bool) + + for _, pkg := range pkgs { + // Collect import paths across all files in the package + importPaths := make(map[string]bool) + for _, file := range pkg.Files { + for _, imp := range file.Imports { + p := strings.Trim(imp.Path.Value, `"`) + importPaths[p] = true + } + } + + // Detect which metric frameworks are imported + detectedPatterns := s.detectImportedPatterns(importPaths) + + for _, file := range pkg.Files { + s.analyzeFile(file, detectedPatterns, cov, metricTypes) + } + } + + for t := range metricTypes { + cov.DetectedMetricTypes = append(cov.DetectedMetricTypes, t) + } + + if cov.TotalExported > 0 { + cov.CoveragePct = float64(cov.InstrumentedCount) * 100.0 / float64(cov.TotalExported) + } + + return cov, nil +} + +// detectImportedPatterns returns the metric patterns whose packages are imported. +func (s *MetricsCoverageScanner) detectImportedPatterns(imports map[string]bool) []MetricPattern { + var matched []MetricPattern + for _, pattern := range knownMetricPatterns { + for imp := range imports { + if strings.Contains(imp, pattern.Package) { + matched = append(matched, pattern) + break + } + } + } + return matched +} + +// analyzeFile walks a parsed Go file and checks each exported function for instrumentation. +func (s *MetricsCoverageScanner) analyzeFile(file *ast.File, patterns []MetricPattern, cov *PackageCoverage, metricTypes map[string]bool) { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name == nil || !fn.Name.IsExported() { + continue + } + + cov.TotalExported++ + + isHandler := isHTTPHandler(fn) + hasInstrumentation := s.functionHasInstrumentation(fn, patterns, metricTypes) + + if hasInstrumentation { + cov.InstrumentedCount++ + } else { + gap := fn.Name.Name + if isHandler { + gap += " (HTTP handler)" + } + if hasErrorReturn(fn) { + gap += " (has error return)" + } + cov.Gaps = append(cov.Gaps, gap) + } + } +} + +// functionHasInstrumentation checks whether a function body contains metric calls. +func (s *MetricsCoverageScanner) functionHasInstrumentation(fn *ast.FuncDecl, patterns []MetricPattern, metricTypes map[string]bool) bool { + if fn.Body == nil { + return false + } + + found := false + ast.Inspect(fn.Body, func(n ast.Node) bool { + if found { + return false + } + + switch node := n.(type) { + case *ast.CallExpr: + if s.isMetricCall(node, patterns, metricTypes) { + found = true + return false + } + case *ast.Ident: + if s.isStructuralMetricIdent(node.Name) { + found = true + return false + } + } + return true + }) + + return found +} + +// isMetricCall checks if a call expression matches known metric patterns. +func (s *MetricsCoverageScanner) isMetricCall(call *ast.CallExpr, patterns []MetricPattern, metricTypes map[string]bool) bool { + name := callExprName(call) + if name == "" { + return false + } + + // Check against imported metric patterns + for _, pattern := range patterns { + for _, kw := range pattern.Keywords { + if strings.Contains(name, kw) { + metricTypes[pattern.Package] = true + return true + } + } + } + + // Check structural keywords in call names + lower := strings.ToLower(name) + for _, kw := range structuralKeywords { + if strings.Contains(lower, kw) { + metricTypes["custom"] = true + return true + } + } + + return false +} + +// isStructuralMetricIdent checks if an identifier name suggests metric usage. +func (s *MetricsCoverageScanner) isStructuralMetricIdent(name string) bool { + lower := strings.ToLower(name) + for _, kw := range structuralKeywords { + if strings.Contains(lower, kw) { + return true + } + } + return false +} + +// callExprName extracts a readable name from a call expression. +func callExprName(call *ast.CallExpr) string { + switch fn := call.Fun.(type) { + case *ast.Ident: + return fn.Name + case *ast.SelectorExpr: + if ident, ok := fn.X.(*ast.Ident); ok { + return ident.Name + "." + fn.Sel.Name + } + return fn.Sel.Name + } + return "" +} + +// isHTTPHandler checks if a function signature matches http.Handler/HandlerFunc patterns. +func isHTTPHandler(fn *ast.FuncDecl) bool { + if fn.Type == nil || fn.Type.Params == nil { + return false + } + params := fn.Type.Params.List + if len(params) < 2 { + return false + } + + // Check for (http.ResponseWriter, *http.Request) pattern + hasWriter := false + hasRequest := false + for _, param := range params { + typeName := typeString(param.Type) + if strings.Contains(typeName, "ResponseWriter") { + hasWriter = true + } + if strings.Contains(typeName, "Request") { + hasRequest = true + } + } + return hasWriter && hasRequest +} + +// hasErrorReturn checks if the function returns an error type. +func hasErrorReturn(fn *ast.FuncDecl) bool { + if fn.Type == nil || fn.Type.Results == nil { + return false + } + for _, field := range fn.Type.Results.List { + if typeString(field.Type) == "error" { + return true + } + } + return false +} + +// typeString returns a simplified string representation of a type expression. +func typeString(expr ast.Expr) string { + switch t := expr.(type) { + case *ast.Ident: + return t.Name + case *ast.SelectorExpr: + if ident, ok := t.X.(*ast.Ident); ok { + return ident.Name + "." + t.Sel.Name + } + return t.Sel.Name + case *ast.StarExpr: + return typeString(t.X) + case *ast.ArrayType: + return "[]" + typeString(t.Elt) + } + return "" +} + +// ComputeSummary computes an overall CoverageSummary from per-package results. +func ComputeSummary(packages []PackageCoverage) *CoverageSummary { + summary := &CoverageSummary{ + TotalPackages: len(packages), + } + + for _, pkg := range packages { + summary.TotalExported += pkg.TotalExported + summary.TotalInstrumented += pkg.InstrumentedCount + summary.GapCount += len(pkg.Gaps) + } + + if summary.TotalExported > 0 { + summary.OverallCoveragePct = float64(summary.TotalInstrumented) * 100.0 / float64(summary.TotalExported) + } + + summary.RiskLevel = assessCoverageRisk(summary.OverallCoveragePct, summary.GapCount) + return summary +} + +// assessCoverageRisk determines the risk level based on coverage percentage and gap count. +func assessCoverageRisk(coveragePct float64, gapCount int) string { + switch { + case coveragePct >= 80 && gapCount <= 5: + return "low" + case coveragePct >= 80: + return "medium" + case coveragePct >= 50: + return "medium" + case coveragePct >= 20: + return "high" + default: + return "critical" + } +} diff --git a/internal/analysis/metrics_coverage_db.go b/internal/analysis/metrics_coverage_db.go new file mode 100644 index 0000000..b65be1f --- /dev/null +++ b/internal/analysis/metrics_coverage_db.go @@ -0,0 +1,173 @@ +package analysis + +import ( + "database/sql" + "encoding/json" + "fmt" + "time" +) + +// MetricsCoverageResult represents a stored metrics coverage analysis result. +type MetricsCoverageResult struct { + ID int64 `json:"id"` + Component string `json:"component"` + Timestamp time.Time `json:"timestamp"` + Summary *CoverageSummary `json:"summary"` + Packages []PackageCoverage `json:"packages"` + CoveragePct float64 `json:"coverage_pct"` + GapCount int `json:"gap_count"` + ReportPath string `json:"report_path,omitempty"` +} + +// Store saves a metrics coverage result to the database. +func (r *MetricsCoverageResult) Store(db *sql.DB) error { + if db == nil { + return fmt.Errorf("database is nil") + } + + summaryJSON, err := json.Marshal(r.Summary) + if err != nil { + return fmt.Errorf("marshaling summary: %w", err) + } + + packagesJSON, err := json.Marshal(r.Packages) + if err != nil { + return fmt.Errorf("marshaling packages: %w", err) + } + + query := ` + INSERT INTO metrics_coverage_results (component, timestamp, summary, packages, coverage_pct, gap_count, report_path) + VALUES (?, ?, ?, ?, ?, ?, ?) + ` + + res, err := db.Exec(query, + r.Component, + r.Timestamp, + string(summaryJSON), + string(packagesJSON), + r.CoveragePct, + r.GapCount, + r.ReportPath, + ) + if err != nil { + return fmt.Errorf("inserting metrics coverage result: %w", err) + } + + id, err := res.LastInsertId() + if err != nil { + return fmt.Errorf("getting insert id: %w", err) + } + r.ID = id + + return nil +} + +// LoadLatestCoverage loads the most recent metrics coverage result for a component. +func LoadLatestCoverage(db *sql.DB, component string) (*MetricsCoverageResult, error) { + if db == nil { + return nil, fmt.Errorf("database is nil") + } + + query := ` + SELECT id, component, timestamp, summary, packages, coverage_pct, gap_count, report_path + FROM metrics_coverage_results + WHERE component = ? + ORDER BY timestamp DESC + LIMIT 1 + ` + + row := db.QueryRow(query, component) + + result := &MetricsCoverageResult{} + var summaryJSON, packagesJSON string + + err := row.Scan( + &result.ID, + &result.Component, + &result.Timestamp, + &summaryJSON, + &packagesJSON, + &result.CoveragePct, + &result.GapCount, + &result.ReportPath, + ) + if err != nil { + if err == sql.ErrNoRows { + return nil, nil + } + return nil, fmt.Errorf("querying metrics coverage result: %w", err) + } + + if err := json.Unmarshal([]byte(summaryJSON), &result.Summary); err != nil { + return nil, fmt.Errorf("unmarshaling summary: %w", err) + } + + if err := json.Unmarshal([]byte(packagesJSON), &result.Packages); err != nil { + return nil, fmt.Errorf("unmarshaling packages: %w", err) + } + + return result, nil +} + +// LoadAllCoverage loads all metrics coverage results for a component, optionally filtered by date. +func LoadAllCoverage(db *sql.DB, component string, since time.Time) ([]MetricsCoverageResult, error) { + if db == nil { + return nil, fmt.Errorf("database is nil") + } + + query := ` + SELECT id, component, timestamp, summary, packages, coverage_pct, gap_count, report_path + FROM metrics_coverage_results + WHERE component = ? + ` + args := []any{component} + + if !since.IsZero() { + query += " AND timestamp >= ?" + args = append(args, since) + } + + query += " ORDER BY timestamp DESC" + + rows, err := db.Query(query, args...) + if err != nil { + return nil, fmt.Errorf("querying metrics coverage results: %w", err) + } + defer func() { _ = rows.Close() }() + + var results []MetricsCoverageResult + for rows.Next() { + result := MetricsCoverageResult{} + var summaryJSON, packagesJSON string + + err := rows.Scan( + &result.ID, + &result.Component, + &result.Timestamp, + &summaryJSON, + &packagesJSON, + &result.CoveragePct, + &result.GapCount, + &result.ReportPath, + ) + if err != nil { + return nil, fmt.Errorf("scanning metrics coverage result: %w", err) + } + + if err := json.Unmarshal([]byte(summaryJSON), &result.Summary); err != nil { + return nil, fmt.Errorf("unmarshaling summary: %w", err) + } + + if err := json.Unmarshal([]byte(packagesJSON), &result.Packages); err != nil { + return nil, fmt.Errorf("unmarshaling packages: %w", err) + } + + results = append(results, result) + } + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterating metrics coverage results: %w", err) + } + + return results, nil +} diff --git a/internal/analysis/metrics_coverage_report.go b/internal/analysis/metrics_coverage_report.go new file mode 100644 index 0000000..bd2870a --- /dev/null +++ b/internal/analysis/metrics_coverage_report.go @@ -0,0 +1,177 @@ +package analysis + +import ( + "bytes" + "fmt" + "time" +) + +// MetricsCoverageReport represents a complete metrics instrumentation coverage report. +type MetricsCoverageReport struct { + Timestamp time.Time `json:"timestamp"` + Component string `json:"component"` + Summary *CoverageSummary `json:"summary"` + Packages []PackageCoverage `json:"packages"` + Recommendations []string `json:"recommendations"` + ReportedAt string `json:"reported_at"` +} + +// MetricsCoverageReportGenerator creates formatted reports from coverage scan results. +type MetricsCoverageReportGenerator struct{} + +// NewMetricsCoverageReportGenerator creates a new report generator. +func NewMetricsCoverageReportGenerator() *MetricsCoverageReportGenerator { + return &MetricsCoverageReportGenerator{} +} + +// GenerateCoverageReport creates a report from scanned packages. +func (g *MetricsCoverageReportGenerator) GenerateCoverageReport(component string, packages []PackageCoverage) *MetricsCoverageReport { + summary := ComputeSummary(packages) + + return &MetricsCoverageReport{ + Timestamp: time.Now(), + Component: component, + Summary: summary, + Packages: packages, + Recommendations: g.generateRecommendations(summary, packages), + ReportedAt: time.Now().Format("2006-01-02 15:04:05"), + } +} + +// generateRecommendations creates action items based on coverage. +func (g *MetricsCoverageReportGenerator) generateRecommendations(summary *CoverageSummary, packages []PackageCoverage) []string { + var recs []string + + switch summary.RiskLevel { + case "critical": + recs = append(recs, "CRITICAL: Less than 20% of exported functions have metrics instrumentation. Implement observability as a priority.") + recs = append(recs, "Start by adding metrics to HTTP handlers and public API entry points.") + case "high": + recs = append(recs, "HIGH RISK: Many exported functions lack metrics. Focus on error paths and handler functions.") + recs = append(recs, "Consider adding Prometheus counters or OpenTelemetry spans to critical paths.") + case "medium": + recs = append(recs, "MEDIUM RISK: Moderate metrics coverage. Identify high-traffic paths missing instrumentation.") + recs = append(recs, "Review gap list and prioritize HTTP handlers and error-returning functions.") + case "low": + recs = append(recs, "GOOD: Metrics coverage is healthy across the codebase.") + recs = append(recs, "Maintain current instrumentation practices and review new code for coverage.") + } + + // Find packages with zero coverage + var zeroCov []string + for _, pkg := range packages { + if pkg.InstrumentedCount == 0 && pkg.TotalExported > 0 { + zeroCov = append(zeroCov, pkg.Path) + } + } + if len(zeroCov) > 0 { + if len(zeroCov) <= 5 { + recs = append(recs, fmt.Sprintf("Packages with zero metrics coverage: %v", zeroCov)) + } else { + recs = append(recs, fmt.Sprintf("%d packages have zero metrics coverage. Prioritize those with HTTP handlers.", len(zeroCov))) + } + } + + // Count handler gaps + handlerGaps := 0 + for _, pkg := range packages { + for _, gap := range pkg.Gaps { + if contains(gap, "HTTP handler") { + handlerGaps++ + } + } + } + if handlerGaps > 0 { + recs = append(recs, fmt.Sprintf("%d HTTP handler(s) lack metrics instrumentation. These are high-priority gaps.", handlerGaps)) + } + + return recs +} + +// contains checks if s contains substr. +func contains(s, substr string) bool { + return len(s) >= len(substr) && bytes.Contains([]byte(s), []byte(substr)) +} + +// RenderCoverageMarkdown generates a markdown representation of the coverage report. +func (g *MetricsCoverageReportGenerator) RenderCoverageMarkdown(report *MetricsCoverageReport) string { + var buf bytes.Buffer + + // Header + fmt.Fprintf(&buf, "# Metrics Coverage Analysis - %s\n\n", report.Component) + fmt.Fprintf(&buf, "*Generated: %s*\n\n", report.ReportedAt) + + // Summary section + buf.WriteString("## Coverage Summary\n\n") + buf.WriteString("| Metric | Value |\n") + buf.WriteString("|--------|-------|\n") + fmt.Fprintf(&buf, "| Risk Level | **%s** |\n", report.Summary.RiskLevel) + fmt.Fprintf(&buf, "| Overall Coverage | %.1f%% |\n", report.Summary.OverallCoveragePct) + fmt.Fprintf(&buf, "| Total Packages | %d |\n", report.Summary.TotalPackages) + fmt.Fprintf(&buf, "| Exported Functions | %d |\n", report.Summary.TotalExported) + fmt.Fprintf(&buf, "| Instrumented | %d |\n", report.Summary.TotalInstrumented) + fmt.Fprintf(&buf, "| Gaps | %d |\n\n", report.Summary.GapCount) + + // Per-package breakdown + if len(report.Packages) > 0 { + buf.WriteString("## Package Breakdown\n\n") + buf.WriteString("| Package | Exported | Instrumented | Coverage | Metric Types |\n") + buf.WriteString("|---------|----------|-------------|----------|-------------|\n") + for _, pkg := range report.Packages { + types := "-" + if len(pkg.DetectedMetricTypes) > 0 { + types = fmt.Sprintf("%v", pkg.DetectedMetricTypes) + } + fmt.Fprintf(&buf, "| %s | %d | %d | %.1f%% | %s |\n", + pkg.Path, pkg.TotalExported, pkg.InstrumentedCount, pkg.CoveragePct, types) + } + buf.WriteString("\n") + } + + // Gaps section + totalGaps := 0 + for _, pkg := range report.Packages { + totalGaps += len(pkg.Gaps) + } + if totalGaps > 0 { + buf.WriteString("## Instrumentation Gaps\n\n") + for _, pkg := range report.Packages { + if len(pkg.Gaps) == 0 { + continue + } + fmt.Fprintf(&buf, "### %s\n\n", pkg.Path) + for _, gap := range pkg.Gaps { + fmt.Fprintf(&buf, "- `%s`\n", gap) + } + buf.WriteString("\n") + } + } + + // Recommendations + if len(report.Recommendations) > 0 { + buf.WriteString("## Recommendations\n\n") + for _, rec := range report.Recommendations { + isHighPriority := len(rec) > 0 && (rec[0] == 'G' || rec[0] == 'H' || rec[0] == 'C' || rec[0] == 'M') && + (bytes.HasPrefix([]byte(rec), []byte("GOOD")) || + bytes.HasPrefix([]byte(rec), []byte("HIGH")) || + bytes.HasPrefix([]byte(rec), []byte("CRITICAL")) || + bytes.HasPrefix([]byte(rec), []byte("MEDIUM"))) + + if isHighPriority { + fmt.Fprintf(&buf, "**%s**\n\n", rec) + } else { + fmt.Fprintf(&buf, "- %s\n", rec) + } + } + buf.WriteString("\n") + } + + // Coverage explanation + buf.WriteString("## Understanding Coverage Levels\n\n") + buf.WriteString("- **Low risk** (>=80%): Well-instrumented codebase with few gaps.\n") + buf.WriteString("- **Medium risk** (50-80%): Moderate coverage, key paths may lack metrics.\n") + buf.WriteString("- **High risk** (20-50%): Significant gaps in observability.\n") + buf.WriteString("- **Critical** (<20%): Minimal metrics instrumentation. Incidents will lack data.\n") + + return buf.String() +} diff --git a/internal/analysis/metrics_coverage_test.go b/internal/analysis/metrics_coverage_test.go new file mode 100644 index 0000000..c32f252 --- /dev/null +++ b/internal/analysis/metrics_coverage_test.go @@ -0,0 +1,521 @@ +package analysis + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// writeGoFile is a test helper that writes a Go source file and returns its path. +func writeGoFile(t *testing.T, dir, name, content string) string { + t.Helper() + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } + return path +} + +func TestScannerNoGoFiles(t *testing.T) { + dir := t.TempDir() + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 0 { + t.Errorf("expected 0 packages, got %d", len(pkgs)) + } +} + +func TestScannerNoExportedFunctions(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "internal.go", `package foo + +func doStuff() {} +func helper() int { return 1 } +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 0 { + t.Errorf("expected 0 packages (no exported funcs), got %d", len(pkgs)) + } +} + +func TestScannerUninstrumentedFunctions(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "api.go", `package api + +func GetUser(id string) (string, error) { + return "user-" + id, nil +} + +func ListItems() []string { + return nil +} + +func DeleteItem(id string) error { + return nil +} +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + + pkg := pkgs[0] + if pkg.TotalExported != 3 { + t.Errorf("expected 3 exported functions, got %d", pkg.TotalExported) + } + if pkg.InstrumentedCount != 0 { + t.Errorf("expected 0 instrumented functions, got %d", pkg.InstrumentedCount) + } + if pkg.CoveragePct != 0 { + t.Errorf("expected 0%% coverage, got %.1f%%", pkg.CoveragePct) + } + if len(pkg.Gaps) != 3 { + t.Errorf("expected 3 gaps, got %d", len(pkg.Gaps)) + } +} + +func TestScannerPrometheusInstrumented(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "handler.go", `package handler + +import "github.com/prometheus/client_golang/prometheus" + +var reqCounter = prometheus.NewCounter(prometheus.CounterOpts{ + Name: "requests_total", +}) + +func HandleRequest(w string, r string) { + reqCounter.Inc() +} + +func HealthCheck() string { + return "ok" +} +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + + pkg := pkgs[0] + if pkg.TotalExported != 2 { + t.Errorf("expected 2 exported functions, got %d", pkg.TotalExported) + } + if pkg.InstrumentedCount != 1 { + t.Errorf("expected 1 instrumented function, got %d", pkg.InstrumentedCount) + } + if pkg.CoveragePct != 50 { + t.Errorf("expected 50%% coverage, got %.1f%%", pkg.CoveragePct) + } + + // Check metric types detected + foundPrometheus := false + for _, mt := range pkg.DetectedMetricTypes { + if mt == "prometheus" { + foundPrometheus = true + } + } + if !foundPrometheus { + t.Errorf("expected prometheus in detected metric types, got %v", pkg.DetectedMetricTypes) + } +} + +func TestScannerExpvarInstrumented(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "stats.go", `package stats + +import "expvar" + +var hits = expvar.NewInt("hits") + +func RecordHit() { + hits.Add(1) +} + +func GetStats() string { + return "stats" +} +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + + pkg := pkgs[0] + if pkg.InstrumentedCount != 1 { + t.Errorf("expected 1 instrumented function, got %d", pkg.InstrumentedCount) + } +} + +func TestScannerHTTPHandlerGap(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "server.go", `package server + +import "net/http" + +func ServeIndex(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("hello")) +} +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + + pkg := pkgs[0] + if len(pkg.Gaps) != 1 { + t.Fatalf("expected 1 gap, got %d", len(pkg.Gaps)) + } + if !strings.Contains(pkg.Gaps[0], "HTTP handler") { + t.Errorf("expected gap to be marked as HTTP handler, got: %s", pkg.Gaps[0]) + } +} + +func TestScannerErrorReturnGap(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "service.go", `package service + +func Process(data string) error { + return nil +} +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + + pkg := pkgs[0] + if len(pkg.Gaps) != 1 { + t.Fatalf("expected 1 gap, got %d", len(pkg.Gaps)) + } + if !strings.Contains(pkg.Gaps[0], "has error return") { + t.Errorf("expected gap to note error return, got: %s", pkg.Gaps[0]) + } +} + +func TestScannerSkipsTestFiles(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "api.go", `package api + +func GetUser() string { return "user" } +`) + writeGoFile(t, dir, "api_test.go", `package api + +func TestGetUser() {} +func BenchmarkGetUser() {} +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + if pkgs[0].TotalExported != 1 { + t.Errorf("expected 1 exported function (test funcs excluded), got %d", pkgs[0].TotalExported) + } +} + +func TestScannerCustomMetricKeywords(t *testing.T) { + dir := t.TempDir() + writeGoFile(t, dir, "app.go", `package app + +func RecordMetrics(name string) { + recordCounter(name) +} + +func recordCounter(name string) {} +`) + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + + // RecordMetrics should be detected as instrumented due to "metric" keyword + pkg := pkgs[0] + if pkg.InstrumentedCount != 1 { + t.Errorf("expected 1 instrumented function (custom keyword), got %d", pkg.InstrumentedCount) + } +} + +func TestComputeSummary(t *testing.T) { + packages := []PackageCoverage{ + {Path: "pkg/a", TotalExported: 10, InstrumentedCount: 8, CoveragePct: 80, Gaps: []string{"Gap1", "Gap2"}}, + {Path: "pkg/b", TotalExported: 5, InstrumentedCount: 1, CoveragePct: 20, Gaps: []string{"Gap3", "Gap4", "Gap5", "Gap6"}}, + } + + summary := ComputeSummary(packages) + + if summary.TotalPackages != 2 { + t.Errorf("expected 2 packages, got %d", summary.TotalPackages) + } + if summary.TotalExported != 15 { + t.Errorf("expected 15 total exported, got %d", summary.TotalExported) + } + if summary.TotalInstrumented != 9 { + t.Errorf("expected 9 instrumented, got %d", summary.TotalInstrumented) + } + if summary.GapCount != 6 { + t.Errorf("expected 6 gaps, got %d", summary.GapCount) + } + + expectedPct := 60.0 + if summary.OverallCoveragePct != expectedPct { + t.Errorf("expected %.1f%% coverage, got %.1f%%", expectedPct, summary.OverallCoveragePct) + } + if summary.RiskLevel != "medium" { + t.Errorf("expected medium risk, got %s", summary.RiskLevel) + } +} + +func TestComputeSummaryEmpty(t *testing.T) { + summary := ComputeSummary(nil) + if summary.TotalPackages != 0 { + t.Errorf("expected 0 packages, got %d", summary.TotalPackages) + } + if summary.OverallCoveragePct != 0 { + t.Errorf("expected 0%% coverage, got %.1f%%", summary.OverallCoveragePct) + } + if summary.RiskLevel != "critical" { + t.Errorf("expected critical risk for empty, got %s", summary.RiskLevel) + } +} + +func TestAssessCoverageRisk(t *testing.T) { + tests := []struct { + pct float64 + gaps int + expected string + }{ + {90, 2, "low"}, + {85, 10, "medium"}, + {60, 5, "medium"}, + {30, 20, "high"}, + {10, 50, "critical"}, + {0, 0, "critical"}, + } + + for _, tt := range tests { + got := assessCoverageRisk(tt.pct, tt.gaps) + if got != tt.expected { + t.Errorf("assessCoverageRisk(%.0f, %d) = %s, want %s", tt.pct, tt.gaps, got, tt.expected) + } + } +} + +func TestReportGeneration(t *testing.T) { + packages := []PackageCoverage{ + {Path: "cmd/app", TotalExported: 5, InstrumentedCount: 3, CoveragePct: 60, Gaps: []string{"Main", "Run"}}, + } + + gen := NewMetricsCoverageReportGenerator() + report := gen.GenerateCoverageReport("test-project", packages) + + if report.Component != "test-project" { + t.Errorf("expected component 'test-project', got %s", report.Component) + } + if report.Summary == nil { + t.Fatal("summary should not be nil") + } + if report.Timestamp.IsZero() { + t.Errorf("timestamp should not be zero") + } + if report.ReportedAt == "" { + t.Errorf("reported_at should not be empty") + } + if len(report.Recommendations) == 0 { + t.Errorf("expected recommendations to be generated") + } +} + +func TestRenderCoverageMarkdown(t *testing.T) { + packages := []PackageCoverage{ + { + Path: "internal/api", + TotalExported: 10, + InstrumentedCount: 6, + CoveragePct: 60, + DetectedMetricTypes: []string{"prometheus"}, + Gaps: []string{"GetUser (has error return)", "DeleteUser (HTTP handler)"}, + }, + } + + gen := NewMetricsCoverageReportGenerator() + report := gen.GenerateCoverageReport("my-service", packages) + markdown := gen.RenderCoverageMarkdown(report) + + checks := []struct { + label string + expected string + }{ + {"title", "# Metrics Coverage Analysis"}, + {"summary section", "## Coverage Summary"}, + {"package breakdown", "## Package Breakdown"}, + {"gaps section", "## Instrumentation Gaps"}, + {"recommendations", "## Recommendations"}, + {"risk level", "Risk Level"}, + {"package name", "internal/api"}, + {"gap entry", "GetUser"}, + {"coverage levels", "Understanding Coverage Levels"}, + } + + for _, c := range checks { + if !strings.Contains(markdown, c.expected) { + t.Errorf("markdown should contain %s (%q)", c.label, c.expected) + } + } +} + +func TestRenderCoverageMarkdownNoGaps(t *testing.T) { + packages := []PackageCoverage{ + { + Path: "pkg/util", + TotalExported: 3, + InstrumentedCount: 3, + CoveragePct: 100, + DetectedMetricTypes: []string{"expvar"}, + }, + } + + gen := NewMetricsCoverageReportGenerator() + report := gen.GenerateCoverageReport("util-lib", packages) + markdown := gen.RenderCoverageMarkdown(report) + + if strings.Contains(markdown, "## Instrumentation Gaps") { + t.Errorf("markdown should not contain gaps section when there are no gaps") + } +} + +func TestRecommendationsForZeroCoveragePackages(t *testing.T) { + packages := []PackageCoverage{ + {Path: "pkg/a", TotalExported: 5, InstrumentedCount: 0, CoveragePct: 0, Gaps: []string{"A", "B", "C", "D", "E"}}, + {Path: "pkg/b", TotalExported: 3, InstrumentedCount: 3, CoveragePct: 100}, + } + + gen := NewMetricsCoverageReportGenerator() + report := gen.GenerateCoverageReport("mixed", packages) + + foundZeroCovRec := false + for _, rec := range report.Recommendations { + if strings.Contains(rec, "zero metrics coverage") { + foundZeroCovRec = true + break + } + } + if !foundZeroCovRec { + t.Errorf("expected recommendation about zero-coverage packages") + } +} + +func TestRecommendationsForHTTPHandlerGaps(t *testing.T) { + packages := []PackageCoverage{ + {Path: "api", TotalExported: 2, InstrumentedCount: 0, CoveragePct: 0, + Gaps: []string{"ServeIndex (HTTP handler)", "HandleLogin (HTTP handler)"}}, + } + + gen := NewMetricsCoverageReportGenerator() + report := gen.GenerateCoverageReport("web-app", packages) + + foundHandlerRec := false + for _, rec := range report.Recommendations { + if strings.Contains(rec, "HTTP handler") { + foundHandlerRec = true + break + } + } + if !foundHandlerRec { + t.Errorf("expected recommendation about HTTP handler gaps") + } +} + +func TestScannerMultiplePackages(t *testing.T) { + dir := t.TempDir() + + // Create sub-packages + pkgA := filepath.Join(dir, "pkga") + pkgB := filepath.Join(dir, "pkgb") + if err := os.MkdirAll(pkgA, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(pkgB, 0755); err != nil { + t.Fatal(err) + } + + writeGoFile(t, pkgA, "a.go", `package pkga + +func DoA() string { return "a" } +`) + writeGoFile(t, pkgB, "b.go", `package pkgb + +import "expvar" + +var counter = expvar.NewInt("counter") + +func DoB() string { + counter.Add(1) + return "b" +} +`) + + scanner := NewMetricsCoverageScanner(dir) + pkgs, err := scanner.Scan() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(pkgs) != 2 { + t.Fatalf("expected 2 packages, got %d", len(pkgs)) + } + + // Find each package + var foundA, foundB bool + for _, pkg := range pkgs { + if strings.Contains(pkg.Path, "pkga") { + foundA = true + if pkg.InstrumentedCount != 0 { + t.Errorf("pkga should have 0 instrumented, got %d", pkg.InstrumentedCount) + } + } + if strings.Contains(pkg.Path, "pkgb") { + foundB = true + if pkg.InstrumentedCount != 1 { + t.Errorf("pkgb should have 1 instrumented, got %d", pkg.InstrumentedCount) + } + } + } + if !foundA || !foundB { + t.Errorf("expected both pkga and pkgb, found A=%v B=%v", foundA, foundB) + } +} diff --git a/internal/db/migrations.go b/internal/db/migrations.go index 3b7d11e..3c7939e 100644 --- a/internal/db/migrations.go +++ b/internal/db/migrations.go @@ -40,6 +40,11 @@ var migrations = []Migration{ Description: "add branch column to run_history", SQL: migration005SQL, }, + { + Version: 6, + Description: "add metrics_coverage_results table for instrumentation analysis", + SQL: migration006SQL, + }, } const migration002SQL = ` @@ -121,6 +126,21 @@ const migration005SQL = ` ALTER TABLE run_history ADD COLUMN branch TEXT NOT NULL DEFAULT ''; ` +const migration006SQL = ` +CREATE TABLE IF NOT EXISTS metrics_coverage_results ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + component TEXT NOT NULL, + timestamp DATETIME NOT NULL, + summary TEXT NOT NULL, + packages TEXT NOT NULL, + coverage_pct REAL NOT NULL, + gap_count INTEGER NOT NULL, + report_path TEXT +); + +CREATE INDEX IF NOT EXISTS idx_metrics_coverage_component_time ON metrics_coverage_results(component, timestamp DESC); +` + // Migrate runs all pending migrations inside transactions. func Migrate(db *sql.DB) error { if db == nil {