diff --git a/internal/generate/generate.go b/internal/generate/generate.go index 51e6260..dc3c5d8 100644 --- a/internal/generate/generate.go +++ b/internal/generate/generate.go @@ -170,6 +170,17 @@ func walkFuncDeclaration(ctx *internal.GeneratorContext, funcDeclaration *dst.Fu // Insert comments if !ctx.DisableDocGeneration && !ctx.FuncCtx.DisableDocGeneration { autometricsComment := generateAutometricsComment(*ctx) + + // HACK: gopls will ignore the doc comment completely when requested for documentation if the docComment + // starts with an indented line. Autometrics currently uses an indented line at the beginning of the + // generated section to be able to clean up after itself. Therefore gopls will _not_ display anx documentation + // if the docComment contains only autometrics docs. + // To fix this issue, if we detect that the autometrics comment would start the function documentation, we + // artificially insert an extra line that contains only the function name. + if listIndex == 0 { + autometricsComment = append([]string{fmt.Sprintf("// %s", funcDeclaration.Name.Name)}, autometricsComment...) + } + funcDeclaration.Decorations().Start.Replace(insertComments(docComments, listIndex, autometricsComment)...) } else { funcDeclaration.Decorations().Start.Replace(docComments...) diff --git a/internal/generate/generate_test.go b/internal/generate/generate_test.go index 614e208..59771f8 100644 --- a/internal/generate/generate_test.go +++ b/internal/generate/generate_test.go @@ -94,6 +94,184 @@ func main() { assert.Equal(t, want, actual, "The generated source code is not as expected.") } +// This test makes sure that creating a doc comment when the function had +// nothing before will add a header with the function name before the +// autometrics section, to be sure that gopls (and IDEs downstream) will +// correctly render the whole doc comment +func TestInstrumentDirectiveWithoutPreexistingComment(t *testing.T) { + sourceCode := `// This is the package comment. +package main + +import ( + prom "github.com/autometrics-dev/autometrics-go/prometheus/autometrics" +) + +//autometrics:inst --slo "Service Test" --success-target 99 +func main() { + fmt.Println(hello) // line comment 3 +} +` + + want := `// This is the package comment. +package main + +import ( + prom "github.com/autometrics-dev/autometrics-go/prometheus/autometrics" +) + +// main +// +// autometrics:doc-start Generated documentation by Autometrics. +// +// # Autometrics +// +// # Prometheus +// +// View the live metrics for the ` + "`main`" + ` function: +// - [Request Rate] +// - [Error Ratio] +// - [Latency (95th and 99th percentiles)] +// - [Concurrent Calls] +// +// Or, dig into the metrics of *functions called by* ` + "`main`" + ` +// - [Request Rate Callee] +// - [Error Ratio Callee] +// +// autometrics:doc-end Generated documentation by Autometrics. +// +// [Request Rate]: http://localhost:9090/graph?g0.expr=%23+Rate+of+calls+to+the+%60main%60+function+per+second%2C+averaged+over+5+minute+windows%0A%0Asum+by+%28function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_total%7Bfunction%3D%22main%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29&g0.tab=0 +// [Error Ratio]: http://localhost:9090/graph?g0.expr=%23+Percentage+of+calls+to+the+%60main%60+function+that+return+errors%2C+averaged+over+5+minute+windows%0A%0A%28sum+by+%28function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_total%7Bfunction%3D%22main%22%2Cresult%3D%22error%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29%29+%2F+%28sum+by+%28function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_total%7Bfunction%3D%22main%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29%29&g0.tab=0 +// [Latency (95th and 99th percentiles)]: http://localhost:9090/graph?g0.expr=%23+95th+and+99th+percentile+latencies+%28in+seconds%29+for+the+%60main%60+function%0A%0Alabel_replace%28histogram_quantile%280.99%2C+sum+by+%28le%2C+function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_duration_seconds_bucket%7Bfunction%3D%22main%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29%29%2C+%22percentile_latency%22%2C+%2299%22%2C+%22%22%2C+%22%22%29+or+label_replace%28histogram_quantile%280.95%2C+sum+by+%28le%2C+function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_duration_seconds_bucket%7Bfunction%3D%22main%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29%29%2C%22percentile_latency%22%2C+%2295%22%2C+%22%22%2C+%22%22%29&g0.tab=0 +// [Concurrent Calls]: http://localhost:9090/graph?g0.expr=%23+Concurrent+calls+to+the+%60main%60+function%0A%0Asum+by+%28function%2C+module%2C+service_name%2C+version%2C+commit%29+%28function_calls_concurrent%7Bfunction%3D%22main%22%7D+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29&g0.tab=0 +// [Request Rate Callee]: http://localhost:9090/graph?g0.expr=%23+Rate+of+function+calls+emanating+from+%60main%60+function+per+second%2C+averaged+over+5+minute+windows%0A%0Asum+by+%28function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_total%7Bcaller_function%3D%22main%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29&g0.tab=0 +// [Error Ratio Callee]: http://localhost:9090/graph?g0.expr=%23+Percentage+of+function+emanating+from+%60main%60+function+that+return+errors%2C+averaged+over+5+minute+windows%0A%0A%28sum+by+%28function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_total%7Bcaller_function%3D%22main%22%2Cresult%3D%22error%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29%29+%2F+%28sum+by+%28function%2C+module%2C+service_name%2C+version%2C+commit%29+%28rate%28function_calls_total%7Bcaller_function%3D%22main%22%7D%5B5m%5D%29+%2A+on+%28instance%2C+job%29+group_left%28version%2C+commit%29+last_over_time%28build_info%5B1s%5D%29%29%29&g0.tab=0 +// +//autometrics:inst --slo "Service Test" --success-target 99 +func main() { + defer prom.Instrument(prom.PreInstrument(prom.NewContext( + nil, + prom.WithConcurrentCalls(true), + prom.WithCallerName(true), + prom.WithSloName("Service Test"), + prom.WithAlertSuccess(99), + )), nil) //autometrics:defer + + fmt.Println(hello) // line comment 3 +} +` + + ctx, err := internal.NewGeneratorContext(autometrics.PROMETHEUS, defaultPrometheusInstanceUrl, false, false) + if err != nil { + t.Fatalf("error creating the generation context: %s", err) + } + + actual, err := GenerateDocumentationAndInstrumentation(ctx, sourceCode, "main") + if err != nil { + t.Fatalf("error generating the documentation: %s", err) + } + + assert.Equal(t, want, actual, "The generated source code is not as expected.") +} + +func TestInstrumentDirectiveWithoutDoc(t *testing.T) { + sourceCode := `// This is the package comment. +package main + +import ( + prom "github.com/autometrics-dev/autometrics-go/prometheus/autometrics" +) + +// main is a function here. +//autometrics:inst --no-doc --slo "Service Test" --success-target 99 +func main() { + fmt.Println(hello) // line comment 3 +} +` + + want := `// This is the package comment. +package main + +import ( + prom "github.com/autometrics-dev/autometrics-go/prometheus/autometrics" +) + +// main is a function here. +// +//autometrics:inst --no-doc --slo "Service Test" --success-target 99 +func main() { + defer prom.Instrument(prom.PreInstrument(prom.NewContext( + nil, + prom.WithConcurrentCalls(true), + prom.WithCallerName(true), + prom.WithSloName("Service Test"), + prom.WithAlertSuccess(99), + )), nil) //autometrics:defer + + fmt.Println(hello) // line comment 3 +} +` + + ctx, err := internal.NewGeneratorContext(autometrics.PROMETHEUS, defaultPrometheusInstanceUrl, false, false) + if err != nil { + t.Fatalf("error creating the generation context: %s", err) + } + + actual, err := GenerateDocumentationAndInstrumentation(ctx, sourceCode, "main") + if err != nil { + t.Fatalf("error generating the documentation: %s", err) + } + + assert.Equal(t, want, actual, "The generated source code is not as expected.") +} + +func TestInstrumentDirectiveWithoutDocNorComment(t *testing.T) { + sourceCode := `// This is the package comment. +package main + +import ( + prom "github.com/autometrics-dev/autometrics-go/prometheus/autometrics" +) + +//autometrics:inst --no-doc --slo "Service Test" --success-target 99 +func main() { + fmt.Println(hello) // line comment 3 +} +` + + want := `// This is the package comment. +package main + +import ( + prom "github.com/autometrics-dev/autometrics-go/prometheus/autometrics" +) + +//autometrics:inst --no-doc --slo "Service Test" --success-target 99 +func main() { + defer prom.Instrument(prom.PreInstrument(prom.NewContext( + nil, + prom.WithConcurrentCalls(true), + prom.WithCallerName(true), + prom.WithSloName("Service Test"), + prom.WithAlertSuccess(99), + )), nil) //autometrics:defer + + fmt.Println(hello) // line comment 3 +} +` + + ctx, err := internal.NewGeneratorContext(autometrics.PROMETHEUS, defaultPrometheusInstanceUrl, false, false) + if err != nil { + t.Fatalf("error creating the generation context: %s", err) + } + + actual, err := GenerateDocumentationAndInstrumentation(ctx, sourceCode, "main") + if err != nil { + t.Fatalf("error generating the documentation: %s", err) + } + + assert.Equal(t, want, actual, "The generated source code is not as expected.") +} + // TestCommentRefresh calls GenerateDocumentationAndInstrumentation on a // decorated function that already has a comment, making sure that the autometrics // directive only updates the comment section about autometrics. @@ -474,7 +652,6 @@ func main() { _, err = GenerateDocumentationAndInstrumentation(ctx, sourceCode, "main") assert.Error(t, err, "Calling generation must fail if no service name is given.") - } func TestInputValidationLatencyErrors(t *testing.T) {