You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "ethan256 (via GitHub)" <gi...@apache.org> on 2023/07/17 03:42:49 UTC
[GitHub] [skywalking-go] ethan256 opened a new pull request, #74: Support echo instrument
ethan256 opened a new pull request, #74:
URL: https://github.com/apache/skywalking-go/pull/74
support [echo](github.com/labstack/echo/v4) instrumentation
--
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] ethan256 commented on a diff in pull request #74: Support echo instrument
Posted by "ethan256 (via GitHub)" <gi...@apache.org>.
ethan256 commented on code in PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#discussion_r1264933512
##########
plugins/echov4/interceptor.go:
##########
@@ -0,0 +1,80 @@
+// 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 echo
+
+import (
+ "fmt"
+
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+
+ "github.com/labstack/echo/v4"
+)
+
+type Interceptor struct{}
+
+// BeforeInvoke would be called before the target method invocation.
+func (h *Interceptor) BeforeInvoke(invocation operator.Invocation) error {
+ return nil
+}
+
+// AfterInvoke would be called after the target method invocation.
+func (h *Interceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ if len(result) != 1 {
+ return fmt.Errorf("echo.New :skywalking return result length not equal 1")
+ }
+ e, ok := result[0].(*echo.Echo)
+ if !ok {
+ return fmt.Errorf("echo :skywalking cannot create middleware for echo not match *Echo: %T", e)
+ }
+
+ e.Use(middleware())
+ return nil
+}
+
+func middleware() echo.MiddlewareFunc {
+ return func(next echo.HandlerFunc) echo.HandlerFunc {
+ return func(c echo.Context) error {
+ request := c.Request()
+ span, err := tracing.CreateEntrySpan(
+ fmt.Sprintf("%s:%s", request.Method, c.Path()), func(headerKey string) (string, error) {
+ return request.Header.Get(headerKey), nil
+ },
+ tracing.WithLayer(tracing.SpanLayerHTTP),
+ tracing.WithTag(tracing.TagHTTPMethod, request.Method),
+ tracing.WithTag(tracing.TagURL, request.Host+request.URL.Path),
+ tracing.WithComponent(5006))
Review Comment:
Ok, I'll register a unique component ID with the master library in another [PR](https://github.com/apache/skywalking/pull/11102)
--
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 #74: Support echo instrument
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#discussion_r1264818511
##########
go.work:
##########
@@ -6,16 +6,17 @@ use (
// define the plugins
./plugins/core
./plugins/dubbo
+ ./plugins/echov4
Review Comment:
Please keep the original style.
##########
plugins/echov4/interceptor.go:
##########
@@ -0,0 +1,80 @@
+// 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 echo
+
+import (
+ "fmt"
+
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+
+ "github.com/labstack/echo/v4"
+)
+
+type Interceptor struct{}
+
+// BeforeInvoke would be called before the target method invocation.
+func (h *Interceptor) BeforeInvoke(invocation operator.Invocation) error {
+ return nil
+}
+
+// AfterInvoke would be called after the target method invocation.
+func (h *Interceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ if len(result) != 1 {
+ return fmt.Errorf("echo.New :skywalking return result length not equal 1")
+ }
+ e, ok := result[0].(*echo.Echo)
+ if !ok {
+ return fmt.Errorf("echo :skywalking cannot create middleware for echo not match *Echo: %T", e)
+ }
+
+ e.Use(middleware())
+ return nil
+}
+
+func middleware() echo.MiddlewareFunc {
+ return func(next echo.HandlerFunc) echo.HandlerFunc {
+ return func(c echo.Context) error {
+ request := c.Request()
+ span, err := tracing.CreateEntrySpan(
+ fmt.Sprintf("%s:%s", request.Method, c.Path()), func(headerKey string) (string, error) {
+ return request.Header.Get(headerKey), nil
+ },
+ tracing.WithLayer(tracing.SpanLayerHTTP),
+ tracing.WithTag(tracing.TagHTTPMethod, request.Method),
+ tracing.WithTag(tracing.TagURL, request.Host+request.URL.Path),
+ tracing.WithComponent(5006))
Review Comment:
The component ID should be updated?
##########
plugins/echov4/interceptor.go:
##########
@@ -0,0 +1,80 @@
+// 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 echo
+
+import (
+ "fmt"
+
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+
+ "github.com/labstack/echo/v4"
+)
+
+type Interceptor struct{}
+
+// BeforeInvoke would be called before the target method invocation.
+func (h *Interceptor) BeforeInvoke(invocation operator.Invocation) error {
+ return nil
+}
+
+// AfterInvoke would be called after the target method invocation.
+func (h *Interceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ if len(result) != 1 {
+ return fmt.Errorf("echo.New :skywalking return result length not equal 1")
+ }
Review Comment:
You have already defined this method as one result, so this judgment is not necessary.
##########
plugins/echov4/go.mod:
##########
@@ -0,0 +1,21 @@
+module github.com/apache/skywalking-go/plugins/echov4
+
+go 1.20
+
+require (
+ github.com/apache/skywalking-go/plugins/core v0.0.0-20230712143836-5d2a80f0e1f5
Review Comment:
We already using the go workspace, so this module no needs to be required.
--
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 #74: Support echo instrument
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1665486467
Also, please resolve the conflict and conversations.
--
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] ethan256 commented on pull request #74: Support echo instrument
Posted by "ethan256 (via GitHub)" <gi...@apache.org>.
ethan256 commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1639125076
> Could you provide a correct link to echo framework? Is it a rpc or something?
It is a web framework, see https://github.com/labstack/echo
--
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 pull request #74: Support echo instrument
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1666444531
Close for now, it seems contributor has no interest to follow reviewers comments.
--
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 #74: Support echo instrument
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#discussion_r1265102161
##########
go.work:
##########
@@ -6,16 +6,17 @@ use (
// define the plugins
./plugins/core
./plugins/dubbo
+ ./plugins/echo
Review Comment:
Why did you change to the `echo` otherwise `echov4`?
--
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] ethan256 commented on pull request #74: Support echo instrument
Posted by "ethan256 (via GitHub)" <gi...@apache.org>.
ethan256 commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1665357298
> @ethan256 Any update?
Sorry, I'm not familiar with certificates here and didn't understand what I needed to do.
--
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 pull request #74: Support echo instrument
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1638484422
Could you provide a correct link to echo framework? Is it a rpc or something?
--
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 pull request #74: Support echo instrument
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1665359513
You are instrumenting a server lib, so generally, you should add the project logo to our website.
--
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 #74: Support echo instrument
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#discussion_r1265102755
##########
plugins/echov4/interceptor.go:
##########
@@ -0,0 +1,80 @@
+// 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 echo
+
+import (
+ "fmt"
+
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+
+ "github.com/labstack/echo/v4"
+)
+
+type Interceptor struct{}
+
+// BeforeInvoke would be called before the target method invocation.
+func (h *Interceptor) BeforeInvoke(invocation operator.Invocation) error {
+ return nil
+}
+
+// AfterInvoke would be called after the target method invocation.
+func (h *Interceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ if len(result) != 1 {
+ return fmt.Errorf("echo.New :skywalking return result length not equal 1")
+ }
+ e, ok := result[0].(*echo.Echo)
+ if !ok {
+ return fmt.Errorf("echo :skywalking cannot create middleware for echo not match *Echo: %T", e)
+ }
+
+ e.Use(middleware())
+ return nil
+}
+
+func middleware() echo.MiddlewareFunc {
+ return func(next echo.HandlerFunc) echo.HandlerFunc {
+ return func(c echo.Context) error {
+ request := c.Request()
+ span, err := tracing.CreateEntrySpan(
+ fmt.Sprintf("%s:%s", request.Method, c.Path()), func(headerKey string) (string, error) {
+ return request.Header.Get(headerKey), nil
+ },
+ tracing.WithLayer(tracing.SpanLayerHTTP),
+ tracing.WithTag(tracing.TagHTTPMethod, request.Method),
+ tracing.WithTag(tracing.TagURL, request.Host+request.URL.Path),
+ tracing.WithComponent(5006))
Review Comment:
The PR has been merged. Please keep fixing other issues.
--
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] ethan256 commented on a diff in pull request #74: Support echo instrument
Posted by "ethan256 (via GitHub)" <gi...@apache.org>.
ethan256 commented on code in PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#discussion_r1266071711
##########
go.work:
##########
@@ -6,16 +6,17 @@ use (
// define the plugins
./plugins/core
./plugins/dubbo
+ ./plugins/echo
Review Comment:
I'm trying to keep it consistent with the original library. I will change this to echov4.
--
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 pull request #74: Support echo instrument
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1665205548
@ethan256 Any update?
--
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] ethan256 commented on pull request #74: Support echo instrument
Posted by "ethan256 (via GitHub)" <gi...@apache.org>.
ethan256 commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1665349902
>
No
--
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 closed pull request #74: Support echo instrument
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng closed pull request #74: Support echo instrument
URL: https://github.com/apache/skywalking-go/pull/74
--
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 pull request #74: Support echo instrument
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #74:
URL: https://github.com/apache/skywalking-go/pull/74#issuecomment-1639277307
It is like a MIT licensed lib, do you need an icon for it on the topo? If so, please send a PR for that as well, otherwise, by default, it would be an earth icon only for server side.
--
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