Add support for "Start" utility methods

README.md

@@ -63,6 +63,13 @@ linters-settings:
     # Default: []
     ignore-check-signatures:
       - "telemetry.RecordError"
+    # A list of regexes for additional function signatures that create spans. This is useful if you have a utility
+    # method to create spans. Each entry should be of the form <regex>:<telemetry-type>, where `telemetry-type`
+    # can be `opentelemetry` or `opencensus`.
+    # https://github.com/jjti/go-spancheck#extra-start-span-signatures
+    # Default: []
+    extra-start-span-signatures:
+      - "github.com/user/repo/telemetry/trace.Start:opentelemetry"
 ```
 
 ### CLI
@@ -123,6 +130,21 @@ The warnings are can be ignored by setting `-ignore-check-signatures` flag to `r
 spancheck -checks 'end,set-status,record-error' -ignore-check-signatures 'recordErr' ./...
 ```
 
+### Extra Start Span Signatures
+
+By default, Span creation will be tracked from calls to [(go.opentelemetry.io/otel/trace.Tracer).Start](https://github.com/open-telemetry/opentelemetry-go/blob/98b32a6c3a87fbee5d34c063b9096f416b250897/trace/trace.go#L523), [go.opencensus.io/trace.StartSpan](https://pkg.go.dev/go.opencensus.io/trace#StartSpan), or [go.opencensus.io/trace.StartSpanWithRemoteParent](https://github.com/census-instrumentation/opencensus-go/blob/v0.24.0/trace/trace_api.go#L66).
+
+You can use the `-extra-start-span-signatures` flag to list additional Span creation functions. For all such functions:
+
+1. their Spans will be linted (for all enable checks)
+1. checks will be disabled (i.e. there is no linting of Spans within the creation functions)
+
+You must pass a comma-separated list of regex patterns and the telemetry library corresponding to the returned Span. Each entry should be of the form `<regex>:<telemetry-type>`, where `telemetry-type` can be `opentelemetry` or `opencensus`. For example, if you have created a function named `StartTrace` in a `telemetry` package, using the `go.opentelemetry.io/otel` library, you can include this function for analysis like so:
+
+```bash
+spancheck -extra-start-span-signatures 'github.com/user/repo/telemetry/StartTrace:opentelemetry' ./...
+```
+
 ## Problem Statement
 
 Tracing is a celebrated [[1](https://andydote.co.uk/2023/09/19/tracing-is-better/),[2](https://charity.wtf/2022/08/15/live-your-best-life-with-structured-events/)] and well marketed [[3](https://docs.datadoghq.com/tracing/),[4](https://www.honeycomb.io/distributed-tracing)] pillar of observability. But self-instrumented tracing requires a lot of easy-to-forget boilerplate:

cmd/spancheck/main.go

@@ -23,11 +23,17 @@ func main() {
 	// Set the list of function signatures to ignore checks for.
 	ignoreCheckSignatures := ""
 	flag.StringVar(&ignoreCheckSignatures, "ignore-check-signatures", "", "comma-separated list of regex for function signatures that disable checks on errors")
+
+	extraStartSpanSignatures := ""
+	flag.StringVar(&extraStartSpanSignatures, "extra-start-span-signatures", "", "comma-separated list of regex:telemetry-type for function signatures that indicate the start of a span")
+
 	flag.Parse()
 
 	cfg := spancheck.NewDefaultConfig()
 	cfg.EnabledChecks = strings.Split(checkStrings, ",")
 	cfg.IgnoreChecksSignaturesSlice = strings.Split(ignoreCheckSignatures, ",")
 
+	cfg.StartSpanMatchersSlice = append(cfg.StartSpanMatchersSlice, strings.Split(extraStartSpanSignatures, ",")...)
+
 	singlechecker.Main(spancheck.NewAnalyzerWithConfig(cfg))
 }

config.go

@@ -22,6 +22,18 @@ const (
 	RecordErrorCheck
 )
 
+var (
+	startSpanSignatureCols     = 2
+	defaultStartSpanSignatures = []string{
+		// https://github.com/open-telemetry/opentelemetry-go/blob/98b32a6c3a87fbee5d34c063b9096f416b250897/trace/trace.go#L523
+		`\(go.opentelemetry.io/otel/trace.Tracer\).Start:opentelemetry`,
+		// https://pkg.go.dev/go.opencensus.io/trace#StartSpan
+		`go.opencensus.io/trace.StartSpan:opencensus`,
+		// https://github.com/census-instrumentation/opencensus-go/blob/v0.24.0/trace/trace_api.go#L66
+		`go.opencensus.io/trace.StartSpanWithRemoteParent:opencensus`,
+	}
+)
+
 func (c Check) String() string {
 	switch c {
 	case EndCheck:
@@ -35,14 +47,17 @@ func (c Check) String() string {
 	}
 }
 
-var (
-	// Checks is a list of all checks by name.
-	Checks = map[string]Check{
-		EndCheck.String():         EndCheck,
-		SetStatusCheck.String():   SetStatusCheck,
-		RecordErrorCheck.String(): RecordErrorCheck,
-	}
-)
+// Checks is a list of all checks by name.
+var Checks = map[string]Check{
+	EndCheck.String():         EndCheck,
+	SetStatusCheck.String():   SetStatusCheck,
+	RecordErrorCheck.String(): RecordErrorCheck,
+}
+
+type spanStartMatcher struct {
+	signature *regexp.Regexp
+	spanType  spanType
+}
 
 // Config is a configuration for the spancheck analyzer.
 type Config struct {
@@ -55,19 +70,25 @@ type Config struct {
 	// the IgnoreSetStatusCheckSignatures regex.
 	IgnoreChecksSignaturesSlice []string
 
+	StartSpanMatchersSlice []string
+
 	endCheckEnabled    bool
 	setStatusEnabled   bool
 	recordErrorEnabled bool
 
 	// ignoreChecksSignatures is a regex that, if matched, disables the
 	// SetStatus and RecordError checks on error.
 	ignoreChecksSignatures *regexp.Regexp
+
+	startSpanMatchers            []spanStartMatcher
+	startSpanMatchersCustomRegex *regexp.Regexp
 }
 
 // NewDefaultConfig returns a new Config with default values.
 func NewDefaultConfig() *Config {
 	return &Config{
-		EnabledChecks: []string{EndCheck.String()},
+		EnabledChecks:          []string{EndCheck.String()},
+		StartSpanMatchersSlice: defaultStartSpanSignatures,
 	}
 }
 
@@ -83,6 +104,11 @@ func (c *Config) finalize() {
 
 // parseSignatures sets the Ignore*CheckSignatures regex from the string slices.
 func (c *Config) parseSignatures() {
+	c.parseIgnoreSignatures()
+	c.parseStartSpanSignatures()
+}
+
+func (c *Config) parseIgnoreSignatures() {
 	if c.ignoreChecksSignatures == nil && len(c.IgnoreChecksSignaturesSlice) > 0 {
 		if len(c.IgnoreChecksSignaturesSlice) == 1 && c.IgnoreChecksSignaturesSlice[0] == "" {
 			return
@@ -92,6 +118,62 @@ func (c *Config) parseSignatures() {
 	}
 }
 
+func (c *Config) parseStartSpanSignatures() {
+	if c.startSpanMatchers != nil {
+		return
+	}
+
+	customMatchers := []string{}
+	for i, sig := range c.StartSpanMatchersSlice {
+		parts := strings.Split(sig, ":")
+
+		// Make sure we have both a signature and a telemetry type
+		if len(parts) != startSpanSignatureCols {
+			log.Default().Printf("[WARN] invalid start span signature \"%s\". expected regex:telemetry-type\n", sig)
+
+			continue
+		}
+
+		sig, sigType := parts[0], parts[1]
+		if len(sig) < 1 {
+			log.Default().Print("[WARN] invalid start span signature, empty pattern")
+
+			continue
+		}
+
+		spanType, ok := SpanTypes[sigType]
+		if !ok {
+			validSpanTypes := make([]string, 0, len(SpanTypes))
+			for k := range SpanTypes {
+				validSpanTypes = append(validSpanTypes, k)
+			}
+
+			log.Default().
+				Printf("[WARN] invalid start span type \"%s\". expected one of %s\n", sigType, strings.Join(validSpanTypes, ", "))
+
+			continue
+		}
+
+		regex, err := regexp.Compile(sig)
+		if err != nil {
+			log.Default().Printf("[WARN] failed to compile regex from signature %s: %v\n", sig, err)
+
+			continue
+		}
+
+		c.startSpanMatchers = append(c.startSpanMatchers, spanStartMatcher{
+			signature: regex,
+			spanType:  spanType,
+		})
+
+		if i >= len(defaultStartSpanSignatures) {
+			customMatchers = append(customMatchers, sig)
+		}
+	}
+
+	c.startSpanMatchersCustomRegex = createRegex(customMatchers)
+}
+
 func parseChecks(checksSlice []string) []Check {
 	if len(checksSlice) == 0 {
 		return nil

spancheck.go

@@ -23,11 +23,15 @@ const (
 	spanOpenCensus                    // from go.opencensus.io/trace
 )
 
-var (
-	// this approach stolen from errcheck
-	// https://github.com/kisielk/errcheck/blob/7f94c385d0116ccc421fbb4709e4a484d98325ee/errcheck/errcheck.go#L22
-	errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
-)
+// SpanTypes is a list of all span types by name.
+var SpanTypes = map[string]spanType{
+	"opentelemetry": spanOpenTelemetry,
+	"opencensus":    spanOpenCensus,
+}
+
+// this approach stolen from errcheck
+// https://github.com/kisielk/errcheck/blob/7f94c385d0116ccc421fbb4709e4a484d98325ee/errcheck/errcheck.go#L22
+var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
 
 // NewAnalyzerWithConfig returns a new analyzer configured with the Config passed in.
 // Its config can be set for testing.
@@ -84,6 +88,12 @@ func runFunc(pass *analysis.Pass, node ast.Node, config *Config) {
 		funcScope = pass.TypesInfo.Scopes[v.Type]
 	case *ast.FuncDecl:
 		funcScope = pass.TypesInfo.Scopes[v.Type]
+		fnSig := pass.TypesInfo.ObjectOf(v.Name).String()
+
+		// Skip checking spans in this function if it's a custom starter/creator.
+		if config.startSpanMatchersCustomRegex != nil && config.startSpanMatchersCustomRegex.MatchString(fnSig) {
+			return
+		}
 	}
 
 	// Maps each span variable to its defining ValueSpec/AssignStmt.
@@ -108,8 +118,12 @@ func runFunc(pass *analysis.Pass, node ast.Node, config *Config) {
 		//   ctx, span     := otel.Tracer("app").Start(...)
 		//   ctx, span     = otel.Tracer("app").Start(...)
 		//   var ctx, span = otel.Tracer("app").Start(...)
-		sType, sStart := isSpanStart(pass.TypesInfo, n)
-		if !sStart || !isCall(stack[len(stack)-2]) {
+		sType, isStart := isSpanStart(pass.TypesInfo, n, config.startSpanMatchers)
+		if !isStart {
+			return true
+		}
+
+		if !isCall(stack[len(stack)-2]) {
 			return true
 		}
 
@@ -169,23 +183,23 @@ func runFunc(pass *analysis.Pass, node ast.Node, config *Config) {
 	for _, sv := range spanVars {
 		if config.endCheckEnabled {
 			// Check if there's no End to the span.
-			if ret := getMissingSpanCalls(pass, g, sv, "End", func(pass *analysis.Pass, ret *ast.ReturnStmt) *ast.ReturnStmt { return ret }, nil); ret != nil {
+			if ret := getMissingSpanCalls(pass, g, sv, "End", func(_ *analysis.Pass, ret *ast.ReturnStmt) *ast.ReturnStmt { return ret }, nil, config.startSpanMatchers); ret != nil {
 				pass.ReportRangef(sv.stmt, "%s.End is not called on all paths, possible memory leak", sv.vr.Name())
 				pass.ReportRangef(ret, "return can be reached without calling %s.End", sv.vr.Name())
 			}
 		}
 
 		if config.setStatusEnabled {
 			// Check if there's no SetStatus to the span setting an error.
-			if ret := getMissingSpanCalls(pass, g, sv, "SetStatus", getErrorReturn, config.ignoreChecksSignatures); ret != nil {
+			if ret := getMissingSpanCalls(pass, g, sv, "SetStatus", getErrorReturn, config.ignoreChecksSignatures, config.startSpanMatchers); ret != nil {
 				pass.ReportRangef(sv.stmt, "%s.SetStatus is not called on all paths", sv.vr.Name())
 				pass.ReportRangef(ret, "return can be reached without calling %s.SetStatus", sv.vr.Name())
 			}
 		}
 
 		if config.recordErrorEnabled && sv.spanType == spanOpenTelemetry { // RecordError only exists in OpenTelemetry
 			// Check if there's no RecordError to the span setting an error.
-			if ret := getMissingSpanCalls(pass, g, sv, "RecordError", getErrorReturn, config.ignoreChecksSignatures); ret != nil {
+			if ret := getMissingSpanCalls(pass, g, sv, "RecordError", getErrorReturn, config.ignoreChecksSignatures, config.startSpanMatchers); ret != nil {
 				pass.ReportRangef(sv.stmt, "%s.RecordError is not called on all paths", sv.vr.Name())
 				pass.ReportRangef(ret, "return can be reached without calling %s.RecordError", sv.vr.Name())
 			}
@@ -194,25 +208,22 @@ func runFunc(pass *analysis.Pass, node ast.Node, config *Config) {
 }
 
 // isSpanStart reports whether n is tracer.Start()
-func isSpanStart(info *types.Info, n ast.Node) (spanType, bool) {
+func isSpanStart(info *types.Info, n ast.Node, startSpanMatchers []spanStartMatcher) (spanType, bool) {
 	sel, ok := n.(*ast.SelectorExpr)
 	if !ok {
 		return spanUnset, false
 	}
 
-	switch sel.Sel.Name {
-	case "Start": // https://github.com/open-telemetry/opentelemetry-go/blob/98b32a6c3a87fbee5d34c063b9096f416b250897/trace/trace.go#L523
-		obj, ok := info.Uses[sel.Sel]
-		return spanOpenTelemetry, ok && obj.Pkg().Path() == "go.opentelemetry.io/otel/trace"
-	case "StartSpan": // https://pkg.go.dev/go.opencensus.io/trace#StartSpan
-		obj, ok := info.Uses[sel.Sel]
-		return spanOpenCensus, ok && obj.Pkg().Path() == "go.opencensus.io/trace"
-	case "StartSpanWithRemoteParent": // https://github.com/census-instrumentation/opencensus-go/blob/v0.24.0/trace/trace_api.go#L66
-		obj, ok := info.Uses[sel.Sel]
-		return spanOpenCensus, ok && obj.Pkg().Path() == "go.opencensus.io/trace"
-	default:
-		return spanUnset, false
+	fnSig := info.ObjectOf(sel.Sel).String()
+
+	// Check if the function is a span start function
+	for _, matcher := range startSpanMatchers {
+		if matcher.signature.MatchString(fnSig) {
+			return matcher.spanType, true
+		}
 	}
+
+	return 0, false
 }
 
 func isCall(n ast.Node) bool {
@@ -225,11 +236,16 @@ func getID(node ast.Node) *ast.Ident {
 	case *ast.ValueSpec:
 		if len(stmt.Names) > 1 {
 			return stmt.Names[1]
+		} else if len(stmt.Names) == 1 {
+			return stmt.Names[0]
 		}
 	case *ast.AssignStmt:
 		if len(stmt.Lhs) > 1 {
 			id, _ := stmt.Lhs[1].(*ast.Ident)
 			return id
+		} else if len(stmt.Lhs) == 1 {
+			id, _ := stmt.Lhs[0].(*ast.Ident)
+			return id
 		}
 	}
 	return nil
@@ -244,13 +260,14 @@ func getMissingSpanCalls(
 	selName string,
 	checkErr func(pass *analysis.Pass, ret *ast.ReturnStmt) *ast.ReturnStmt,
 	ignoreCheckSig *regexp.Regexp,
+	spanStartMatchers []spanStartMatcher,
 ) *ast.ReturnStmt {
 	// blockUses computes "uses" for each block, caching the result.
 	memo := make(map[*cfg.Block]bool)
 	blockUses := func(pass *analysis.Pass, b *cfg.Block) bool {
 		res, ok := memo[b]
 		if !ok {
-			res = usesCall(pass, b.Nodes, sv, selName, ignoreCheckSig, 0)
+			res = usesCall(pass, b.Nodes, sv, selName, ignoreCheckSig, spanStartMatchers, 0)
 			memo[b] = res
 		}
 		return res
@@ -272,7 +289,7 @@ outer:
 	}
 
 	// Is the call "used" in the remainder of its defining block?
-	if usesCall(pass, rest, sv, selName, ignoreCheckSig, 0) {
+	if usesCall(pass, rest, sv, selName, ignoreCheckSig, spanStartMatchers, 0) {
 		return nil
 	}
 
@@ -314,7 +331,15 @@ outer:
 }
 
 // usesCall reports whether stmts contain a use of the selName call on variable v.
-func usesCall(pass *analysis.Pass, stmts []ast.Node, sv spanVar, selName string, ignoreCheckSig *regexp.Regexp, depth int) bool {
+func usesCall(
+	pass *analysis.Pass,
+	stmts []ast.Node,
+	sv spanVar,
+	selName string,
+	ignoreCheckSig *regexp.Regexp,
+	startSpanMatchers []spanStartMatcher,
+	depth int,
+) bool {
 	if depth > 1 { // for perf reasons, do not dive too deep thru func literals, just one level deep check.
 		return false
 	}
@@ -329,7 +354,7 @@ func usesCall(pass *analysis.Pass, stmts []ast.Node, sv spanVar, selName string,
 					cfgs := pass.ResultOf[ctrlflow.Analyzer].(*ctrlflow.CFGs)
 					g := cfgs.FuncLit(n)
 					if g != nil && len(g.Blocks) > 0 {
-						return usesCall(pass, g.Blocks[0].Nodes, sv, selName, ignoreCheckSig, depth+1)
+						return usesCall(pass, g.Blocks[0].Nodes, sv, selName, ignoreCheckSig, startSpanMatchers, depth+1)
 					}
 
 					return false
@@ -352,8 +377,8 @@ func usesCall(pass *analysis.Pass, stmts []ast.Node, sv spanVar, selName string,
 			stack = append(stack, n) // push
 
 			// Check whether the span was assigned over top of its old value.
-			_, spanStart := isSpanStart(pass.TypesInfo, n)
-			if spanStart {
+			_, isStart := isSpanStart(pass.TypesInfo, n, startSpanMatchers)
+			if isStart {
 				if id := getID(stack[len(stack)-3]); id != nil && id.Obj.Decl == sv.id.Obj.Decl {
 					reAssigned = true
 					return false