Skip to content

Commit

Permalink
Fix context replacement for http.Request
Browse files Browse the repository at this point in the history
The context of a HTTP request is not assignable, so we need to read from the existing context and create a new one
  • Loading branch information
gagbo committed Nov 6, 2023
1 parent 2204339 commit 71c2fd0
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 22 deletions.
9 changes: 5 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ specification.
tracking. If `autometrics` detects a `context.Context` it can use, it will shadow the
context with the autometrics augmented one to reduce the changes to make in the code.
Currently, `autometrics` can detect arguments of specific types in the function signatures
to replace contexts:
+ `context.Context`
+ `http.Request`
+ `buffalo.Request`
to read from and replace contexts:
+ `context.Context` (read and replace)
+ `http.Request` (read only)
+ `buffalo.Request` (read and replace)
+ `gin.Context` (read only; _very_ experimental)
- [Generator] The generator now tries to keep going with instrumentation even if some instrumentation
fails. On error, still _No file will be modified_, and the generator will exit with an error code
and output all the collected errors instead of only the first one.
Expand Down
8 changes: 4 additions & 4 deletions examples/otel/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ func main() {
//
//autometrics:doc --slo "API" --latency-target 99 --latency-ms 5
func indexHandler(w http.ResponseWriter, r *http.Request) error {
r.Context() = autometrics.PreInstrument(autometrics.NewContext(
amCtx := autometrics.PreInstrument(autometrics.NewContext(
r.Context(),
autometrics.WithConcurrentCalls(true),
autometrics.WithCallerName(true),
autometrics.WithSloName("API"),
autometrics.WithAlertLatency(5000000*time.Nanosecond, 99),
)) //autometrics:shadow-ctx
defer autometrics.Instrument(r.Context(), nil) //autometrics:defer
defer autometrics.Instrument(amCtx, nil) //autometrics:defer

msSleep := rand.Intn(200)
time.Sleep(time.Duration(msSleep) * time.Millisecond)
Expand Down Expand Up @@ -149,14 +149,14 @@ var handlerError = errors.New("failed to handle request")
//
//autometrics:doc --slo "API" --success-target 90
func randomErrorHandler(w http.ResponseWriter, r *http.Request) (err error) {
r.Context() = autometrics.PreInstrument(autometrics.NewContext(
amCtx := autometrics.PreInstrument(autometrics.NewContext(
r.Context(),
autometrics.WithConcurrentCalls(true),
autometrics.WithCallerName(true),
autometrics.WithSloName("API"),
autometrics.WithAlertSuccess(90),
)) //autometrics:shadow-ctx
defer autometrics.Instrument(r.Context(), &err) //autometrics:defer
defer autometrics.Instrument(amCtx, &err) //autometrics:defer

isOk := rand.Intn(10) == 0

Expand Down
4 changes: 2 additions & 2 deletions examples/web/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,14 @@ func main() {
//
//autometrics:inst --slo "API" --latency-target 99 --latency-ms 5
func indexHandler(w http.ResponseWriter, r *http.Request) error {
r.Context() = autometrics.PreInstrument(autometrics.NewContext(
amCtx := autometrics.PreInstrument(autometrics.NewContext(
r.Context(),
autometrics.WithConcurrentCalls(true),
autometrics.WithCallerName(true),
autometrics.WithSloName("API"),
autometrics.WithAlertLatency(5000000*time.Nanosecond, 99),
)) //autometrics:shadow-ctx
defer autometrics.Instrument(r.Context(), nil) //autometrics:defer
defer autometrics.Instrument(amCtx, nil) //autometrics:defer

msSleep := rand.Intn(200)
time.Sleep(time.Duration(msSleep) * time.Millisecond)
Expand Down
12 changes: 8 additions & 4 deletions internal/autometrics/ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,11 @@ type GeneratorContext struct {
// only statically-known information is useful at this point.
type RuntimeCtxInfo struct {
// Name of the variable to use as context.Context when building the autometrics.Context.
// The string will be empty if and only if 'nil' must be used as autometrics.NewContext() argument.
// The string will be empty if 'nil' must be used as autometrics.NewContext() argument.
ContextVariableName string
// Name of the variable to assign to the context returned by autometrics.PreInstrument
// The string will be empty if a new variable must be used as autometrics.PreInstrument() return value assignment.
NewContextVariableName string
// Verbatim code to use to fetch the TraceID.
// For example, if the instrumented function is detected to use Gin, like
// `func ginHandler(ginVarName *gin.Context)`
Expand All @@ -65,9 +68,10 @@ type RuntimeCtxInfo struct {

func DefaultRuntimeCtxInfo() RuntimeCtxInfo {
return RuntimeCtxInfo{
TrackConcurrentCalls: true,
TrackCallerName: true,
ContextVariableName: "nil",
TrackConcurrentCalls: true,
TrackCallerName: true,
ContextVariableName: "nil",
NewContextVariableName: "",
}
}

Expand Down
10 changes: 8 additions & 2 deletions internal/generate/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ func buildAutometricsContextNode(agc *internal.GeneratorContext) (newContextCall
}
}

contextShadowName = agc.RuntimeCtx.ContextVariableName
if contextShadowName == "nil" {
contextShadowName = agc.RuntimeCtx.NewContextVariableName
if contextShadowName == "nil" || contextShadowName == "" {
contextShadowName = amDefaultContextName
}

Expand Down Expand Up @@ -224,6 +224,7 @@ func detectContextIdentImpl(ctx *internal.GeneratorContext, argName string, iden

if canonical == vanillaContext && typeName == "Context" {
ctx.RuntimeCtx.ContextVariableName = argName
ctx.RuntimeCtx.NewContextVariableName = argName
ctx.RuntimeCtx.SpanIDGetter = ""
ctx.RuntimeCtx.TraceIDGetter = ""
return true, nil
Expand All @@ -236,6 +237,7 @@ func detectContextIdentImpl(ctx *internal.GeneratorContext, argName string, iden
} else {
ctx.RuntimeCtx.ContextVariableName = fmt.Sprintf("%s.Context()", argName)
}
ctx.RuntimeCtx.NewContextVariableName = ""
ctx.RuntimeCtx.SpanIDGetter = ""
ctx.RuntimeCtx.TraceIDGetter = ""
return true, nil
Expand All @@ -250,6 +252,7 @@ func detectContextIdentImpl(ctx *internal.GeneratorContext, argName string, iden
// Buffalo context embeds a context.Context so it can work like vanilla
if canonical == buffalo && typeName == "Context" {
ctx.RuntimeCtx.ContextVariableName = argName
ctx.RuntimeCtx.NewContextVariableName = argName
ctx.RuntimeCtx.SpanIDGetter = ""
ctx.RuntimeCtx.TraceIDGetter = ""
return true, nil
Expand All @@ -275,6 +278,7 @@ func detectContextSelectorImpl(ctx *internal.GeneratorContext, argName string, s
for alias, canonical := range ctx.ImportsMap {
if canonical == vanillaContext && parentName == alias && typeName == "Context" {
ctx.RuntimeCtx.ContextVariableName = argName
ctx.RuntimeCtx.NewContextVariableName = argName
ctx.RuntimeCtx.SpanIDGetter = ""
ctx.RuntimeCtx.TraceIDGetter = ""
return true, nil
Expand All @@ -288,6 +292,7 @@ func detectContextSelectorImpl(ctx *internal.GeneratorContext, argName string, s
} else {
ctx.RuntimeCtx.ContextVariableName = fmt.Sprintf("%s.Context()", argName)
}
ctx.RuntimeCtx.NewContextVariableName = ""
ctx.RuntimeCtx.SpanIDGetter = ""
ctx.RuntimeCtx.TraceIDGetter = ""
return true, nil
Expand All @@ -302,6 +307,7 @@ func detectContextSelectorImpl(ctx *internal.GeneratorContext, argName string, s
// Buffalo context embeds a context.Context so it can work like vanilla
if canonical == buffalo && parentName == alias && typeName == "Context" {
ctx.RuntimeCtx.ContextVariableName = argName
ctx.RuntimeCtx.NewContextVariableName = argName
ctx.RuntimeCtx.SpanIDGetter = ""
ctx.RuntimeCtx.TraceIDGetter = ""
return true, nil
Expand Down
12 changes: 6 additions & 6 deletions internal/generate/defer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,14 @@ func main(w http.ResponseWriter, req *http.Request) {
"//\n" +
"//autometrics:inst --no-doc --slo \"Service Test\" --success-target 99\n" +
"func main(w http.ResponseWriter, req *http.Request) {\n" +
"\treq.Context() = prom.PreInstrument(prom.NewContext(\n" +
"\tamCtx := prom.PreInstrument(prom.NewContext(\n" +
"\t\treq.Context(),\n" +
"\t\tprom.WithConcurrentCalls(true),\n" +
"\t\tprom.WithCallerName(true),\n" +
"\t\tprom.WithSloName(\"Service Test\"),\n" +
"\t\tprom.WithAlertSuccess(99),\n" +
"\t)) //autometrics:shadow-ctx\n" +
"\tdefer prom.Instrument(req.Context(), nil) //autometrics:defer\n" +
"\tdefer prom.Instrument(amCtx, nil) //autometrics:defer\n" +
"\n" +
" fmt.Println(hello) // line comment 3\n" +
"}\n"
Expand Down Expand Up @@ -274,14 +274,14 @@ func main(w vanilla.ResponseWriter, req *vanilla.Request) {
"//\n" +
"//autometrics:inst --no-doc --slo \"Service Test\" --success-target 99\n" +
"func main(w vanilla.ResponseWriter, req *vanilla.Request) {\n" +
"\treq.Context() = prom.PreInstrument(prom.NewContext(\n" +
"\tamCtx := prom.PreInstrument(prom.NewContext(\n" +
"\t\treq.Context(),\n" +
"\t\tprom.WithConcurrentCalls(true),\n" +
"\t\tprom.WithCallerName(true),\n" +
"\t\tprom.WithSloName(\"Service Test\"),\n" +
"\t\tprom.WithAlertSuccess(99),\n" +
"\t)) //autometrics:shadow-ctx\n" +
"\tdefer prom.Instrument(req.Context(), nil) //autometrics:defer\n" +
"\tdefer prom.Instrument(amCtx, nil) //autometrics:defer\n" +
"\n" +
" fmt.Println(hello) // line comment 3\n" +
"}\n"
Expand Down Expand Up @@ -332,14 +332,14 @@ func main(w ResponseWriter, req *Request) {
"//\n" +
"//autometrics:inst --no-doc --slo \"Service Test\" --success-target 99\n" +
"func main(w ResponseWriter, req *Request) {\n" +
"\treq.Context() = prom.PreInstrument(prom.NewContext(\n" +
"\tamCtx := prom.PreInstrument(prom.NewContext(\n" +
"\t\treq.Context(),\n" +
"\t\tprom.WithConcurrentCalls(true),\n" +
"\t\tprom.WithCallerName(true),\n" +
"\t\tprom.WithSloName(\"Service Test\"),\n" +
"\t\tprom.WithAlertSuccess(99),\n" +
"\t)) //autometrics:shadow-ctx\n" +
"\tdefer prom.Instrument(req.Context(), nil) //autometrics:defer\n" +
"\tdefer prom.Instrument(amCtx, nil) //autometrics:defer\n" +
"\n" +
" fmt.Println(hello) // line comment 3\n" +
"}\n"
Expand Down

0 comments on commit 71c2fd0

Please sign in to comment.