You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "mrproliu (via GitHub)" <gi...@apache.org> on 2023/04/22 13:57:39 UTC

[GitHub] [skywalking-go] mrproliu opened a new pull request, #12: Support native http-client plugin

mrproliu opened a new pull request, #12:
URL: https://github.com/apache/skywalking-go/pull/12

   Support native HTTP-client plugin, and fix some template bugs when intercepted methods have results.
   
   Here are screenshots when sending traffic to services(consumer and provider):
   
   <img width="1017" alt="image" src="https://user-images.githubusercontent.com/3417650/233789196-95178f5a-378f-4651-a392-892a020a2aae.png">
   <img width="1061" alt="image" src="https://user-images.githubusercontent.com/3417650/233789206-6e13c281-0b4d-4f5c-ba63-7ca042527ea8.png">
   <img width="1125" alt="image" src="https://user-images.githubusercontent.com/3417650/233789215-f2d90322-491c-4efe-b0c8-255a6cd029a7.png">
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174485919


##########
tools/go-agent/instrument/plugins/instrument.go:
##########
@@ -362,21 +362,25 @@ func (i *Instrument) validateMethodInsMatch(matcher *instrument.EnhanceMatcher,
 	return true
 }
 
-func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, pkg string) (string, error) {
+func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, ins instrument.Instrument) (string, error) {
 	for _, arg := range opts.AllArgs {
 		// find the go file
 		if !strings.HasSuffix(arg, ".go") {
 			continue
 		}
+		basePkg := ins.BasePackage()
 
-		parts := strings.SplitN(arg, pkg, 2)
+		parts := strings.SplitN(arg, basePkg, 2)
 		// example: github.com/gin-gonic/gin@1.1.1/gin.go
-		if len(parts) != 2 || !strings.HasPrefix(parts[1], "@") {
-			return "", fmt.Errorf("could not found the go version of the package %s, go file path: %s", pkg, arg)
+		if len(parts) != 2 {
+			return "", fmt.Errorf("could not found the go version of the package %s, go file path: %s", basePkg, arg)
+		}
+		if !strings.HasPrefix(parts[1], "@") {
+			return "", nil
 		}
 		firstDir := strings.Index(parts[1], "/")

Review Comment:
   Got it. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] jcchavezs commented on pull request #12: Support native http-client plugin

Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#issuecomment-1518703232

   I wonder why this PR does not have tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174484238


##########
plugins/http/instrument.go:
##########
@@ -0,0 +1,62 @@
+// 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 http
+
+import (
+	"embed"
+
+	"github.com/apache/skywalking-go/plugins/core/instrument"
+)
+
+//go:embed *

Review Comment:
   It's better to embed all files, then let agent tools decide which file needs to copy. 
   Native HTTP client is a simple case for the plugin. If the plugins need to monitor multiple packages, the embed files may be difficult to describe there.  



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] jcchavezs commented on a diff in pull request #12: Support native http-client plugin

Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1175764572


##########
plugins/http/instrument.go:
##########
@@ -0,0 +1,62 @@
+// 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 http
+
+import (
+	"embed"
+
+	"github.com/apache/skywalking-go/plugins/core/instrument"
+)
+
+//go:embed *

Review Comment:
   Thanks for the clarification.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] wu-sheng merged pull request #12: Support native http-client plugin

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #12:
URL: https://github.com/apache/skywalking-go/pull/12


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] wu-sheng commented on a diff in pull request #12: Support native http-client plugin

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1175913561


