You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/11/17 07:02:22 UTC

[GitHub] [skywalking-rover] mrproliu opened a new pull request, #58: Support upload the full HTTP content to span attached event

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

   Following https://github.com/apache/skywalking/issues/9802, Support submits the request/response raw data to the OAP


-- 
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-rover] mrproliu commented on a diff in pull request #58: Support upload the full HTTP content to span attached event

Posted by GitBox <gi...@apache.org>.
mrproliu commented on code in PR #58:
URL: https://github.com/apache/skywalking-rover/pull/58#discussion_r1025030996


##########
docs/en/setup/configuration/profiling.md:
##########
@@ -106,4 +106,10 @@ Based on the above two data types, the following metrics are provided.
 
 | Name         | Type  | Unit        | Description                |
 |--------------|-------|-------------|----------------------------|
-| slow_traces  | TopN  | millisecond | The Top N slow trace(id)s  |
\ No newline at end of file
+| slow_traces  | TopN  | millisecond | The Top N slow trace(id)s  |
+
+##### Span Attached Event
+| Name               | Description                                                                                  |
+|--------------------|----------------------------------------------------------------------------------------------|
+| http-full-request  | Complete information about the HTTP request, it's only uploads when it matches slow traces.  |
+| http-full-response | Complete information about the HTTP response, it's only uploads when it matches slow traces. |

Review Comment:
   For this PR, I only implemented the slow policies. I will create other PR to implement them all(contains protocols 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-rover] mrproliu commented on a diff in pull request #58: Support upload the full HTTP content to span attached event

Posted by GitBox <gi...@apache.org>.
mrproliu commented on code in PR #58:
URL: https://github.com/apache/skywalking-rover/pull/58#discussion_r1025030060


##########
pkg/profiling/task/network/analyze/layer7/protocols/base/tracing.go:
##########
@@ -15,23 +15,32 @@
 // specific language governing permissions and limitations
 // under the License.
 
-package protocols
+package base
 
 import (
 	"encoding/base64"
 	"fmt"
 	"strings"
+
+	v3 "skywalking.apache.org/repo/goapi/collect/language/agent/v3"
 )
 
 type TracingContext interface {
 	TraceID() string
-	Provider() string
+	TraceSegmentID() string
+	SpanID() string
+	Provider() *TraceContextProvider
+}
+
+type TraceContextProvider struct {
+	Type v3.SpanAttachedEvent_SpanReferenceType
+	Name string
 }
 
 type SkyWalkingTracingContext struct {
 	TraceID0              string
-	SegmentID             string
-	SpanID                string
+	SegmentID0            string
+	SpanID0               string

Review Comment:
   That's used to avoid conflict between field and method. 



##########
pkg/profiling/task/network/analyze/layer7/protocols/http1/analyzer_test.go:
##########
@@ -243,19 +245,19 @@ func TestBuildHTTP1(t *testing.T) {
 
 	for _, testCase := range tests {
 		//t.Run(testCase.name, func(t *testing.T) {
-		analyzer := NewHTTP1Analyzer().(*HTTP1Analyzer)
+		analyzer := NewHTTP1Analyzer().(*Analyzer)
 		l := list.New()
 		var events = make([]struct {
 			start, end int
 		}, 0)
 		for _, event := range testCase.events {
-			req, resp := analyzer.buildHTTP1(l, &SocketDataUploadEvent{
-				DataID:   uint64(event.dataID),
-				MsgType:  base.SocketMessageType(event.dataType),
-				Sequence: uint16(event.sequence),
-				Finished: uint8(event.finished),
-				Buffer:   bufferConvert(event.data),
-				DataLen:  uint16(len(event.data)),
+			req, resp := analyzer.buildHTTP1(l, &base2.SocketDataUploadEvent{
+				DataID:       uint64(event.dataID),
+				MsgType:      base.SocketMessageType(event.dataType),
+				Sequence:     uint16(event.sequence),
+				FinishStatus: uint8(event.finished),

Review Comment:
   Updated.



-- 
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-rover] wu-sheng commented on a diff in pull request #58: Support upload the full HTTP content to span attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #58:
URL: https://github.com/apache/skywalking-rover/pull/58#discussion_r1024983996


##########
docs/en/setup/configuration/profiling.md:
##########
@@ -106,4 +106,10 @@ Based on the above two data types, the following metrics are provided.
 
 | Name         | Type  | Unit        | Description                |
 |--------------|-------|-------------|----------------------------|
-| slow_traces  | TopN  | millisecond | The Top N slow trace(id)s  |
\ No newline at end of file
+| slow_traces  | TopN  | millisecond | The Top N slow trace(id)s  |
+
+##### Span Attached Event
+| Name               | Description                                                                                  |
+|--------------------|----------------------------------------------------------------------------------------------|
+| http-full-request  | Complete information about the HTTP request, it's only uploads when it matches slow traces.  |
+| http-full-response | Complete information about the HTTP response, it's only uploads when it matches slow traces. |

Review Comment:
   ```suggestion
   | http-full-request  | Complete information about the HTTP request, it's only reported when it matches slow traces.  |
   | http-full-response | Complete information about the HTTP response, it's only reported when it matches slow traces. |
   ```



-- 
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-rover] wu-sheng commented on a diff in pull request #58: Support upload the full HTTP content to span attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #58:
URL: https://github.com/apache/skywalking-rover/pull/58#discussion_r1024984550


##########
pkg/profiling/task/network/analyze/layer7/protocols/base/tracing.go:
##########
@@ -15,23 +15,32 @@
 // specific language governing permissions and limitations
 // under the License.
 
-package protocols
+package base
 
 import (
 	"encoding/base64"
 	"fmt"
 	"strings"
+
+	v3 "skywalking.apache.org/repo/goapi/collect/language/agent/v3"
 )
 
 type TracingContext interface {
 	TraceID() string
-	Provider() string
+	TraceSegmentID() string
+	SpanID() string
+	Provider() *TraceContextProvider
+}
+
+type TraceContextProvider struct {
+	Type v3.SpanAttachedEvent_SpanReferenceType
+	Name string
 }
 
 type SkyWalkingTracingContext struct {
 	TraceID0              string
-	SegmentID             string
-	SpanID                string
+	SegmentID0            string
+	SpanID0               string

Review Comment:
   Why ends of `0`?



-- 
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-rover] mrproliu merged pull request #58: Support upload the full HTTP content to span attached event

Posted by GitBox <gi...@apache.org>.
mrproliu merged PR #58:
URL: https://github.com/apache/skywalking-rover/pull/58


-- 
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-rover] wu-sheng commented on a diff in pull request #58: Support upload the full HTTP content to span attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #58:
URL: https://github.com/apache/skywalking-rover/pull/58#discussion_r1024986704


##########
pkg/profiling/task/network/analyze/layer7/protocols/http1/analyzer_test.go:
##########
@@ -243,19 +245,19 @@ func TestBuildHTTP1(t *testing.T) {
 
 	for _, testCase := range tests {
 		//t.Run(testCase.name, func(t *testing.T) {
-		analyzer := NewHTTP1Analyzer().(*HTTP1Analyzer)
+		analyzer := NewHTTP1Analyzer().(*Analyzer)
 		l := list.New()
 		var events = make([]struct {
 			start, end int
 		}, 0)
 		for _, event := range testCase.events {
-			req, resp := analyzer.buildHTTP1(l, &SocketDataUploadEvent{
-				DataID:   uint64(event.dataID),
-				MsgType:  base.SocketMessageType(event.dataType),
-				Sequence: uint16(event.sequence),
-				Finished: uint8(event.finished),
-				Buffer:   bufferConvert(event.data),
-				DataLen:  uint16(len(event.data)),
+			req, resp := analyzer.buildHTTP1(l, &base2.SocketDataUploadEvent{
+				DataID:       uint64(event.dataID),
+				MsgType:      base.SocketMessageType(event.dataType),
+				Sequence:     uint16(event.sequence),
+				FinishStatus: uint8(event.finished),

Review Comment:
   I think `finished` is a better status name. FinishStatus means `finish the status`



-- 
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-rover] wu-sheng commented on a diff in pull request #58: Support upload the full HTTP content to span attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #58:
URL: https://github.com/apache/skywalking-rover/pull/58#discussion_r1024973141


##########
docs/en/setup/configuration/profiling.md:
##########
@@ -106,4 +106,10 @@ Based on the above two data types, the following metrics are provided.
 
 | Name         | Type  | Unit        | Description                |
 |--------------|-------|-------------|----------------------------|
-| slow_traces  | TopN  | millisecond | The Top N slow trace(id)s  |
\ No newline at end of file
+| slow_traces  | TopN  | millisecond | The Top N slow trace(id)s  |
+
+##### Span Attached Event
+| Name               | Description                                                                                  |
+|--------------------|----------------------------------------------------------------------------------------------|
+| http-full-request  | Complete information about the HTTP request, it's only uploads when it matches slow traces.  |
+| http-full-response | Complete information about the HTTP response, it's only uploads when it matches slow traces. |

Review Comment:
   For slow trace only? I think we have `slow`, `4xx`, `5xx` policies?



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