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/07/14 01:58:48 UTC
[GitHub] [skywalking-agent-test-tool] mrproliu opened a new pull request, #52: Enhance the segment span validation
mrproliu opened a new pull request, #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52
Currently, in the Go version of the agent, the order of spans in a segment can become unstable due to fast execution, although the span IDs are always ordered. To address this issue, this PR is submitted to allow validation of corresponding data in the segment when spans are out of order based on their span IDs.
--
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-agent-test-tool] kezhenxu94 commented on pull request #52: Enhance the segment span validation
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#issuecomment-1635181563
> Yes, We could. I'm just concerned about whether it will affect the existing validation. If you think sorting is enough, I can change it to validate after sorting.
I believe it doesn't affect the existing validation because we have been assuming the spans are in order.
BTW, why don't we switch to the infra-e2e project for validation?
--
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-agent-test-tool] kezhenxu94 commented on pull request #52: Enhance the segment span validation
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#issuecomment-1635171840
> Currently, in the Go version of the agent, the order of spans in a segment can become unstable due to fast execution, although the span IDs are always ordered. To address this issue, this PR is submitted to allow validation of corresponding data in the segment when spans are out of order based on their span IDs.
If it's the order's matter, can we just sort them out every time before we do an actual validation?
--
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-agent-test-tool] mrproliu commented on pull request #52: Enhance the segment span validation
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#issuecomment-1635189897
> I believe it doesn't affect the existing validation because we have been assuming the spans are in order.
BTW, I think maybe we cannot sort it, because the excepted span id may be an expression.
--
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-agent-test-tool] wu-sheng commented on pull request #52: Enhance the segment span validation
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#issuecomment-1635188260
> BTW, why don't we switch to the infra-e2e project for validation?
Because it doesn't have capabilities to roll the versions(supported 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
[GitHub] [skywalking-agent-test-tool] mrproliu closed pull request #52: Enhance the segment span validation
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu closed pull request #52: Enhance the segment span validation
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52
--
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-agent-test-tool] kezhenxu94 commented on a diff in pull request #52: Enhance the segment span validation
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#discussion_r1263377329
##########
validator/src/main/java/org/apache/skywalking/plugin/test/agent/tool/validator/assertor/SegmentAssert.java:
##########
@@ -90,7 +91,28 @@ private static boolean spansEquals(List<Span> excepted, List<Span> actual) {
try {
spanEquals(exceptedSpan, actualSpan);
} catch (AssertFailedException e) {
- throw new SpanAssertFailedException(e, exceptedSpan, actualSpan);
+ if (Objects.equals(exceptedSpan.spanId(), actualSpan.spanId())) {
+ throw new SpanAssertFailedException(e, exceptedSpan, actualSpan);
+ }
+ // if the span id is not equals, trying to find the span by span id
+ Span tmpSpan = null;
+ for (Span s : actual) {
+ if (Objects.equals(exceptedSpan.spanId(), s.spanId())) {
Review Comment:
Is `exceptedSpan.spanId()` an expression ? @mrproliu
--
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-agent-test-tool] mrproliu commented on pull request #52: Enhance the segment span validation
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#issuecomment-1635187814
> I believe it doesn't affect the existing validation because we have been assuming the spans are in order.
Sure, I could do an update for this.
> BTW, why don't we switch to the infra-e2e project for validation?
They have different processes, such as agent tests needing to change framework versions in customized configuration files, and they contain different health checkers, etc.
f we want to switch to infra-e2e, there may still be a lot of customization work that needs to be done, but I will consider how to handle it. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-agent-test-tool] mrproliu commented on pull request #52: Enhance the segment span validation
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#issuecomment-1635174508
Yes, We could. I'm just concerned about whether it will affect the existing validation. If you think sorting is enough, I can change it to validate after sorting.
--
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-agent-test-tool] mrproliu commented on pull request #52: Enhance the segment span validation
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #52:
URL: https://github.com/apache/skywalking-agent-test-tool/pull/52#issuecomment-1641711966
The GHA is always Queued, so I cannot merge this PR. Let me create another PR for trying to run the CI.
--
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