diff --git a/.github/workflows/plugin-tests.yaml b/.github/workflows/plugin-tests.yaml index c061238d..40801e52 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 f5185b4c..b8a9a4de 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 29287ae1..320599e2 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/cross-goroutine/bin/startup.sh b/test/plugins/scenarios/cross-goroutine/bin/startup.sh new file mode 100644 index 00000000..329e7a9f --- /dev/null +++ b/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 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 00000000..22be6b6e --- /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 00000000..406bd308 --- /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/test/plugins/scenarios/cross-goroutine/main.go b/test/plugins/scenarios/cross-goroutine/main.go new file mode 100644 index 00000000..9c7d4457 --- /dev/null +++ b/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) +} diff --git a/test/plugins/scenarios/cross-goroutine/plugin.yml b/test/plugins/scenarios/cross-goroutine/plugin.yml new file mode 100644 index 00000000..e5f892bf --- /dev/null +++ b/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 diff --git a/test/plugins/scenarios/irisv12/plugin.yml b/test/plugins/scenarios/irisv12/plugin.yml index c8e36959..2ca55395 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 5ad3ca6d..79321acd 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 cf9adcd6..6f98d2a6 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 }