apache / skywalking

APM, Application Performance Monitoring System
https://skywalking.apache.org/
Apache License 2.0
23.62k stars 6.48k forks source link

[Bug] go agent enhance multiple arguments of the same type #11753

Closed zealotjx closed 7 months ago

zealotjx commented 7 months ago

Search before asking

Apache SkyWalking Component

OAP server (apache/skywalking)

What happened

When using go agent to enhance a function with multiple parameters of the same type, If the function is in the following format: func myFunc(x, y, z string) {}, it can only be obtained in invocation.Args()of Interceptor.BeforeInvoke a parameter x

What you expected to happen

I think we should be able to get the three parameters x, y and z

How to reproduce

Enhance a function like the following func myFunc(x, y, z string) {}

Anything else

I checked the source code and found that when traversing dst.Field.Names, break was used. Should it be continue?

func EnhanceParameterNames(fields *dst.FieldList, fieldType FieldListType) []*ParameterInfo {
    if fields == nil {
        return nil
    }
    result := make([]*ParameterInfo, 0)
    for i, f := range fields.List {
        var defineName string
        switch fieldType {
        case FieldListTypeParam:
            defineName = fmt.Sprintf("skywalking_param_%d", i)
        case FieldListTypeResult:
            defineName = fmt.Sprintf("skywalking_result_%d", i)
        case FieldListTypeRecv:
            defineName = fmt.Sprintf("skywalking_recv_%d", i)
        }
        if len(f.Names) == 0 {
            f.Names = []*dst.Ident{{Name: defineName}}
            result = append(result, newParameterInfo(defineName, f.Type))
        } else {
            for _, n := range f.Names {
                if n.Name == "_" {
                    *n = *dst.NewIdent(defineName)
                    break
                }
            }

            for _, n := range f.Names {
                result = append(result, newParameterInfo(n.Name, f.Type))
                break
            }
        }
    }
    return result
}

Are you willing to submit a pull request to fix on your own?

Code of Conduct

mrproliu commented 7 months ago

For now, I think we only should single name of same type. If you can handle the multiple names, welcome to contribute the codes.

wu-sheng commented 7 months ago

@mrproliu Should we move this to discussion?

mrproliu commented 7 months ago

Sure. It's just enhancement feature I think.