This is an automated email from the ASF dual-hosted git repository.
liuhan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-go.git
The following commit(s) were added to refs/heads/main by this push:
new b7f3404 Fix: Correctly instrument newproc1 for Go 1.23+ parameter
counts (#13188) (#216)
b7f3404 is described below
commit b7f3404dacc0b3ce620d774588153f5a72897094
Author: zhujiajun <[email protected]>
AuthorDate: Wed Apr 23 15:52:09 2025 +0800
Fix: Correctly instrument newproc1 for Go 1.23+ parameter counts (#13188)
(#216)
---
.github/workflows/plugin-tests.yaml | 1 +
CHANGES.md | 1 +
go.work | 1 +
.../plugin.yml => cross-goroutine/bin/startup.sh} | 21 +++------
.../scenarios/cross-goroutine/config/excepted.yml | 50 ++++++++++++++++++++++
test/plugins/scenarios/cross-goroutine/go.mod | 3 ++
.../plugins/scenarios/cross-goroutine/main.go | 43 +++++++++++++------
.../{irisv12 => cross-goroutine}/plugin.yml | 15 +++----
test/plugins/scenarios/irisv12/plugin.yml | 4 +-
tools/go-agent/instrument/api/flags.go | 42 +++++++++++++++++-
tools/go-agent/instrument/runtime/instrument.go | 19 +++-----
11 files changed, 146 insertions(+), 54 deletions(-)
diff --git a/.github/workflows/plugin-tests.yaml
b/.github/workflows/plugin-tests.yaml
index c061238..40801e5 100644
--- a/.github/workflows/plugin-tests.yaml
+++ b/.github/workflows/plugin-tests.yaml
@@ -105,6 +105,7 @@ jobs:
- go-elasticsearchv8
- goframe
- so11y
+ - cross-goroutine
steps:
- uses: actions/checkout@v2
with:
diff --git a/CHANGES.md b/CHANGES.md
index f5185b4..b8a9a4d 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -25,6 +25,7 @@ Release Notes.
* Fix cannot find file when exec build in test/plugins.
* Fix not set span error when http status code >= 400
* Fix http plugin cannot provide peer name when optional Host is empty.
+* Fix Correctly instrument newproc1 for Go 1.23+ parameter counts
#### Issues and PR
- All issues are
[here](https://github.com/apache/skywalking/milestone/219?closed=1)
diff --git a/go.work b/go.work
index 29287ae..320599e 100644
--- a/go.work
+++ b/go.work
@@ -68,6 +68,7 @@ use (
./test/plugins/scenarios/metric-activation
./test/plugins/scenarios/goframe
./test/plugins/scenarios/so11y
+ ./test/plugins/scenarios/cross-goroutine
./tools/go-agent
diff --git a/test/plugins/scenarios/irisv12/plugin.yml
b/test/plugins/scenarios/cross-goroutine/bin/startup.sh
similarity index 68%
copy from test/plugins/scenarios/irisv12/plugin.yml
copy to test/plugins/scenarios/cross-goroutine/bin/startup.sh
index c8e3695..329e7a9 100644
--- a/test/plugins/scenarios/irisv12/plugin.yml
+++ b/test/plugins/scenarios/cross-goroutine/bin/startup.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
@@ -14,18 +16,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-entry-service: http://${HTTP_HOST}:${HTTP_PORT}/consumer
-health-checker: http://${HTTP_HOST}:${HTTP_PORT}/health
-start-script: ./bin/startup.sh
-framework: github.com/kataras/iris/v12
-export-port: 8080
-support-version:
- - go: 1.19
- framework:
- - v12.2.0
- - v12.1.0
- - go: 1.21
- framework:
- - v12.2.5
- - v12.2.0
- - v12.1.0
\ No newline at end of file
+home="$(cd "$(dirname $0)"; pwd)"
+go build ${GO_BUILD_OPTS} -o cross-goroutine
+
+./cross-goroutine
diff --git a/test/plugins/scenarios/cross-goroutine/config/excepted.yml
b/test/plugins/scenarios/cross-goroutine/config/excepted.yml
new file mode 100644
index 0000000..22be6b6
--- /dev/null
+++ b/test/plugins/scenarios/cross-goroutine/config/excepted.yml
@@ -0,0 +1,50 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+segmentItems:
+ - serviceName: cross-goroutine
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: GET:/execute
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5004
+ isError: false
+ spanType: Entry
+ peer: ''
+ skipAnalysis: false
+ tags:
+ - {key: http.method, value: GET}
+ - {key: url, value: 'service:8080/execute'}
+ - {key: status_code, value: '200'}
+ - operationName: testGoroutineLocalSpan
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Unknown
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 0
+ isError: false
+ spanType: Local
+ peer: ''
+ skipAnalysis: false
+meterItems: []
+logItems: []
\ No newline at end of file
diff --git a/test/plugins/scenarios/cross-goroutine/go.mod
b/test/plugins/scenarios/cross-goroutine/go.mod
new file mode 100644
index 0000000..406bd30
--- /dev/null
+++ b/test/plugins/scenarios/cross-goroutine/go.mod
@@ -0,0 +1,3 @@
+module test/plugins/scenarios/cross-goroutine
+
+go 1.19
diff --git a/tools/go-agent/instrument/api/flags.go
b/test/plugins/scenarios/cross-goroutine/main.go
similarity index 51%
copy from tools/go-agent/instrument/api/flags.go
copy to test/plugins/scenarios/cross-goroutine/main.go
index 5ad3ca6..9c7d445 100644
--- a/tools/go-agent/instrument/api/flags.go
+++ b/test/plugins/scenarios/cross-goroutine/main.go
@@ -15,23 +15,38 @@
// specific language governing permissions and limitations
// under the License.
-package api
+package main
-import "path/filepath"
+import (
+ "log"
+ "net/http"
+ "sync"
+ "time"
-type CompileOptions struct {
- Package string `swflag:"-p"`
- Output string `swflag:"-o"`
- AllArgs []string `swflag:"all-args"`
- Lang string `swflag:"-lang"`
+ _ "github.com/apache/skywalking-go"
+ "github.com/apache/skywalking-go/toolkit/trace"
+)
- DebugDir string `swflag:"-debug"` // from tools flag
+func executeHandler(w http.ResponseWriter, r *http.Request) {
+ log.Println("Received request for /execute")
+ var wg sync.WaitGroup
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ trace.CreateLocalSpan("testGoroutineLocalSpan")
+ time.Sleep(100 * time.Millisecond)
+ trace.StopSpan()
+ }()
+ wg.Wait()
+ log.Println("Goroutine finished, sending response")
+ w.WriteHeader(http.StatusOK)
+ _, _ = w.Write([]byte("success"))
}
-func (c *CompileOptions) IsValid() bool {
- return c.Package != "" && c.Output != ""
-}
-
-func (c *CompileOptions) CompileBaseDir() string {
- return filepath.Dir(filepath.Dir(c.Output))
+func main() {
+ http.HandleFunc("/execute", executeHandler)
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request
*http.Request) {
+ writer.WriteHeader(http.StatusOK)
+ })
+ _ = http.ListenAndServe(":8080", nil)
}
diff --git a/test/plugins/scenarios/irisv12/plugin.yml
b/test/plugins/scenarios/cross-goroutine/plugin.yml
similarity index 81%
copy from test/plugins/scenarios/irisv12/plugin.yml
copy to test/plugins/scenarios/cross-goroutine/plugin.yml
index c8e3695..e5f892b 100644
--- a/test/plugins/scenarios/irisv12/plugin.yml
+++ b/test/plugins/scenarios/cross-goroutine/plugin.yml
@@ -14,18 +14,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-entry-service: http://${HTTP_HOST}:${HTTP_PORT}/consumer
+entry-service: http://${HTTP_HOST}:${HTTP_PORT}/execute
health-checker: http://${HTTP_HOST}:${HTTP_PORT}/health
start-script: ./bin/startup.sh
-framework: github.com/kataras/iris/v12
+framework: go
export-port: 8080
support-version:
- go: 1.19
- framework:
- - v12.2.0
- - v12.1.0
+ - go: 1.20
- go: 1.21
- framework:
- - v12.2.5
- - v12.2.0
- - v12.1.0
\ No newline at end of file
+ - go: 1.22
+ - go: 1.23
+ - go: 1.24
diff --git a/test/plugins/scenarios/irisv12/plugin.yml
b/test/plugins/scenarios/irisv12/plugin.yml
index c8e3695..2ca5539 100644
--- a/test/plugins/scenarios/irisv12/plugin.yml
+++ b/test/plugins/scenarios/irisv12/plugin.yml
@@ -23,9 +23,7 @@ support-version:
- go: 1.19
framework:
- v12.2.0
- - v12.1.0
- go: 1.21
framework:
- v12.2.5
- - v12.2.0
- - v12.1.0
\ No newline at end of file
+ - v12.2.0
\ No newline at end of file
diff --git a/tools/go-agent/instrument/api/flags.go
b/tools/go-agent/instrument/api/flags.go
index 5ad3ca6..79321ac 100644
--- a/tools/go-agent/instrument/api/flags.go
+++ b/tools/go-agent/instrument/api/flags.go
@@ -17,7 +17,11 @@
package api
-import "path/filepath"
+import (
+ "path/filepath"
+ "strconv"
+ "strings"
+)
type CompileOptions struct {
Package string `swflag:"-p"`
@@ -35,3 +39,39 @@ func (c *CompileOptions) IsValid() bool {
func (c *CompileOptions) CompileBaseDir() string {
return filepath.Dir(filepath.Dir(c.Output))
}
+
+func (c *CompileOptions) CheckGoVersionGreaterOrEqual(requiredMajor,
requiredMinor int) bool {
+ if c.Lang == "" {
+ return false
+ }
+ if !strings.HasPrefix(c.Lang, "go") {
+ return false
+ }
+ versionStr := strings.TrimPrefix(c.Lang, "go")
+ parts := strings.SplitN(versionStr, ".", 3)
+ if len(parts) < 2 {
+ return false
+ }
+
+ majorStr := parts[0]
+ currentMajor64, err := strconv.ParseInt(majorStr, 10, 64)
+ if err != nil {
+ return false
+ }
+ currentMajor := int(currentMajor64)
+
+ minorStr := parts[1]
+ currentMinor64, err := strconv.ParseInt(minorStr, 10, 64)
+ if err != nil {
+ return false
+ }
+ currentMinor := int(currentMinor64)
+
+ if currentMajor > requiredMajor {
+ return true
+ }
+ if currentMajor == requiredMajor && currentMinor >= requiredMinor {
+ return true
+ }
+ return false
+}
diff --git a/tools/go-agent/instrument/runtime/instrument.go
b/tools/go-agent/instrument/runtime/instrument.go
index cf9adcd..6f98d2a 100644
--- a/tools/go-agent/instrument/runtime/instrument.go
+++ b/tools/go-agent/instrument/runtime/instrument.go
@@ -18,9 +18,6 @@
package runtime
import (
- "strconv"
- "strings"
-
"github.com/dave/dst"
"github.com/dave/dst/dstutil"
@@ -73,7 +70,11 @@ func (r *Instrument) FilterAndEdit(path string, curFile
*dst.File, cursor *dstut
if len(n.Type.Results.List) != 1 {
return false
}
- if len(n.Type.Params.List) != 3 {
+ expectedParamCount := 3
+ if r.opts.CheckGoVersionGreaterOrEqual(1, 23) {
+ expectedParamCount = 5
+ }
+ if len(n.Type.Params.List) != expectedParamCount {
return false
}
parameters := tools.EnhanceParameterNames(n.Type.Params,
tools.FieldListTypeParam)
@@ -106,14 +107,8 @@ func (r *Instrument) AfterEnhanceFile(fromPath, newPath
string) error {
}
func (r *Instrument) parseInternalAtomicPath() string {
- if strings.HasPrefix(r.opts.Lang, "go1.") {
- _, after, found := strings.Cut(r.opts.Lang, ".")
- if found {
- i, err := strconv.ParseInt(after, 10, 64)
- if err == nil && i >= 23 {
- return "internal/runtime/atomic"
- }
- }
+ if r.opts.CheckGoVersionGreaterOrEqual(1, 23) {
+ return "internal/runtime/atomic"
}
return defaultInternalAtomicPath
}
