Skip to content

Commit

Permalink
Fix enhance method error when unknown parameter type (#160)
Browse files Browse the repository at this point in the history
  • Loading branch information
mrproliu authored Jan 10, 2024
1 parent 2c1ffe8 commit 63bab93
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 43 deletions.
3 changes: 2 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ Release Notes.
* Fix users can not use async api in toolkit-trace.
* Fix cannot enhance the vendor management project.
* Fix SW_AGENT_REPORTER_GRPC_MAX_SEND_QUEUE not working on metricsSendCh & logSendCh chans of gRPC reporter.
* Fix ParseVendorModule error for special case in vendor/modules.txt
* Fix ParseVendorModule error for special case in vendor/modules.txt.
* Fix enhance method error when unknown parameter type.

#### Issues and PR
- All issues are [here](https://github.com/apache/skywalking/milestone/197?closed=1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ if (invocation.isContinue) {
if invocation.returnValues[{{$index}}] != nil {
*ret_{{$index}} = (invocation.returnValues[{{$index}}]).({{$value.PackagedTypeName}})
} else {
*ret_{{$index}} = {{$value.DefaultValueAsString}}
var tmp_{{$index}} {{$value.PackagedTypeName}}
*ret_{{$index}} = tmp_{{$index}}
}
{{- end }}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,21 @@ invocation.changeArgCallback = func(index int, value interface{}) {
panic("cannot found the argument index")
}

{{- range $index, $value := .Results }}
var def_res_{{$index}} {{$value.PackagedTypeName}}
{{- end}}
// real invoke
if err := {{.InterceptorVarName}}.BeforeInvoke(invocation); err != nil {
// using go2sky log error
log.Warnf("execute interceptor before invoke error, instrument name: %s, interceptor name: %s, function ID: %s, error: %v",
"{{.InstrumentName}}", "{{.InterceptorDefineName}}", "{{.FuncID}}", err)

return {{ range $index, $value := .Results -}}
{{$value.DefaultValueAsString}},
def_res_{{$index}},
{{- end}}invocation, false
}
if (invocation.isContinue) {
{{- range $index, $value := .Results }}
var def_res_{{$index}} {{$value.PackagedTypeName}}
if invocation.returnValues[{{$index}}] != nil {
def_res_{{$index}} = (invocation.returnValues[{{$index}}]).({{$value.PackagedTypeName}})
}
Expand All @@ -46,5 +49,5 @@ if (invocation.isContinue) {
{{- end}}invocation, true
}
return {{ range $index, $value := .Results -}}
{{- if ne $index 0}}, {{end}}{{$value.DefaultValueAsString }}
{{- end}}{{if .Results}}, {{- end}}invocation, false
def_res_{{$index}},
{{- end}}invocation, false
27 changes: 3 additions & 24 deletions tools/go-agent/tools/enhancement.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ package tools

import (
"fmt"
"go/token"
"strings"

"github.com/dave/dst"
"github.com/dave/dst/decorator"
Expand All @@ -31,10 +29,9 @@ const OtherPackageRefPrefix = "swref_"
const parameterAppender = ", "

type ParameterInfo struct {
Name string
Type dst.Expr
DefaultValueAsString string
TypeName string
Name string
Type dst.Expr
TypeName string
}

type PackagedParameterInfo struct {
Expand Down Expand Up @@ -141,24 +138,6 @@ func newParameterInfo(name string, tp dst.Expr) *ParameterInfo {
Type: tp,
TypeName: GenerateTypeNameByExp(tp),
}
var defaultNil = "nil"
switch n := tp.(type) {
case *dst.Ident:
if n.Name == "string" {
defaultNil = `""`
} else if n.Name == "bool" {
defaultNil = "false"
} else if strings.HasPrefix(n.Name, "int") || strings.HasPrefix(n.Name, "uint") ||
strings.HasPrefix(n.Name, "float") || n.Name == "byte" || n.Name == "rune" {
defaultNil = "0"
}
case *dst.UnaryExpr:
if n.Op == token.INT || n.Op == token.FLOAT {
defaultNil = "0"
}
}
result.DefaultValueAsString = defaultNil

return result
}

Expand Down
22 changes: 9 additions & 13 deletions tools/go-agent/tools/enhancement_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ import (
"github.com/dave/dst"
)

func buildParameterValidateInfo(name, typeName, defaultValue string) *ParameterInfo {
func buildParameterValidateInfo(name, typeName string) *ParameterInfo {
return &ParameterInfo{
Name: name,
TypeName: typeName,
DefaultValueAsString: defaultValue,
Name: name,
TypeName: typeName,
}
}

Expand All @@ -43,27 +42,27 @@ func TestEnhanceParameterNames(t *testing.T) {
return false
}`,
recvs: []*ParameterInfo{
buildParameterValidateInfo("skywalking_recv_0", "*Example", "nil"),
buildParameterValidateInfo("skywalking_recv_0", "*Example"),
},
params: []*ParameterInfo{
buildParameterValidateInfo("skywalking_param_0", "int", "0"),
buildParameterValidateInfo("skywalking_param_0", "int"),
},
results: []*ParameterInfo{
buildParameterValidateInfo("skywalking_result_0", "bool", "false"),
buildParameterValidateInfo("skywalking_result_0", "bool"),
},
},
{
funcCode: `func (e *Example) Test(i int) (b bool) {
return false
}`,
recvs: []*ParameterInfo{
buildParameterValidateInfo("e", "*Example", "nil"),
buildParameterValidateInfo("e", "*Example"),
},
params: []*ParameterInfo{
buildParameterValidateInfo("i", "int", "0"),
buildParameterValidateInfo("i", "int"),
},
results: []*ParameterInfo{
buildParameterValidateInfo("b", "bool", "false"),
buildParameterValidateInfo("b", "bool"),
},
},
}
Expand Down Expand Up @@ -95,8 +94,5 @@ func validateParameterInfo(t *testing.T, inx int, flistType FieldListType, actua
if exp.TypeName != act.TypeName {
t.Errorf("case %d:%s: expected type %s , actual %s", inx, flistType, exp.TypeName, act.TypeName)
}
if exp.DefaultValueAsString != act.DefaultValueAsString {
t.Errorf("case %d:%s: expected default value %s , actual %s", inx, flistType, exp.DefaultValueAsString, act.DefaultValueAsString)
}
}
}

0 comments on commit 63bab93

Please sign in to comment.