Fix: Correctly instrument newproc1 for Go 1.23+ parameter counts (#13188)

.github/workflows/plugin-tests.yaml

@@ -105,6 +105,7 @@ jobs:
           - go-elasticsearchv8
           - goframe
           - so11y
+          - cross-goroutine
     steps:
       - uses: actions/checkout@v2
         with:

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)

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
 

test/plugins/scenarios/cross-goroutine/bin/startup.sh

@@ -0,0 +1,22 @@
+#!/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
+# 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.
+
+home="$(cd "$(dirname $0)"; pwd)"
+go build ${GO_BUILD_OPTS} -o cross-goroutine
+
+./cross-goroutine

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: []

test/plugins/scenarios/cross-goroutine/go.mod

@@ -0,0 +1,3 @@
+module test/plugins/scenarios/cross-goroutine
+
+go 1.19

test/plugins/scenarios/cross-goroutine/main.go

@@ -0,0 +1,52 @@
+// Licensed to 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. Apache Software Foundation (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.
+
+package main
+
+import (
+	"log"
+	"net/http"
+	"sync"
+	"time"
+
+	_ "github.com/apache/skywalking-go"
+	"github.com/apache/skywalking-go/toolkit/trace"
+)
+
+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 main() {
+	http.HandleFunc("/execute", executeHandler)
+	http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+		writer.WriteHeader(http.StatusOK)
+	})
+	_ = http.ListenAndServe(":8080", nil)
+}

test/plugins/scenarios/cross-goroutine/plugin.yml

@@ -0,0 +1,28 @@
+# 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.
+
+entry-service: http://${HTTP_HOST}:${HTTP_PORT}/execute
+health-checker: http://${HTTP_HOST}:${HTTP_PORT}/health
+start-script: ./bin/startup.sh
+framework: go
+export-port: 8080
+support-version:
+  - go: 1.19
+  - go: 1.20
+  - go: 1.21
+  - go: 1.22
+  - go: 1.23
+  - go: 1.24

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
+      - v12.2.0

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
+}

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
 }