##########
plugins/http/intercepter.go:
##########
@@ -0,0 +1,66 @@
+// 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 http
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/apache/skywalking-go/plugins/core/operator"
+	"github.com/apache/skywalking-go/plugins/core/tracing"
+)
+
+type Interceptor struct {
+}
+
+func (h *Interceptor) BeforeInvoke(invocation *operator.Invocation) error {
+	request := invocation.Args[0].(*http.Request)
+	s, err := tracing.CreateExitSpan(fmt.Sprintf("%s:%s", request.Method, request.URL.Path), request.Host, func(headerKey, headerValue string) error {
+		request.Header.Add(headerKey, headerValue)
+		return nil
+	}, tracing.WithLayer(tracing.SpanLayerHTTP),
+		tracing.WithTag(tracing.TagHTTPMethod, request.Method),
+		tracing.WithTag(tracing.TagURL, request.Host+request.URL.Path),
+		tracing.WithComponent(5005))
+	if err != nil {
+		return err
+	}
+	invocation.Context = s
+	return nil
+}
+
+func (h *Interceptor) AfterInvoke(invocation *operator.Invocation, result ...interface{}) error {
+	if invocation.Context == nil {
+		return nil
+	}
+	span := invocation.Context.(tracing.Span)

Review Comment:
   Let's run the agent level performance test first. (Benchmark will show difference for sure)
   If we really have a performance impact, `nil` is even better than the `NoopSpan`. After all, noop concept is to avoid all checks. Let's see.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] jcchavezs commented on a diff in pull request #12: Support native http-client plugin

Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174428911


##########
plugins/http/intercepter.go:
##########
@@ -0,0 +1,66 @@
+// 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 http
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/apache/skywalking-go/plugins/core/operator"
+	"github.com/apache/skywalking-go/plugins/core/tracing"
+)
+
+type Interceptor struct {
+}
+
+func (h *Interceptor) BeforeInvoke(invocation *operator.Invocation) error {
+	request := invocation.Args[0].(*http.Request)
+	s, err := tracing.CreateExitSpan(fmt.Sprintf("%s:%s", request.Method, request.URL.Path), request.Host, func(headerKey, headerValue string) error {
+		request.Header.Add(headerKey, headerValue)
+		return nil
+	}, tracing.WithLayer(tracing.SpanLayerHTTP),
+		tracing.WithTag(tracing.TagHTTPMethod, request.Method),
+		tracing.WithTag(tracing.TagURL, request.Host+request.URL.Path),
+		tracing.WithComponent(5005))
+	if err != nil {
+		return err
+	}
+	invocation.Context = s
+	return nil
+}
+
+func (h *Interceptor) AfterInvoke(invocation *operator.Invocation, result ...interface{}) error {
+	if invocation.Context == nil {
+		return nil
+	}
+	span := invocation.Context.(tracing.Span)

Review Comment:
   Shall we check if span is noop to avoid function calls?



##########
tools/go-agent/tools/enhancement.go:
##########
@@ -25,10 +25,18 @@ import (
 	"github.com/dave/dst/decorator"
 )
 
+var interfaceName = "interface{}"

Review Comment:
   I think const may play better here.



##########
tools/go-agent/instrument/plugins/instrument.go:
##########
@@ -68,7 +68,7 @@ func (i *Instrument) CouldHandle(opts *api.CompileOptions) bool {
 			continue
 		}
 		// check the version of the framework could handler
-		version, err := i.tryToFindThePluginVersion(opts, ins.BasePackage())
+		version, err := i.tryToFindThePluginVersion(opts, ins)
 		if err != nil {
 			logrus.Warnf("ignore the plugin %s, because: %s", ins.Name(), err)

Review Comment:
   I would avoid this interpolation and use fields instead. Interpolation and this logging style causes unnecessary allocations.



##########
tools/go-agent/instrument/plugins/instrument.go:
##########
@@ -362,21 +362,25 @@ func (i *Instrument) validateMethodInsMatch(matcher *instrument.EnhanceMatcher,
 	return true
 }
 
-func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, pkg string) (string, error) {
+func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, ins instrument.Instrument) (string, error) {
 	for _, arg := range opts.AllArgs {
 		// find the go file
 		if !strings.HasSuffix(arg, ".go") {
 			continue
 		}
+		basePkg := ins.BasePackage()
 
-		parts := strings.SplitN(arg, pkg, 2)
+		parts := strings.SplitN(arg, basePkg, 2)
 		// example: github.com/gin-gonic/gin@1.1.1/gin.go
-		if len(parts) != 2 || !strings.HasPrefix(parts[1], "@") {
-			return "", fmt.Errorf("could not found the go version of the package %s, go file path: %s", pkg, arg)
+		if len(parts) != 2 {
+			return "", fmt.Errorf("could not found the go version of the package %s, go file path: %s", basePkg, arg)
+		}
+		if !strings.HasPrefix(parts[1], "@") {
+			return "", nil
 		}
 		firstDir := strings.Index(parts[1], "/")

Review Comment:
   I know it isn't part of the PR but I think strings.Cut would help.



##########
tools/go-agent/instrument/plugins/instrument.go:
##########
@@ -362,21 +362,25 @@ func (i *Instrument) validateMethodInsMatch(matcher *instrument.EnhanceMatcher,
 	return true
 }
 
-func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, pkg string) (string, error) {
+func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, ins instrument.Instrument) (string, error) {
 	for _, arg := range opts.AllArgs {
 		// find the go file
 		if !strings.HasSuffix(arg, ".go") {
 			continue
 		}
+		basePkg := ins.BasePackage()
 
-		parts := strings.SplitN(arg, pkg, 2)
+		parts := strings.SplitN(arg, basePkg, 2)

Review Comment:
   Better to use strings.Cut to avoid checking if the parts are actually 2.



##########
tools/go-agent/tools/types.go:
##########
@@ -0,0 +1,26 @@
+// 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 tools
+
+// nolint
+func IsBasicDataType(name string) bool {

Review Comment:
   I wonder if using a map[string]struct{} is more performant here. Worth to add benchmarks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174484543


##########
plugins/http/intercepter.go:
##########
@@ -0,0 +1,66 @@
+// 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 http
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/apache/skywalking-go/plugins/core/operator"
+	"github.com/apache/skywalking-go/plugins/core/tracing"
+)
+
+type Interceptor struct {
+}
+
+func (h *Interceptor) BeforeInvoke(invocation *operator.Invocation) error {
+	request := invocation.Args[0].(*http.Request)
+	s, err := tracing.CreateExitSpan(fmt.Sprintf("%s:%s", request.Method, request.URL.Path), request.Host, func(headerKey, headerValue string) error {
+		request.Header.Add(headerKey, headerValue)
+		return nil
+	}, tracing.WithLayer(tracing.SpanLayerHTTP),
+		tracing.WithTag(tracing.TagHTTPMethod, request.Method),
+		tracing.WithTag(tracing.TagURL, request.Host+request.URL.Path),
+		tracing.WithComponent(5005))
+	if err != nil {
+		return err
+	}
+	invocation.Context = s
+	return nil
+}
+
+func (h *Interceptor) AfterInvoke(invocation *operator.Invocation, result ...interface{}) error {
+	if invocation.Context == nil {
+		return nil
+	}
+	span := invocation.Context.(tracing.Span)

Review Comment:
   Sure, but in this way the interceptor code could be more complex. 
   In the noop span, the user still needs to call `span.End()` to ensure the span is closed.  
   @wu-sheng WDYT?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#issuecomment-1518908377

   > I wonder why this PR does not have tests.
   
   For the quick preview, I haven't done too many tests, only have some basic tracing UT for now.
   I will add some plugin tests and UT in this milestone. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174485986


##########
tools/go-agent/tools/enhancement.go:
##########
@@ -25,10 +25,18 @@ import (
 	"github.com/dave/dst/decorator"
 )
 
+var interfaceName = "interface{}"

Review Comment:
   Sure.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174486000


##########
tools/go-agent/tools/types.go:
##########
@@ -0,0 +1,26 @@
+// 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 tools
+
+// nolint
+func IsBasicDataType(name string) bool {

Review Comment:
   Sure, I will update it. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174485222


##########
tools/go-agent/instrument/plugins/instrument.go:
##########
@@ -362,21 +362,25 @@ func (i *Instrument) validateMethodInsMatch(matcher *instrument.EnhanceMatcher,
 	return true
 }
 
-func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, pkg string) (string, error) {
+func (i *Instrument) tryToFindThePluginVersion(opts *api.CompileOptions, ins instrument.Instrument) (string, error) {
 	for _, arg := range opts.AllArgs {
 		// find the go file
 		if !strings.HasSuffix(arg, ".go") {
 			continue
 		}
+		basePkg := ins.BasePackage()
 
-		parts := strings.SplitN(arg, pkg, 2)
+		parts := strings.SplitN(arg, basePkg, 2)

Review Comment:
   Sure, I will fix it. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #12: Support native http-client plugin

Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174485073


##########
tools/go-agent/instrument/plugins/instrument.go:
##########
@@ -68,7 +68,7 @@ func (i *Instrument) CouldHandle(opts *api.CompileOptions) bool {
 			continue
 		}
 		// check the version of the framework could handler
-		version, err := i.tryToFindThePluginVersion(opts, ins.BasePackage())
+		version, err := i.tryToFindThePluginVersion(opts, ins)
 		if err != nil {
 			logrus.Warnf("ignore the plugin %s, because: %s", ins.Name(), err)

Review Comment:
   Do you mean logging with a function call? Have any good way for this type of logging?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [skywalking-go] jcchavezs commented on a diff in pull request #12: Support native http-client plugin

Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on code in PR #12:
URL: https://github.com/apache/skywalking-go/pull/12#discussion_r1174438116


##########
plugins/http/instrument.go:
##########
@@ -0,0 +1,62 @@
+// 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 http
+
+import (
+	"embed"
+
+	"github.com/apache/skywalking-go/plugins/core/instrument"
+)
+
+//go:embed *

Review Comment:
   Shall we embed all files or just the templates?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org