You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "zjh129 (via GitHub)" <gi...@apache.org> on 2024/10/16 07:36:51 UTC
[PR] gozero framework plugin support. [skywalking-go]
zjh129 opened a new pull request, #209:
URL: https://github.com/apache/skywalking-go/pull/209
feat(go-zero): framework plugin support.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1805788982
##########
test/plugins/scenarios/gozero/config/excepted.yml:
##########
@@ -0,0 +1,136 @@
+# 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: gozero
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: GET:/user/info
+ parentSpanId: -1
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Entry
+ peer: ''
+ skipAnalysis: false
+ tags:
+ - {key: http.method, value: GET}
+ - {key: url, value: 'localhost:8888/user/info'}
+ - {key: http.params, value: not null}
+ - {key: status_code, value: '200'}
+ refs:
+ - {parentEndpoint: 'GET:/user/info?id=1', networkAddress: 'localhost:8080', refType: CrossProcess,
+ parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not null,
+ parentService: gozero, traceId: not null}
+ - operationName: logx.info
+ parentSpanId: 1
+ spanId: 2
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'user api find: &{Id:0 Uuid:}'}
+ - operationName: logx.error
+ parentSpanId: 1
+ spanId: 3
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'test error log'}
+ - operationName: user.User.UserFind
+ parentSpanId: 1
+ spanId: 4
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Exit
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: user.User.UserFind/Client/Request/SendMsg
+ parentSpanId: 4
+ spanId: 5
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind/Client/Response/RecvMsg
+ parentSpanId: 4
+ spanId: 6
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind
+ parentSpanId: 4
+ spanId: 7
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Entry
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: /user.User/UserFind
Review Comment:
For the additional package import, please read this document to know the [implementation key principles](https://skywalking.apache.org/docs/skywalking-go/next/en/concepts-and-designs/key-principles/).
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1803978562
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,70 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// SkyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
I know this place. I encountered an issue where I couldn't use extra packages while developing the interceptor functionality; otherwise, I get the error: `skywalking_enhance_rest_server_middleware_intercepter.go:4:2: could not import encoding/json`. I am currently waiting for a solution.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804043835
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,70 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// SkyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
The interceptor is currently unable to use packages other than enhanced objects, so we will remove the functionality for collecting request parameters and response data and will submit a PR once support is available in the future.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804043179
##########
plugins/gozero/zrpc/server_middleware_intercepter.go:
##########
@@ -0,0 +1,56 @@
+package zrpc
+
+import (
+ "context"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/zrpc"
+ "google.golang.org/grpc"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// BeforeInvoke intercepts the HTTP request before invoking the handler.
+func (h *ServerMiddlewareInterceptor) BeforeInvoke(invocation operator.Invocation) error {
+ server := invocation.CallerInstance().(*zrpc.RpcServer)
+ server.AddUnaryInterceptors(RpcServeInterceptor(invocation))
+ return nil
+}
+
+// AfterInvoke processes after the HTTP request has been handled.
+func (h *ServerMiddlewareInterceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ return nil
+}
+
+// RpcServeInterceptor is a grpc server interceptor that creates a new span for each incoming request.
+var RpcServeInterceptor = func(invocation operator.Invocation) grpc.UnaryServerInterceptor {
+ return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) {
+ s, err := tracing.CreateLocalSpan(info.FullMethod,
+ tracing.WithLayer(tracing.SpanLayerRPCFramework),
+ tracing.WithTag(tracing.TagURL, info.FullMethod),
+ tracing.WithComponent(5023),
+ )
+ if err != nil {
+ return nil, err
+ }
+ defer s.End()
+
+ resp, err = handler(ctx, req)
+ if err != nil {
+ s.Error(err.Error())
+ }
+ // collect request and response data
+ if config.CollectRequestParameters {
+ //TODO: collect request data
Review Comment:
The interceptor is currently unable to use packages other than enhanced objects, so we will remove the functionality for collecting request parameters and response data and will submit a PR once support is available in the future.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1805790144
##########
test/plugins/scenarios/gozero/config/excepted.yml:
##########
@@ -0,0 +1,136 @@
+# 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: gozero
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: GET:/user/info
+ parentSpanId: -1
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Entry
+ peer: ''
+ skipAnalysis: false
+ tags:
+ - {key: http.method, value: GET}
+ - {key: url, value: 'localhost:8888/user/info'}
+ - {key: http.params, value: not null}
+ - {key: status_code, value: '200'}
+ refs:
+ - {parentEndpoint: 'GET:/user/info?id=1', networkAddress: 'localhost:8080', refType: CrossProcess,
+ parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not null,
+ parentService: gozero, traceId: not null}
+ - operationName: logx.info
+ parentSpanId: 1
+ spanId: 2
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'user api find: &{Id:0 Uuid:}'}
+ - operationName: logx.error
+ parentSpanId: 1
+ spanId: 3
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'test error log'}
+ - operationName: user.User.UserFind
+ parentSpanId: 1
+ spanId: 4
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Exit
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: user.User.UserFind/Client/Request/SendMsg
+ parentSpanId: 4
+ spanId: 5
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind/Client/Response/RecvMsg
+ parentSpanId: 4
+ spanId: 6
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind
+ parentSpanId: 4
+ spanId: 7
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Entry
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: /user.User/UserFind
Review Comment:
If all of spans still in one single segment(only create local rather entry and exit span), we cannot building the topology, because OAP don't know which one is exit span to communicate with other application.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802756245
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // set trace id to response header
+ if config.OutputTraceId {
+ writer.Header().Set("trace-id", s.TraceID())
Review Comment:
All header things should be consistent for all agents by following https://skywalking.apache.org/docs/main/latest/en/api/x-process-propagation-headers-v3/
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804043670
##########
plugins/gozero/rest/config.go:
##########
@@ -0,0 +1,9 @@
+package rest
+
+//skywalking:config gozero
+var config struct {
+ CollectRequestParameters bool `config:"collect_request_parameters"` // CollectRequestParameters is used to determine whether to collect request parameters.
+ CollectResponseData bool `config:"collect_response_data"` // CollectResponseData is used to determine whether to collect response data.
+ DataSensitiveFields string `config:"data_sensitive_fields"` // DataSensitiveFields is used to determine the sensitive fields in the request and response data.
Review Comment:
Alright, I have now retained only the necessary configurations and will wait to further develop and refine it later.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802640975
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
I know that. When developing this feature, I needed to use the packages io, bytes, encoding/json, regexp, and strings. At that time, I received the following error when compiling the project: skywalking_enhance_rest_server_middleware_intercepter.go:4:2: could not import encoding/json. Currently, I am unable to resolve this issue.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 closed pull request #209: gozero framework plugin support.
URL: https://github.com/apache/skywalking-go/pull/209
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802631610
##########
test/plugins/scenarios/gozero/pb/userpb/user_grpc.testpb.go:
##########
@@ -0,0 +1,163 @@
+// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
Review Comment:
This was generated using the `goctl` command to enable testing of the complete framework tracing.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802640972
##########
test/plugins/scenarios/gozero/pb/userpb/user_grpc.testpb.go:
##########
@@ -0,0 +1,163 @@
+// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
Review Comment:
Yes, I mean you can move this generated command to the scenario.sh file to make these files generate automatically.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802843125
##########
test/plugins/scenarios/gozero/README.md:
##########
@@ -0,0 +1,6 @@
+
+### 生成rpc代码
Review Comment:
@mrproliu We should add a CI to avoid CN words. Main repo is using maven style check.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1805085544
##########
test/plugins/scenarios/gozero/config/excepted.yml:
##########
@@ -0,0 +1,136 @@
+# 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: gozero
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: GET:/user/info
+ parentSpanId: -1
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Entry
+ peer: ''
+ skipAnalysis: false
+ tags:
+ - {key: http.method, value: GET}
+ - {key: url, value: 'localhost:8888/user/info'}
+ - {key: http.params, value: not null}
+ - {key: status_code, value: '200'}
+ refs:
+ - {parentEndpoint: 'GET:/user/info?id=1', networkAddress: 'localhost:8080', refType: CrossProcess,
+ parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not null,
+ parentService: gozero, traceId: not null}
+ - operationName: logx.info
+ parentSpanId: 1
+ spanId: 2
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'user api find: &{Id:0 Uuid:}'}
+ - operationName: logx.error
+ parentSpanId: 1
+ spanId: 3
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'test error log'}
+ - operationName: user.User.UserFind
+ parentSpanId: 1
+ spanId: 4
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Exit
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: user.User.UserFind/Client/Request/SendMsg
+ parentSpanId: 4
+ spanId: 5
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind/Client/Response/RecvMsg
+ parentSpanId: 4
+ spanId: 6
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind
+ parentSpanId: 4
+ spanId: 7
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Entry
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: /user.User/UserFind
Review Comment:
The method enhanced by zrpc is to add a unary interceptor. The underlying service is still a gRPC service. The purpose of adding this unary interceptor is to extend the RPC service provided by go-zero, such as logging parameters, responses, errors, etc. Currently, due to proxy restrictions, additional packages cannot be introduced, so the span can be considered local. Additionally, can you explain the issue of not being able to introduce additional packages in the interceptor? Otherwise, it will be impossible to complete some more distinctive tag reporting.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802525862
##########
docs/en/agent/support-plugins.md:
##########
@@ -12,6 +12,7 @@ metrics based on the tracing data.
* `fiber`: [Fiber](https://github.com/gofiber/fiber) tested v2.49.0 to v2.50.0.
* `echov4`: [Echov4](https://github.com/labstack/echo) tested v4.0.0 to v4.11.4
* `goframe`: [GoFrame](https://github.com/gogf/gf/) tested v2.6.0 to v2.7.3
+ * `gozero`: [GoZero](https://github.com/zeromicro/go-zero) tested v1.7.2
Review Comment:
We should not only tesr one version, please adopt test frameworks, and verify all recent versions.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802543034
##########
plugins/gozero/go.mod:
##########
@@ -0,0 +1,11 @@
+module github.com/apache/skywalking-go/plugins/gozero
+
+go 1.19
+
+require github.com/apache/skywalking-go/plugins/core v0.0.0-20241010090250-3981a26a05db
Review Comment:
we are already using `go.work` to management all dependencies, so I think this require is not necessary.
##########
plugins/gozero/instrument.go:
##########
@@ -0,0 +1,135 @@
+// 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 gozero
+
+import (
+ "embed"
+ "github.com/apache/skywalking-go/plugins/core/instrument"
+ "strings"
+
+ _ "fmt"
Review Comment:
Why need to import here?
##########
plugins/gozero/zrpc/server_middleware_intercepter.go:
##########
@@ -0,0 +1,56 @@
+package zrpc
+
+import (
+ "context"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/zrpc"
+ "google.golang.org/grpc"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// BeforeInvoke intercepts the HTTP request before invoking the handler.
+func (h *ServerMiddlewareInterceptor) BeforeInvoke(invocation operator.Invocation) error {
+ server := invocation.CallerInstance().(*zrpc.RpcServer)
+ server.AddUnaryInterceptors(RpcServeInterceptor(invocation))
+ return nil
+}
+
+// AfterInvoke processes after the HTTP request has been handled.
+func (h *ServerMiddlewareInterceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ return nil
+}
+
+// RpcServeInterceptor is a grpc server interceptor that creates a new span for each incoming request.
+var RpcServeInterceptor = func(invocation operator.Invocation) grpc.UnaryServerInterceptor {
+ return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) {
+ s, err := tracing.CreateLocalSpan(info.FullMethod,
+ tracing.WithLayer(tracing.SpanLayerRPCFramework),
+ tracing.WithTag(tracing.TagURL, info.FullMethod),
+ tracing.WithComponent(5023),
+ )
+ if err != nil {
+ return nil, err
+ }
+ defer s.End()
+
+ resp, err = handler(ctx, req)
+ if err != nil {
+ s.Error(err.Error())
+ }
+ // collect request and response data
+ if config.CollectRequestParameters {
+ //TODO: collect request data
Review Comment:
Please fix TODO.
##########
plugins/gozero/zrpc/server_middleware_intercepter.go:
##########
@@ -0,0 +1,56 @@
+package zrpc
+
+import (
+ "context"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/zrpc"
+ "google.golang.org/grpc"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// BeforeInvoke intercepts the HTTP request before invoking the handler.
+func (h *ServerMiddlewareInterceptor) BeforeInvoke(invocation operator.Invocation) error {
+ server := invocation.CallerInstance().(*zrpc.RpcServer)
+ server.AddUnaryInterceptors(RpcServeInterceptor(invocation))
+ return nil
+}
+
+// AfterInvoke processes after the HTTP request has been handled.
+func (h *ServerMiddlewareInterceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ return nil
+}
+
+// RpcServeInterceptor is a grpc server interceptor that creates a new span for each incoming request.
+var RpcServeInterceptor = func(invocation operator.Invocation) grpc.UnaryServerInterceptor {
+ return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) {
+ s, err := tracing.CreateLocalSpan(info.FullMethod,
+ tracing.WithLayer(tracing.SpanLayerRPCFramework),
+ tracing.WithTag(tracing.TagURL, info.FullMethod),
+ tracing.WithComponent(5023),
+ )
+ if err != nil {
+ return nil, err
+ }
+ defer s.End()
+
+ resp, err = handler(ctx, req)
+ if err != nil {
+ s.Error(err.Error())
+ }
+ // collect request and response data
+ if config.CollectRequestParameters {
+ //TODO: collect request data
+ //s.Tag(tracing.TagRpcRequest, "request")
+ }
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
Please fix TODO.
##########
test/plugins/scenarios/gozero/plugin.yml:
##########
@@ -0,0 +1,25 @@
+# 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}/user/info
Review Comment:
I can't see the excepted file to validate all generated tracing data?
##########
test/plugins/scenarios/gozero/README.md:
##########
@@ -0,0 +1,6 @@
+
+### 生成rpc代码
Review Comment:
Why user need this documentation? If you want to running this command before starting testing, please add this into [startup.sh](https://github.com/apache/skywalking-go/pull/209/files#diff-a57cf9e1481207842ec8431d1a36ccfc1823f0e77f2aadfe9b9af5d6b8e6790a)
##########
plugins/gozero/rest/config.go:
##########
@@ -0,0 +1,9 @@
+package rest
+
+//skywalking:config gozero
+var config struct {
+ CollectRequestParameters bool `config:"collect_request_parameters"` // CollectRequestParameters is used to determine whether to collect request parameters.
+ CollectResponseData bool `config:"collect_response_data"` // CollectResponseData is used to determine whether to collect response data.
+ DataSensitiveFields string `config:"data_sensitive_fields"` // DataSensitiveFields is used to determine the sensitive fields in the request and response data.
Review Comment:
I can't see any place have use this configuration.
##########
test/plugins/run.sh:
##########
@@ -204,7 +204,7 @@ for framework_version in $frameworks; do
-go-agent ${go_agent} > ${case_logs}/runner-helper.log
echo "staring the testcase ${scenario_name}, ${case_name}"
-
+ dos2unix ${case_home}/scenarios.sh
Review Comment:
Please remove unnecessary codes.
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
Please fix the TODO.
##########
test/plugins/Makefile:
##########
@@ -18,12 +18,16 @@
GO = go
GO_BUILD = $(GO) build
+REPODIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))/../../
+VERSION_PATH = $(REPODIR)/VERSION
+VERSION ?= $(shell grep version $(VERSION_PATH) | awk -F ': ' '{print $$2}')
+
.PHONY: build
build:
mkdir -p dist
# go agent
@make -C ../../tools/go-agent linux
- cp ../../bin/skywalking-go-agent--linux-$(shell go env GOARCH) dist/skywalking-go-agent
+ cp ../../bin/skywalking-go-agent-$(VERSION)-linux-$(shell go env GOARCH) dist/skywalking-go-agent
Review Comment:
Please do not submit content unrelated to this plugin. If necessary, please open a separate PR, and we can discuss it there.
##########
test/plugins/scenarios/gozero/pb/userpb/user_grpc.testpb.go:
##########
@@ -0,0 +1,163 @@
+// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
Review Comment:
I think these files can be generated by the plugin test, right?
##########
test/plugins/scenarios/gozero/handler.go:
##########
@@ -0,0 +1,101 @@
+package main
+
+import (
+ "github.com/zeromicro/go-zero/rest"
+ "github.com/zeromicro/go-zero/rest/httpx"
+ "net/http"
+)
+
+func RegisterApiHandlers(server *rest.Server, serverCtx *ApiServiceContext) {
+ server.AddRoutes(
+ []rest.Route{
+ {
+ Method: http.MethodGet,
+ Path: "/health",
+ Handler: func(w http.ResponseWriter, r *http.Request) {
+ httpx.OkJson(w, "ok")
+ },
+ },
+ },
+ )
+ server.AddRoutes(
+ []rest.Route{
+ {
+ // 用户查找
Review Comment:
Please remove all Chinese comments.
##########
test/plugins/scenarios/gozero/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 gozero
+
+./gozero
Review Comment:
Please activate all plugin configuration before start, we should check all features are works or not.
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // set trace id to response header
+ if config.OutputTraceId {
+ writer.Header().Set("trace-id", s.TraceID())
Review Comment:
Can we validate whether the trace ID is setting success or not in the plugin test?
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802845220
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,64 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
Review Comment:
```suggestion
// SkyWalking middleware
```
Please keep the project name correctly.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802634192
##########
plugins/gozero/instrument.go:
##########
@@ -0,0 +1,135 @@
+// 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 gozero
+
+import (
+ "embed"
+ "github.com/apache/skywalking-go/plugins/core/instrument"
+ "strings"
+
+ _ "fmt"
Review Comment:
Okay, the import here is unnecessary, and I will clean it up.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1803984675
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // set trace id to response header
+ if config.OutputTraceId {
+ writer.Header().Set("trace-id", s.TraceID())
Review Comment:
Okay, I have removed 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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#issuecomment-2415967452
Please clean up your commits, rebase them to one commit.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1803953372
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,70 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// SkyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
TODO, need to wait for this before merging.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804057603
##########
plugins/core/tracing/span.go:
##########
@@ -53,7 +53,9 @@ const (
TagStatusCode = "status_code"
TagHTTPMethod = "http.method"
TagHTTPParams = "http.params"
+ TagHTTPBody = "http.body"
TagHTTPHeaders = "http.headers"
+ TagHTTPResponse = "http.response"
Review Comment:
If we are not using these tags, please remove them.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804052699
##########
test/plugins/scenarios/gozero/plugin.yml:
##########
@@ -0,0 +1,25 @@
+# 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}/user/info
Review Comment:
@zjh129 I think the question is, where is the validation? You have the sample application, and with this booting script. Then the CI tool requires the `excepted.yml`.
An reference, https://github.com/apache/skywalking-go/blob/main/test/plugins/scenarios/dubbo/config/excepted.yml
Otherwise, CI can't tell whether your codes are able to work.
We as maintainers can't validate all through manual and logic setup, because it is not practical and costs too much time.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804052563
##########
test/plugins/scenarios/gozero/rpc_server.go:
##########
@@ -0,0 +1,34 @@
+// Code generated by goctl. DO NOT EDIT.
+// goctl 1.7.2
+// Source: user.proto
+
+package main
Review Comment:
You have already add the auto-generate command, so I think this file can be deleted.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804300997
##########
tools/go-agent/config/agent.default.yaml:
##########
@@ -118,5 +118,9 @@ plugin:
collect_request_headers: ${SW_AGENT_PLUGIN_CONFIG_GOFRAME_COLLECT_REQUEST_HEADERS:}
# Controlling the length limitation of all header values
header_length_threshold: ${SW_AGENT_PLUGIN_CONFIG_GOFRAME_HEADER_LENGTH_THRESHOLD:2048}
-
+ gozero:
+ # Collect the parameters of gozero request
+ collect_request_parameters: ${SW_AGENT_PLUGIN_CONFIG_GOZERO_COLLECT_REQUEST_PARAMETERS:true}
+ # Collect the parameters of the gozero logx
+ collect_logx: ${SW_AGENT_PLUGIN_CONFIG_GOZERO_COLLECT_LOGX:true}
Review Comment:
To reduce network traffic usage and storage usage, please default to turning off parameters and logs.
##########
plugins/gozero/core/logx/logx_error_intercepter.go:
##########
@@ -0,0 +1,45 @@
+package logx
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+)
+
+type LoggerErrorInterceptor struct {
+}
+
+var createLogxSpan = func(invocation operator.Invocation, operation string) (tracing.Span, error) {
+ argLen := len(invocation.Args())
+ if argLen == 0 {
+ return nil, nil
+ }
+ var logxData string
+ if argLen == 1 {
+ logxData = fmt.Sprint(invocation.Args()[0].([]any)...)
+ } else {
+ logxData = fmt.Sprintf(invocation.Args()[0].(string), invocation.Args()[1].([]any)...)
+ }
+ return tracing.CreateLocalSpan(fmt.Sprintf("logx.%s", operation), tracing.WithTag("content", logxData), tracing.WithComponent(5023))
Review Comment:
```suggestion
return tracing.CreateLocalSpan(fmt.Sprintf("logx/%s", operation), tracing.WithTag("content", logxData), tracing.WithComponent(5023))
```
##########
test/plugins/scenarios/gozero/config/excepted.yml:
##########
@@ -0,0 +1,136 @@
+# 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: gozero
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: GET:/user/info
+ parentSpanId: -1
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Entry
+ peer: ''
+ skipAnalysis: false
+ tags:
+ - {key: http.method, value: GET}
+ - {key: url, value: 'localhost:8888/user/info'}
+ - {key: http.params, value: not null}
+ - {key: status_code, value: '200'}
+ refs:
+ - {parentEndpoint: 'GET:/user/info?id=1', networkAddress: 'localhost:8080', refType: CrossProcess,
+ parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not null,
+ parentService: gozero, traceId: not null}
+ - operationName: logx.info
+ parentSpanId: 1
+ spanId: 2
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'user api find: &{Id:0 Uuid:}'}
+ - operationName: logx.error
+ parentSpanId: 1
+ spanId: 3
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'test error log'}
+ - operationName: user.User.UserFind
+ parentSpanId: 1
+ spanId: 4
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Exit
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: user.User.UserFind/Client/Request/SendMsg
+ parentSpanId: 4
+ spanId: 5
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind/Client/Response/RecvMsg
+ parentSpanId: 4
+ spanId: 6
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind
+ parentSpanId: 4
+ spanId: 7
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Entry
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: /user.User/UserFind
Review Comment:
I think zrpc should be a framework similar to grpc, mainly used for communication between multiple processes, right? Then why is the RPC-related span still local? As I understand it, there should be entry and exit spans.
##########
test/plugins/scenarios/gozero/config/excepted.yml:
##########
@@ -0,0 +1,136 @@
+# 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: gozero
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
Review Comment:
Why is there just a single segment, if they are cross-process, it should have two segments. Please correct me if I'm wrong.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802754954
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // set trace id to response header
+ if config.OutputTraceId {
+ writer.Header().Set("trace-id", s.TraceID())
Review Comment:
> writer.Header().Set("trace-id", s.TraceID())
Why write the trace ID in the response? This isn't declared in our protocol. Please remove 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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1802644298
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
+ //s.Tag(tracing.TagHTTPResponse, "resp")
+ }
+
+ // set trace id to response header
+ if config.OutputTraceId {
+ writer.Header().Set("trace-id", s.TraceID())
Review Comment:
The test code runs successfully, and the response header successfully returns during the HTTP request.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1803988878
##########
test/plugins/scenarios/gozero/plugin.yml:
##########
@@ -0,0 +1,25 @@
+# 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}/user/info
Review Comment:
The file path is: `test/plugins/scenarios/gozero/handler.go`. I compressed the application code generated by the goZero tool goctrl and consolidated the `HTTP` and `RPC` code into one directory for easier testing.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804052747
##########
test/plugins/scenarios/gozero/user/user.go:
##########
@@ -0,0 +1,50 @@
+// Code generated by goctl. DO NOT EDIT.
+// goctl 1.7.2
+// Source: user.proto
+
+package user
Review Comment:
This file can also deleted.
##########
test/plugins/scenarios/gozero/plugin.yml:
##########
@@ -0,0 +1,25 @@
+# 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}/user/info
+health-checker: http://${HTTP_HOST}:${HTTP_PORT}/health
+start-script: ./bin/startup.sh
+framework: github.com/zeromicro/go-zero
+export-port: 8888
+support-version:
+ - go: 1.18
+ framework:
+ - v1.7.2
Review Comment:
Since you have declared this plugin support 1.6.0 - 1.7.2, please add all test cases to check is all of the versions available.
##########
docs/en/agent/plugin-configurations.md:
##########
@@ -1,14 +1,17 @@
# Plugin Configurations
-| key | environment key | default value | description |
-|------------------------------------|-----------------------------------------------------------|---------------|----------------------------------------------------------------|
-| http.server_collect_parameters | SW_AGENT_PLUGIN_CONFIG_HTTP_SERVER_COLLECT_PARAMETERS | false | Collect the parameters of the HTTP request on the server side. |
-| mongo.collect_statement | SW_AGENT_PLUGIN_CONFIG_MONGO_COLLECT_STATEMENT | false | Collect the statement of the MongoDB request. |
-| sql.collect_parameter | SW_AGENT_PLUGIN_CONFIG_SQL_COLLECT_PARAMETER | false | Collect the parameter of the SQL request. |
-| redis.max_args_bytes | SW_AGENT_PLUGIN_CONFIG_REDIS_MAX_ARGS_BYTES | 1024 | Limit the bytes size of redis args request. |
-| reporter.discard | SW_AGENT_REPORTER_DISCARD | false | Discard the reporter. |
-| gin.collect_request_headers | SW_AGENT_PLUGIN_CONFIG_GIN_COLLECT_REQUEST_HEADERS | | Collect the http header of gin request. |
-| gin.header_length_threshold | SW_AGENT_PLUGIN_CONFIG_GIN_HEADER_LENGTH_THRESHOLD | 2048 | Controlling the length limitation of all header values. |
-| goframe.collect_request_parameters | SW_AGENT_PLUGIN_CONFIG_GOFRAME_COLLECT_REQUEST_PARAMETERS | false | Collect the parameters of the HTTP request on the server side. |
-| goframe.collect_request_headers | SW_AGENT_PLUGIN_CONFIG_GOFRAME_COLLECT_REQUEST_HEADERS | | Collect the http header of goframe request. |
-| goframe.header_length_threshold | SW_AGENT_PLUGIN_CONFIG_GOFRAME_HEADER_LENGTH_THRESHOLD | 2048 | Controlling the length limitation of all header values. |
\ No newline at end of file
+| key | environment key | default value | description |
+|------------------------------------|-----------------------------------------------------------|---------------|----------------------------------------------------------------------------|
+| http.server_collect_parameters | SW_AGENT_PLUGIN_CONFIG_HTTP_SERVER_COLLECT_PARAMETERS | false | Collect the parameters of the HTTP request on the server side. |
+| mongo.collect_statement | SW_AGENT_PLUGIN_CONFIG_MONGO_COLLECT_STATEMENT | false | Collect the statement of the MongoDB request. |
+| sql.collect_parameter | SW_AGENT_PLUGIN_CONFIG_SQL_COLLECT_PARAMETER | false | Collect the parameter of the SQL request. |
+| redis.max_args_bytes | SW_AGENT_PLUGIN_CONFIG_REDIS_MAX_ARGS_BYTES | 1024 | Limit the bytes size of redis args request. |
+| reporter.discard | SW_AGENT_REPORTER_DISCARD | false | Discard the reporter. |
+| gin.collect_request_headers | SW_AGENT_PLUGIN_CONFIG_GIN_COLLECT_REQUEST_HEADERS | | Collect the http header of gin request. |
+| gin.header_length_threshold | SW_AGENT_PLUGIN_CONFIG_GIN_HEADER_LENGTH_THRESHOLD | 2048 | Controlling the length limitation of all header values. |
+| goframe.collect_request_parameters | SW_AGENT_PLUGIN_CONFIG_GOFRAME_COLLECT_REQUEST_PARAMETERS | false | Collect the parameters of the HTTP request on the server side. |
+| goframe.collect_request_headers | SW_AGENT_PLUGIN_CONFIG_GOFRAME_COLLECT_REQUEST_HEADERS | | Collect the http header of goframe request. |
+| goframe.header_length_threshold | SW_AGENT_PLUGIN_CONFIG_GOFRAME_HEADER_LENGTH_THRESHOLD | 2048 | Controlling the length limitation of all header values. |
+| gozero.collect_request_parameters | SW_AGENT_PLUGIN_CONFIG_GOZERO_COLLECT_REQUEST_PARAMETERS | true | Collect the parameters of the HTTP request on the server side. |
+| gozero.collect_response_data | SW_AGENT_PLUGIN_CONFIG_GOZERO_COLLECT_RESPONSE_DATA | true | CollectResponseData is used to determine whether to collect response data. |
Review Comment:
This parameter not exist.
##########
plugins/gozero/core/logx/logx_debug_intercepter.go:
##########
@@ -0,0 +1,30 @@
+package logx
+
+import (
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+)
+
+type LoggerDebugInterceptor struct {
+}
+
+// BeforeInvoke implements the instrument.Interceptor interface.
+func (h *LoggerDebugInterceptor) BeforeInvoke(invocation operator.Invocation) error {
+ if config.CollectLogx {
+ span, err := createLogxSpan(invocation, "logx.debug")
Review Comment:
We should consider another format of span name. Such as redis plugin, we use `redis/[operation]` as span name.
##########
test/plugins/scenarios/gozero/plugin.yml:
##########
@@ -0,0 +1,25 @@
+# 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}/user/info
Review Comment:
I mean you have to submit the tracing data which generated from the skywalking-go. Such as when we enhance the `net/http` framework, we have to generate [an excepted.yaml file](https://github.com/apache/skywalking-go/blob/main/test/plugins/scenarios/http/config/excepted.yml) to validate the tracing data are reported success or not.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "zjh129 (via GitHub)" <gi...@apache.org>.
zjh129 commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1804043014
##########
plugins/gozero/rest/server_middleware_intercepter.go:
##########
@@ -0,0 +1,69 @@
+package rest
+
+import (
+ "fmt"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/rest"
+ "net/http"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// skyWalking middleware
+var SkyWalkingMiddleware rest.Middleware = func(next http.HandlerFunc) http.HandlerFunc {
+ return func(writer http.ResponseWriter, request *http.Request) {
+ s, err := tracing.CreateEntrySpan(fmt.Sprintf("%s:%s", request.Method, request.URL.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(5023))
+ if err != nil {
+ next(writer, request)
+ return
+ }
+
+ defer s.End()
+
+ // collect response data
+ if config.CollectRequestParameters {
+ switch request.Method {
+ case http.MethodGet:
+ if request.URL.RawQuery != "" {
+ s.Tag(tracing.TagHTTPParams, request.URL.RawQuery)
+ }
+ case http.MethodPost, http.MethodPut, http.MethodPatch:
+ //TODO collect request body data
+ if request.Body != nil {
+ }
+ }
+ }
+
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
The interceptor is currently unable to use packages other than enhanced objects, so we will remove the functionality for collecting request parameters and response data and will submit a PR once support is available in the future.
##########
plugins/gozero/zrpc/server_middleware_intercepter.go:
##########
@@ -0,0 +1,56 @@
+package zrpc
+
+import (
+ "context"
+ "github.com/apache/skywalking-go/plugins/core/operator"
+ "github.com/apache/skywalking-go/plugins/core/tracing"
+ "github.com/zeromicro/go-zero/zrpc"
+ "google.golang.org/grpc"
+)
+
+type ServerMiddlewareInterceptor struct {
+}
+
+// BeforeInvoke intercepts the HTTP request before invoking the handler.
+func (h *ServerMiddlewareInterceptor) BeforeInvoke(invocation operator.Invocation) error {
+ server := invocation.CallerInstance().(*zrpc.RpcServer)
+ server.AddUnaryInterceptors(RpcServeInterceptor(invocation))
+ return nil
+}
+
+// AfterInvoke processes after the HTTP request has been handled.
+func (h *ServerMiddlewareInterceptor) AfterInvoke(invocation operator.Invocation, result ...interface{}) error {
+ return nil
+}
+
+// RpcServeInterceptor is a grpc server interceptor that creates a new span for each incoming request.
+var RpcServeInterceptor = func(invocation operator.Invocation) grpc.UnaryServerInterceptor {
+ return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) {
+ s, err := tracing.CreateLocalSpan(info.FullMethod,
+ tracing.WithLayer(tracing.SpanLayerRPCFramework),
+ tracing.WithTag(tracing.TagURL, info.FullMethod),
+ tracing.WithComponent(5023),
+ )
+ if err != nil {
+ return nil, err
+ }
+ defer s.End()
+
+ resp, err = handler(ctx, req)
+ if err != nil {
+ s.Error(err.Error())
+ }
+ // collect request and response data
+ if config.CollectRequestParameters {
+ //TODO: collect request data
+ //s.Tag(tracing.TagRpcRequest, "request")
+ }
+ // collect response data
+ if config.CollectResponseData {
+ //TODO: collect response data
Review Comment:
The interceptor is currently unable to use packages other than enhanced objects, so we will remove the functionality for collecting request parameters and response data and will submit a PR once support is available in the future.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1805788982
##########
test/plugins/scenarios/gozero/config/excepted.yml:
##########
@@ -0,0 +1,136 @@
+# 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: gozero
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: GET:/user/info
+ parentSpanId: -1
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Entry
+ peer: ''
+ skipAnalysis: false
+ tags:
+ - {key: http.method, value: GET}
+ - {key: url, value: 'localhost:8888/user/info'}
+ - {key: http.params, value: not null}
+ - {key: status_code, value: '200'}
+ refs:
+ - {parentEndpoint: 'GET:/user/info?id=1', networkAddress: 'localhost:8080', refType: CrossProcess,
+ parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not null,
+ parentService: gozero, traceId: not null}
+ - operationName: logx.info
+ parentSpanId: 1
+ spanId: 2
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'user api find: &{Id:0 Uuid:}'}
+ - operationName: logx.error
+ parentSpanId: 1
+ spanId: 3
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'test error log'}
+ - operationName: user.User.UserFind
+ parentSpanId: 1
+ spanId: 4
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Exit
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: user.User.UserFind/Client/Request/SendMsg
+ parentSpanId: 4
+ spanId: 5
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind/Client/Response/RecvMsg
+ parentSpanId: 4
+ spanId: 6
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind
+ parentSpanId: 4
+ spanId: 7
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Entry
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: /user.User/UserFind
Review Comment:
For the additional package import, please read this document to know the [implementation key principles](https://skywalking.apache.org/docs/skywalking-go/next/en/concepts-and-designs/key-principles/). The short version is because the package import mechanism of golang compiler, we cannot add more package, that's why we have tools operator to let plugin do more operation without package import.
--
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
Re: [PR] gozero framework plugin support. [skywalking-go]
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #209:
URL: https://github.com/apache/skywalking-go/pull/209#discussion_r1805790784
##########
test/plugins/scenarios/gozero/config/excepted.yml:
##########
@@ -0,0 +1,136 @@
+# 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: gozero
+ segmentSize: ge 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: GET:/user/info
+ parentSpanId: -1
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Entry
+ peer: ''
+ skipAnalysis: false
+ tags:
+ - {key: http.method, value: GET}
+ - {key: url, value: 'localhost:8888/user/info'}
+ - {key: http.params, value: not null}
+ - {key: status_code, value: '200'}
+ refs:
+ - {parentEndpoint: 'GET:/user/info?id=1', networkAddress: 'localhost:8080', refType: CrossProcess,
+ parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not null,
+ parentService: gozero, traceId: not null}
+ - operationName: logx.info
+ parentSpanId: 1
+ spanId: 2
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'user api find: &{Id:0 Uuid:}'}
+ - operationName: logx.error
+ parentSpanId: 1
+ spanId: 3
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5023
+ isError: false
+ spanType: Local
+ tags:
+ - {key: content, value: 'test error log'}
+ - operationName: user.User.UserFind
+ parentSpanId: 1
+ spanId: 4
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Exit
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: user.User.UserFind/Client/Request/SendMsg
+ parentSpanId: 4
+ spanId: 5
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind/Client/Response/RecvMsg
+ parentSpanId: 4
+ spanId: 6
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Local
+ tags:
+ - { key: url, value: not null }
+ - operationName: user.User.UserFind
+ parentSpanId: 4
+ spanId: 7
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 23
+ isError: false
+ spanType: Entry
+ tags:
+ - { key: url, value: not null }
+ - { key: rpc.type, value: Unary }
+ - operationName: /user.User/UserFind
Review Comment:
Also, I cannot see any exit span by building grpc or zrpc, could you help to explain this?
--
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