feat: toolkit span add event impl

[ShyunnY]

What type of PR?

feat: toolkit span add event api impl

What did you do?

Completed the implementation of the AddEvent API in toolkit/span. Support attaching Event to Span in manual mode.

[wu-sheng]

Please make sure CI pass.

[ShyunnY]

@mrproliu

When we use a separate const.go file to define constants, there will be a problem: the variable name in the interceptor does not correspond to the const variable name.

We can view the mixed compiled code
const.go:

package trace

// Event related constant definitions
const (
        skywalking_operatorVarTracedefaultEventType     = "info"
        skywalking_operatorVarTracedefaultEmptyEvent    = "unknown"
)

add_event.go:

package trace

type skywalking_operatorTypeTraceAddEventInterceptor struct {
}

func (h *skywalking_operatorTypeTraceAddEventInterceptor) BeforeInvoke(invocation skywalking_operatorTypeOperatorInvocation) error {
        ......
        if span != nil {
                if et == "" {
                        et = skywalking_operatorTypeTracedefaultEventType
                }
                if event == "" {
                        event = skywalking_operatorTypeTracedefaultEmptyEvent
                }
 
        }
        ......
        return nil
}

async_event.go:

func (h *skywalking_operatorTypeTraceAsyncAddEventInterceptor) AfterInvoke(invocation skywalking_operatorTypeOperatorInvocation, _ ...interface{}) error {
        ......
        if et == "" {
                et = skywalking_operatorTypeTracedefaultEventType
        }
        event := invocation.Args()[1].(string)
        if event == "" {
                event = skywalking_operatorTypeTracedefaultEmptyEvent
        }
        ......
        return nil
}

After analysis, this is because when go-agent parses AssignStmt, it parses Rhs into Ident, and Ident lacks corresponding information to tell go-agent how to modify the mapping relationship.
This is a problem that cannot be solved at this stage, so we still choose to define temporary variables directly in the function stack.

WDYT?

[mrproliu]

Please check these code: https://github.com/apache/skywalking-go/blob/main/tools/go-agent/instrument/plugins/rewrite/rewrite.go#L126-L155
The go agent will check all top-level DST nodes, and add there name mapping. Maybe there missing const token check? I think you need to check in there.

[ShyunnY]

Please check these code: https://github.com/apache/skywalking-go/blob/main/tools/go-agent/instrument/plugins/rewrite/rewrite.go#L126-L155 The go agent will check all top-level DST nodes, and add there name mapping. Maybe there missing const token check? I think you need to check in there.

I have done relevant checks, as I said above. When enhancing add_event.go,async_add_event.go, AST cannot determine the type of LRs and can only perform the default enhancement.

[mrproliu]

Can we add a var mapping check when rewriting LRs data?

[ShyunnY]

In LRs Ident, it can be Const or Var. But we don’t know these, so we can’t map them.

[mrproliu]

We have already mapping all var and const, so we know all about it in the rewrite files. Why we can't rewrite about this?

[ShyunnY]

For example, the following code:

if et == "" {
et = defaultEventType
}

The corresponding AST result is

"Rhs": [
{
"Loc": {
"End": {
"Filename": "main.go",
"Offset": 407,
"Line": 16,
"Column": 25
},
"Start": {
"Filename": "main.go",
"Offset": 391,
"Line": 16,
"Column": 9
}
},
"Name": "defaultEventType",   # <--- we know
"_type": "Ident"
}

When we rewrite, we need to determine its type, but the AST lacks the type and can only know the value of Ident

[ShyunnY]

Therefore, in the rewrite phase, it can only perform the default rewrite and cannot match it. You can try it.

To solve this problem, I rebuilt the code in the interceptor to avoid introducing too many external dependencies. I think this is a good way

[mrproliu]

Read the rewrite logic carefully in go agent, when Ident type is involved, the rewrite function will look for an existing name in mapping: https://github.com/apache/skywalking-go/blob/main/tools/go-agent/instrument...plugins...rewrite..context.go#L164-L175

And I have tried, you need to check the const and adding them to the var rewrite strategy, that's enough: https://github.com/apache/skywalking-go/blob/main/tools/go-agent/instrument/plugins/rewrite/rewrite.go#L126-L155

[ShyunnY]

I added a small feature to the original rewrite, which prevents it from rewriting when the type has SWNoRewrite suffix. This mainly solves the following problems:

• Since EventType is defined under the toolkit/trace package, we need to assert it in the interceptor, but because of the existence of rewrite, it will cause assertion errors during runtime (the type is not an EventType).

So we prevent it from rewriting, which ensures that when mixed compilation, the EventType type of the interceptor can point to the type defined in the toolkit/trace api.

By the way: we can also use this when implementing other functions later, such as metrics and logs!