Skip to content

Commit

Permalink
Ensure unindented header for documented functions
Browse files Browse the repository at this point in the history
Having an unindented header is the only way to make sure that gopls will
correctly render the documentation of an instrumented function with the
links
  • Loading branch information
gagbo committed Oct 13, 2023
1 parent 6fb3c8d commit 6ded71b
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 1 deletion.
11 changes: 11 additions & 0 deletions internal/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down
179 changes: 178 additions & 1 deletion internal/generate/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 6ded71b

Please sign in to comment.