Skip to content

feat(add trace profile for go agent) #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat(add trace profile for go agent) #229

wants to merge 16 commits into from

Conversation

cqhasy
Copy link

@cqhasy cqhasy commented Aug 14, 2025

No description provided.

@mrproliu mrproliu self-requested a review August 14, 2025 02:40
@mrproliu mrproliu added the enhancement New feature or request label Aug 14, 2025
@mrproliu mrproliu added this to the 0.7.0 milestone Aug 14, 2025
_ "strconv"
_ "strings"
_ "sync"
_ "time"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the empty line, it could help with reading the code.

_ "github.com/apache/skywalking-go/agent/core/operator"
_ "github.com/apache/skywalking-go/log"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here, keep the empty line

@@ -1,20 +1,29 @@
module github.com/apache/skywalking-go

go 1.19
go 1.24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the agent need to update to 1.24? The user may use the older go version and it will break the user dependency.

toolchain go1.24.4

replace (
skywalking.apache.org/repo/goapi => ../skywalking-goapi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the goapi back to the original version.


replace (
skywalking.apache.org/repo/goapi => ../skywalking-goapi
)

require (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature no needs to update the dependency, please roll back. Only goapi should be updated.

@@ -42,6 +42,7 @@ func CreateEntrySpan(operationName string, extractor Extractor, opts ...SpanOpti
if err != nil {
return nil, err
}
//id := span.(AdaptSpan).GetTraceID()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the comment line.

@@ -60,7 +60,8 @@ func middleware() echo.MiddlewareFunc {
if err != nil {
return err
}

id := span.TraceID()
fmt.Println("trace_id:", id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fmt.Println should be remove.

@@ -41,7 +41,6 @@ func (h *ServerInterceptor) BeforeInvoke(invocation operator.Invocation) error {
if err != nil {
return err
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the empty line.

@@ -193,7 +193,7 @@ func {{.InitFuncName}}(logger operator.LogOperator) (Reporter, error) {
return NewDiscardReporter(), nil
}
checkIntervalVal := {{.Config.Reporter.CheckInterval.ToGoIntValue "the reporter check interval must be number"}}
checkInterval := time.Second * time.Duration(checkIntervalVal)
checkInterval := time.Second * time.Duration(checkIntervalVal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between this line?

@@ -235,15 +235,17 @@ func initManager(logger operator.LogOperator, checkInterval time.Duration) (*Con
const grpcReporterInitFunc = `

func initGRPCReporter(logger operator.LogOperator,
checkInterval time.Duration,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the original format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants