You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "mrproliu (via GitHub)" <gi...@apache.org> on 2023/03/09 07:58:35 UTC
[GitHub] [skywalking-query-protocol] mrproliu opened a new pull request, #111: Support cross-thread trace profiling
mrproliu opened a new pull request, #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111
Following https://github.com/apache/skywalking/issues/10373
We need to let GraphQL support query/analysis with multiple segments.
--
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-query-protocol] wu-sheng commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1133393060
##########
profile.graphqls:
##########
@@ -131,6 +133,8 @@ type ProfiledSpan {
layer: String
tags: [KeyValue!]!
logs: [LogEntity!]!
+ # is the span belongs the segments been profiled
+ profiled: Boolean!
Review Comment:
```suggestion
# Status represents profiling data that covers the duration of the span.
profiled: Boolean!
```
--
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-query-protocol] wu-sheng merged pull request #111: Support cross-thread trace profiling
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111
--
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-query-protocol] wu-sheng commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130632497
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
> No verification, just query the segments and combine them when the cross-thread segment matches with the parent segment. Then, The UI can show the spans in the same process, and analyze the profiling results with the same process(segment id list).
I think UI is hard to verify things like `ref` linking segments. We may only bind segments with trace ID belonging to one task ID. This should be enough.
--
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-query-protocol] wu-sheng commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130625761
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
Are you going to verify the segment data somehow?
--
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-query-protocol] mrproliu commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130634828
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
Because they may become different traces/processes, this decide by the query.
--
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-query-protocol] wu-sheng commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130643824
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
Review Comment:
@mrproliu How about UI enhancing this way,
1. Use this API, to group `segmentId`s by `traceId` and `Instance ID`(new Added). Could our backend these without agent update?
2. `getProfiledSegments`(new APIs) to get `ProfiledSegment`s.
3. Build profiling.
The major differences with yours are
1. `getProfiledSegment` should be `deprecated`, only a shell(special condition) to `getProfiledSegments`.
2. No new `ProfiledSegments`.
--
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-query-protocol] wu-sheng commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130631040
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
And why do we need `ProfiledSegments` Array and this new object rather than only `ProfiledSegment` array only?
--
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-query-protocol] mrproliu commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130630778
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
No verification, just query the segments and combine them when the cross-thread segment matches with the parent segment.
Then, The UI can show the spans in the same process, and analyze the profiling results with the same process(segment id list).
--
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-query-protocol] mrproliu commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130642383
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
> > No verification, just query the segments and combine them when the cross-thread segment matches with the parent segment. Then, The UI can show the spans in the same process, and analyze the profiling results with the same process(segment id list).
>
> I think UI is hard to verify things like `ref` linking segments. We may only bind segments with trace ID belonging to one task ID. This should be enough.
Do you mean to query all segments in the same trace? But there may have multiple segments that are not profiled, so they cannot be analyzed.
--
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-query-protocol] wu-sheng commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130619054
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
What do you mean `by ref`? It is better query by Trace ID?
--
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-query-protocol] mrproliu commented on a diff in pull request #111: Support cross-thread trace profiling
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #111:
URL: https://github.com/apache/skywalking-query-protocol/pull/111#discussion_r1130622053
##########
profile.graphqls:
##########
@@ -156,6 +163,10 @@ extend type Query {
getProfileTaskSegmentList(taskID: String): [BasicTrace!]!
# query profiled segment
getProfiledSegment(segmentId: String): ProfiledSegment
+ # query combined segments by ref
+ getProfiledSegments(segmentIds: [String!]!): [ProfiledSegments!]!
Review Comment:
This means combining all the same segments under one process.
Query by segments because this query only focuses on the profiled segments, not all segments under one trace.
--
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