You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "youjie23 (via GitHub)" <gi...@apache.org> on 2025/10/23 04:52:35 UTC
[PR] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
youjie23 opened a new pull request, #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134
Since some existing test cases rely on the behavior described in #132, we need to maintain compatibility with them to avoid extensive modifications to the e2e tests. Correspondingly, we will only modify the necessary test cases by explicitly setting the `stop-on-reach-times` configuration to true
--
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] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134#issuecomment-3435466463
That looks weird to me. Do we need to fix many test cases? Can we fix them with some script?
--
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] feat: allow `times` to be <= 0 to simulate endless trigger [skywalking-infra-e2e]
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 merged PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134
--
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] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
Posted by "youjie23 (via GitHub)" <gi...@apache.org>.
youjie23 commented on PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134#issuecomment-3455486879
> The `times` is expected to control how many times the call should be triggered, the test case you mentioned worked before because of this bug, and since we fixed this bug we should adjust the test case instead of adding a new opposite config to make it work again.
>
> > On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the alert recovery detection scenario(#13539). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.
>
> I think we can set the times to a non-positive value like (-1 or 0) to indicate never stop (if this doesn't already exist). Instead of adding a config `stop-on-reach-times` that had been expected to work with the `times` field.
I've pushed new commits to address the points you raised. Please take another look when you have a moment, and let me know if anything else needs adjustment.
--
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] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
Posted by "youjie23 (via GitHub)" <gi...@apache.org>.
youjie23 commented on PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134#issuecomment-3436532499
> That looks weird to me. Do we need to fix many test cases? Can we fix them with some script?
After the GHA run, I've identified approximately 10 test cases that require modification and I suspect there might be more to it.
On one hand, we could simply set the times value in all trigger configurations to a very high number. This would make the triggers run continuously throughout the entire end-to-end test verification period, behave as before. However, this adjustment seems a bit odd, doesn't it??
On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the alert recovery detection scenario([#13539](https://github.com/apache/skywalking/pull/13539)). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.
Therefore, my proposal is to add a new configuration option to explicitly control whether the trigger stops after reaching the specified times value. This provides the necessary flexibility for the alert recovery use case without affecting other scenarios.
Looking forward to your suggestions on this approach.
--
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] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134#issuecomment-3436596334
The `times` is expected to control how many times the call should be triggered, the test case you mentioned worked before because of this bug, and since we fixed this bug we should adjust the test case instead of adding a new opposite config to make it work again.
> On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the alert recovery detection scenario(#13539). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.
>
>
I think we can set the times to a non-positive value like (-1 or 0) to indicate never stop (if this doesn't already exist). Instead of adding a config `stop-on-reach-times` that had been expected to work with the `times` field.
--
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] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134#issuecomment-3435105245
I think we need to update docs about this config?
--
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] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
Posted by "youjie23 (via GitHub)" <gi...@apache.org>.
youjie23 commented on PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134#issuecomment-3437610244
> The `times` is expected to control how many times the call should be triggered, the test case you mentioned worked before because of this bug, and since we fixed this bug we should adjust the test case instead of adding a new opposite config to make it work again.
>
> > On the other hand, having the trigger run continuously is acceptable and works fine in other scenarios. The specific issue here is with the alert recovery detection scenario(#13539). This scenario requires a defined recovery time window, meaning the trigger needs to stop after a certain number of runs to accurately assess recovery.
>
> I think we can set the times to a non-positive value like (-1 or 0) to indicate never stop (if this doesn't already exist). Instead of adding a config `stop-on-reach-times` that had been expected to work with the `times` field.
Thanks for the feedback! I'll work on a revision and submit 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] Support `stop-on-reach-times` configuration in the trigger phase [skywalking-infra-e2e]
Posted by "youjie23 (via GitHub)" <gi...@apache.org>.
youjie23 commented on PR #134:
URL: https://github.com/apache/skywalking-infra-e2e/pull/134#issuecomment-3435360762
> I think we need to update docs about this config?
I've updated the corresponding documentation. Could you please take another look when you have a moment to see if it meets the requirements
--
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