You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "JamesSunXX (via GitHub)" <gi...@apache.org> on 2023/08/04 07:56:16 UTC
[GitHub] [skywalking-go] JamesSunXX opened a new pull request, #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
JamesSunXX opened a new pull request, #86:
URL: https://github.com/apache/skywalking-go/pull/86
`c.AddHook(newRedisHook(""))` in cluster client, ring client may lead err msg: "go-redis :skyWalking failed to create exit span, got error: parameter are nil"
--
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-go] JamesSunXX commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1666675253
I need create redis service containers in workflows.
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1665488285
Please focus on this CI: https://github.com/apache/skywalking-go/actions/runs/5759830947/job/15614675582?pr=86
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1678540577
Please follow [this section of documentation](https://skywalking.apache.org/docs/skywalking-go/next/en/development-and-contribution/development-guide/#tracing-context-operation) to operate tracing context.
--
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-go] mrproliu commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288186111
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,99 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster slots
Review Comment:
Please don't add the blank in the operation name, they should separate by '/'. Also, whey their have so many no entry segment(only exit span).
--
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-go] JamesSunXX commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1671454061
docker-compose doesn't support boolean data types. In this commit, I fix 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-go] wu-sheng commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288251101
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,99 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster slots
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: cluster slots }
+ - { key: cache.key, value: slots }
+ - { key: cache.args, value: 'cluster slots: []' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: not null
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: GET:/health
Review Comment:
It would not. The tool would only check declared, not the exact match.
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1666678077
Then, I think you should mock the data when check the data in the UT.
If you want to startup a full redis container for checking the updates, you should update the plugin test instead of UT.
--
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-go] JamesSunXX commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288204580
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,99 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster slots
Review Comment:
redis command `cluster slots` is a full operationName. I will handle this action.I find this original PR contributor only use `CreateExitSpan` handle redis hook.
--
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-go] wu-sheng commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672438950
We had cost a lot of resources on the CI process. Why didn't you run the tests and make sure it could pass locally?
--
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-go] jcchavezs commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288103098
##########
test/plugins/scenarios/go-redisv9/main.go:
##########
@@ -101,6 +107,19 @@ func TestPipelineSetAndGet(ctx context.Context) error {
}
func main() {
+ connectSingleRedis()
+ connectClusterRedis()
+
+ http.HandleFunc("/execute", executeHandler)
+
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+ writer.WriteHeader(http.StatusOK)
Review Comment:
totally, what I am saying is that you don't need to write 200.
--
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-go] jcchavezs commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672872204
> Do all commits made by the user in their own branch automatically trigger CI? Will this result in a significant consumption of GHA resources and frequent receipt of CI emails from GitHub Actions for the user?
Nope. A pre-commit hooks runs locally, run all targets you decide in local before the commit can be made **in local**. See https://pre-commit.com/
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1665250382
The CI failed, please 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
[GitHub] [skywalking-go] mrproliu commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288317240
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,82 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster/slots
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: cluster slots }
+ - { key: cache.key, value: slots }
+ - { key: cache.args, value: 'cluster slots: []' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: not null
Review Comment:
Could you please provide the peer address?
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672684288
> I did run the tests locally and it pass.Should I delete this commit change about the tests?
Please follow the GHA document to test locally. For example, the lint errors should still be prompted on your local machine.
--
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-go] JamesSunXX commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672794877
(base) ➜ plugins git:(main) ✗ ./run.sh go-redisv9
+++ dirname ./run.sh
++ cd .
++ pwd
+ home=/Users/android/Downloads/skywalking-go/test/plugins
+ scenario_name=
+ debug_mode=off
+ cleanup=off
+ scenarios_home=/Users/android/Downloads/skywalking-go/test/plugins/scenarios
+ num_of_testcases=0
++ date +%s
+ start_stamp=1691654837
+ export -f replace
+ parse_commandline go-redisv9
+ test 1 -gt 0
+ _key=go-redisv9
+ case "$_key" in
+ scenario_name=go-redisv9
+ shift
+ test 0 -gt 0
+ [[ off == \o\n ]]
+ test -z go-redisv9
+ scenario_home=/Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9
+ configuration=/Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ [[ ! -f /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml ]]
++ yq e .framework /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ framework_name=github.com/redis/go-redis/v9
+ '[' -z github.com/redis/go-redis/v9 ']'
++ yq e '.support-version | length' /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ support_version_count=2
+ '[' 2 -eq 0 ']'
+ index=0
+ '[' 0 -lt 2 ']'
++ yq e '.support-version[0].go' /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ go_version=1.17
++ yq e '.support-version[0].framework | length' /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ framework_count=1
+ '[' -z 1.17 ']'
+ [[ github.com/redis/go-redis/v9 != \g\o ]]
+ '[' 1 -eq 0 ']'
+ index=1
+ '[' 1 -lt 2 ']'
++ yq e '.support-version[1].go' /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ go_version=1.18
++ yq e '.support-version[1].framework | length' /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ framework_count=2
+ '[' -z 1.18 ']'
+ [[ github.com/redis/go-redis/v9 != \g\o ]]
+ '[' 2 -eq 0 ']'
+ index=2
+ '[' 2 -lt 2 ']'
+ workspace=/Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9
+ [[ -d /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9 ]]
+ rm -rf /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9
+ plugin_runner_helper=/Users/android/Downloads/skywalking-go/test/plugins/dist/runner-helper
+ [[ ! -f /Users/android/Downloads/skywalking-go/test/plugins/dist/runner-helper ]]
+ go_agent=/Users/android/Downloads/skywalking-go/test/plugins/dist/skywalking-go-agent
+ [[ ! -f /Users/android/Downloads/skywalking-go/test/plugins/dist/skywalking-go-agent ]]
+ yq e '.support-version[].go' /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ read -r go_version
++ yq e '.support-version[] | select(.go == "1.17") | .framework[]' /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml
+ frameworks=v9.0.3
+ [[ github.com/redis/go-redis/v9 == \g\o ]]
+ for framework_version in '$frameworks'
+ echo 'ready to run test case: go-redisv9 with go version: 1.17 and framework version: v9.0.3'
ready to run test case: go-redisv9 with go version: 1.17 and framework version: v9.0.3
+ case_name=go1.17-v9.0.3
+ case_home=/Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3
+ case_logs=/Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3/logs
+ mkdir -p /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3
+ mkdir -p /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3/logs
+ cp -rf /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/bin /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/config /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/go.mod /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/go.sum /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/main.go /Users/android/Downloads/skywalking-go/test/plugins/scenarios/go-redisv9/plugin.yml /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3
+ cd /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3
++ echo go1.17-v9.0.3
++ sed 's/\//_/g; s/\./_/g; s/-/_/g'
+ mod_case_name=go1_17_v9_0_3
++ head -n 1 go.mod
++ cut -d ' ' -f 2
+ mod_name=test/plugins/scenarios/go-redisv9
+ replace 's/^go [0-9]*\.[0-9]*/go 1.17/' go.mod
+ '[' 2 -lt 2 ']'
+ opt=
+ cmd='s/^go [0-9]*\.[0-9]*/go 1.17/'
+ file=go.mod
+ '[' 2 -eq 3 ']'
++ uname
+ '[' Darwin = Darwin ']'
+ sed -i '' 's/^go [0-9]*\.[0-9]*/go 1.17/' go.mod
+ replace 's/^module /module go1_17_v9_0_3\//' go.mod
+ '[' 2 -lt 2 ']'
+ opt=
+ cmd='s/^module /module go1_17_v9_0_3\//'
+ file=go.mod
+ '[' 2 -eq 3 ']'
++ uname
+ '[' Darwin = Darwin ']'
+ sed -i '' 's/^module /module go1_17_v9_0_3\//' go.mod
+ find . -name '*.go' -type f -exec bash -c 'replace "s|test/plugins/scenarios/go-redisv9|go1_17_v9_0_3/test/plugins/scenarios/go-redisv9|g" "{}"' ';'
+ replace -E '/^replace/ s#(\.\./)#\1../#' go.mod
+ '[' 3 -lt 2 ']'
+ opt=
+ cmd=-E
+ file='/^replace/ s#(\.\./)#\1../#'
+ '[' 3 -eq 3 ']'
+ opt=-E
+ cmd='/^replace/ s#(\.\./)#\1../#'
+ file=go.mod
++ uname
+ '[' Darwin = Darwin ']'
+ sed -i '' -E '/^replace/ s#(\.\./)#\1../#' go.mod
+ [[ v9.0.3 != \n\a\t\i\v\e ]]
+ go get github.com/redis/go-redis/v9@v9.0.3
go: downgraded github.com/redis/go-redis/v9 v9.0.5 => v9.0.3
+ /Users/android/Downloads/skywalking-go/test/plugins/dist/runner-helper -workspace /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3 -project /Users/android/Downloads/skywalking-go/test/plugins/../../ -go-version 1.17 -scenario go-redisv9 -case go1.17-v9.0.3 -debug off -go-agent /Users/android/Downloads/skywalking-go/test/plugins/dist/skywalking-go-agent
2023/08/10 16:07:18 helper execute args: [/Users/android/Downloads/skywalking-go/test/plugins/dist/runner-helper -workspace /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3 -project /Users/android/Downloads/skywalking-go/test/plugins/../../ -go-version 1.17 -scenario go-redisv9 -case go1.17-v9.0.3 -debug off -go-agent /Users/android/Downloads/skywalking-go/test/plugins/dist/skywalking-go-agent]
+ echo 'staring the testcase go-redisv9, go1.17-v9.0.3'
staring the testcase go-redisv9, go1.17-v9.0.3
+ bash /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3/scenarios.sh
++ docker-compose version --short
+ compose_version=2.20.2-desktop.1
+ [[ 2.20.2-desktop.1 =~ ^(v)?1 ]]
+ [[ 2.20.2-desktop.1 =~ ^(v)?2 ]]
+ separator=-
++ echo go-redisv9
++ sed -e 's/\.//g'
++ awk '{print tolower($0)}'
+ project_name=go-redisv9
+ service_container_name=go-redisv9-service-1
+ validator_container_name=go-redisv9-validator-1
+ set +ex
[+] Running 5/5
✔ Container go-redisv9-redis-cluster-server-1 Healthy5.8s
✔ Container go-redisv9-oap-1 Healthy5.8s
✔ Container go-redisv9-redis-single-server-1 Healthy6.2s
✔ Container go-redisv9-service-1 Healthy77.4s
✔ Container go-redisv9-validator-1 Started78.3s
+ sleep 3
++ docker ps -aqf name=go-redisv9-validator-1
+ validator_container_id=f248b390d191
++ docker wait f248b390d191
+ status=1
+ [[ -z f248b390d191 ]]
+ [[ 1 -ne 0 ]]
+ docker logs f248b390d191
+ HTTP_HOST=service
+ HTTP_PORT=8080
+ echo 'Checking the service health status...'
+ healthCheck http://service:8080/health
+ HEALTH_CHECK_URL=http://service:8080/health
+ STATUS=
+ TIMES=60
+ i=1
+ [[ 1 -lt 60 ]]
Checking the service health status...
++ curl --max-time 3 -is http://service:8080/health
++ grep -oE 'HTTP/.*\s+200'
http://service:8080/health: HTTP/1.1 200
Visiting entry service...
+ STATUS='HTTP/1.1 200'
+ [[ -n HTTP/1.1 200 ]]
+ echo 'http://service:8080/health: HTTP/1.1 200'
+ return 0
+ echo 'Visiting entry service...'
++ echo curl -s --max-time 3 http://service:8080/execute
+ curl -s --max-time 3 http://service:8080/execute
+ sleep 5
+ echo 'Receiving actual data...'
execute redis op successReceiving actual data...
+ curl -s --max-time 3 http://oap:12800/receiveData
+ [[ ! -f /workspace/config/actual.yaml ]]
+ echo 'Validating actual data...'
Validating actual data...
++ curl -X POST --data-binary @/workspace/config/excepted.yml -s -w '\n%{http_code}' http://oap:12800/dataValidate
+ response='
500'
++ echo '
500'
++ tail -n1
+ status_code=500
++ echo '
500'
++ head -n -1
+ response_body=
+ '[' 500 -ne 200 ']'
+ exitOnError 'go-redisv9-go1.17-v9.0.3, validate actual data failed! \n'
+ echo -e '\033[31m[ERROR] go-redisv9-go1.17-v9.0.3, validate actual data failed! \n\033[0m'
[ERROR] go-redisv9-go1.17-v9.0.3, validate actual data failed!
+ exit 1
+ docker logs go-redisv9-service-1
WORK=/tmp/go-build3848096159
skywalking-go 2023/08/10 08:18:23 open stream error rpc error: code = Unavailable desc = connection error: desc = "error reading server preface: read tcp 172.31.0.5:37524->172.31.0.4:19876: use of closed network connection"
skywalking-go 2023/08/10 08:18:23 open stream error rpc error: code = Unavailable desc = connection error: desc = "error reading server preface: read tcp 172.31.0.5:37524->172.31.0.4:19876: use of closed network connection"
skywalking-go 2023/08/10 08:18:23 open stream error rpc error: code = Unavailable desc = connection error: desc = "error reading server preface: read tcp 172.31.0.5:37524->172.31.0.4:19876: use of closed network connection"
skywalking-go 2023/08/10 08:18:23 report serviceInstance properties error rpc error: code = Unavailable desc = connection error: desc = "error reading server preface: read tcp 172.31.0.5:37524->172.31.0.4:19876: use of closed network connection"
skywalking-go 2023/08/10 08:18:23 fetch dynamic configuration error rpc error: code = Unavailable desc = connection error: desc = "error reading server preface: read tcp 172.31.0.5:37524->172.31.0.4:19876: use of closed network connection"
2023/08/10 08:18:25 excute test case set_and_get_single
2023/08/10 08:18:25 excute test case pipeline_set_and_get_single
2023/08/10 08:18:25 excute test case set_and_get_cluster
2023/08/10 08:18:25 excute test case pipeline_set_and_get_cluster
+ docker-compose -p go-redisv9 -f /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3/docker-compose.yml kill
[+] Killing 4/4
✔ Container go-redisv9-oap-1 Killed0.6s
✔ Container go-redisv9-redis-cluster-server-1 Killed0.4s
✔ Container go-redisv9-service-1 Killed0.5s
✔ Container go-redisv9-redis-single-server-1 Killed0.6s
+ docker-compose -p go-redisv9 -f /Users/android/Downloads/skywalking-go/test/plugins/workspace/go-redisv9/go1.17-v9.0.3/docker-compose.yml rm -f
[+] Removing 5/5
✔ Container go-redisv9-oap-1 Removed0.1s
✔ Container go-redisv9-validator-1 Removed0.2s
✔ Container go-redisv9-redis-cluster-server-1 Removed0.2s
✔ Container go-redisv9-service-1 Removed0.9s
✔ Container go-redisv9-redis-single-server-1 Removed0.2s
+ exit 1
--
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-go] JamesSunXX commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1665429751
This is my first PR.
I check this CI log. I can't find any useful log.
<img width="326" alt="image" src="https://github.com/apache/skywalking-go/assets/38456335/9092f76a-0fe6-49db-9935-249a5e15934b">
--
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-go] JamesSunXX commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288129713
##########
test/plugins/scenarios/go-redisv9/main.go:
##########
@@ -101,6 +107,19 @@ func TestPipelineSetAndGet(ctx context.Context) error {
}
func main() {
+ connectSingleRedis()
+ connectClusterRedis()
+
+ http.HandleFunc("/execute", executeHandler)
+
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+ writer.WriteHeader(http.StatusOK)
Review Comment:
I understand. If I don't register this "/health" route, it indeed returns a 200 status code.I will delete this route register.
--
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-go] mrproliu commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288160115
##########
test/plugins/scenarios/go-redisv9/main.go:
##########
@@ -101,6 +107,19 @@ func TestPipelineSetAndGet(ctx context.Context) error {
}
func main() {
+ connectSingleRedis()
+ connectClusterRedis()
+
+ http.HandleFunc("/execute", executeHandler)
+
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+ writer.WriteHeader(http.StatusOK)
Review Comment:
@JamesSunXX Please don't delete the `/health` route, we need to [access it to check the application has been set up success](https://github.com/apache/skywalking-go/blob/main/test/plugins/scenarios/go-redisv9/plugin.yml#L18). @jcchavezs means you don't need to write `writer.WriteHeader(http.StatusOK)`(only this line).
--
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-go] mrproliu commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288160115
##########
test/plugins/scenarios/go-redisv9/main.go:
##########
@@ -101,6 +107,19 @@ func TestPipelineSetAndGet(ctx context.Context) error {
}
func main() {
+ connectSingleRedis()
+ connectClusterRedis()
+
+ http.HandleFunc("/execute", executeHandler)
+
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+ writer.WriteHeader(http.StatusOK)
Review Comment:
@JamesSunXX Please don't delete the `/health` route, we need to access it to check the application has been set up success. @jcchavezs means you don't need to write `writer.WriteHeader(http.StatusOK)`(only this line).
--
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-go] JamesSunXX commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288538041
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,82 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster/slots
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: cluster slots }
+ - { key: cache.key, value: slots }
+ - { key: cache.args, value: 'cluster slots: []' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: not null
Review Comment:
The image `bitnami/redis-cluster` provides environment variables `REDIS_CLUSTER_ANNOUNCE_IP `, but after trying many times, I couldn't successfully initialize the cluster redis with the specified IP. Also, the Go service couldn't access redis.
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672834214
> Yeah ideally the person writing the PR should verify everything in local. What I am seeking here is to be defensive in the way we incur in CI costs.
>
> About running tests in local. Would it be a good idea to have a pre-commit hook? This would trigger tests before commit which is handy and automated.
Do all commits made by the user in their own branch automatically trigger CI? Will this result in a significant consumption of GHA resources and frequent receipt of CI emails from GitHub Actions for the user?
--
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-go] mrproliu commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288187451
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,99 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster slots
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: cluster slots }
+ - { key: cache.key, value: slots }
+ - { key: cache.args, value: 'cluster slots: []' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: not null
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: GET:/health
Review Comment:
We don't need to verify the 'GET:/health' segment, please delete 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-go] JamesSunXX commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288098689
##########
test/plugins/scenarios/go-redisv9/main.go:
##########
@@ -101,6 +107,19 @@ func TestPipelineSetAndGet(ctx context.Context) error {
}
func main() {
+ connectSingleRedis()
+ connectClusterRedis()
+
+ http.HandleFunc("/execute", executeHandler)
+
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+ writer.WriteHeader(http.StatusOK)
Review Comment:
I noticed that there is a "/health" route written in each plugin file. This is probably used to check the health status of the service, right?
--
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-go] wu-sheng commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672479171
You should pass the plugin tests locally, not just test the codes manually.
All steps on CI are runnable locally.
Your failures show your plugin test codes are not verified by yourself.
You made us to run those dozens of times, I don't think it makes sense.
--
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-go] wu-sheng commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672847039
> Do all commits made by the user in their own branch automatically trigger CI?
For now, it wouldn't. But we can't see the result of CI, so does the contributor.
> Will this result in a significant consumption of GHA resources and frequent receipt of CI emails from GitHub Actions for the user?
It costs the slots in the whole ASF org, which is why INFRA disabled the auto-running.
____
This is not the reason I am asking. The issue is, we have to approve the CI to run, in order to see whether the tests passed. But every time, it fails. I can't see the point of doing this.
These automatical tests are reproducible at the contributor's local env, so, CI is just for reviewers to confirm. If the users want to verify on CI and cost its now resources of CI available times, they could close the PR and run on their own PR. Then it is not upstream's concern.
--
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-go] JamesSunXX commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1677079985
```
- segmentId: cdc63b6a382911eeb5b20242ac120005.21.39034998715200004
spans:
- operationName: redis/pipeline
parentSpanId: -1
spanId: 0
spanLayer: Cache
startTime: 1691746271520
endTime: 1691746271521
componentId: 5014
isError: false
spanType: Exit
peer: 172.18.0.2:7001
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.cmd, value: 'pipeline:set/set'}
- segmentId: cdc63b6a382911eeb5b20242ac120005.21.39034998715210006
spans:
- operationName: redis/pipeline
parentSpanId: -1
spanId: 0
spanLayer: Cache
startTime: 1691746271521
endTime: 1691746271521
componentId: 5014
isError: false
spanType: Exit
peer: 172.18.0.2:7001
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.cmd, value: 'pipeline:get/get'}
- segmentId: cdc63b6a382911eeb5b20242ac120005.71.39034998715150002
spans:
- operationName: redis/set
parentSpanId: 0
spanId: 1
spanLayer: Cache
startTime: 1691746271516
endTime: 1691746271517
componentId: 5014
isError: false
spanType: Exit
peer: redis-server:6379
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.op, value: write}
- {key: cache.cmd, value: set}
- {key: cache.key, value: key_TestSetAndGet}
- {key: cache.args, value: 'set key_TestSetAndGet value_TestSetAndGet ex 10: '}
- operationName: redis/get
parentSpanId: 0
spanId: 2
spanLayer: Cache
startTime: 1691746271517
endTime: 1691746271517
componentId: 5014
isError: false
spanType: Exit
peer: redis-server:6379
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.op, value: read}
- {key: cache.cmd, value: get}
- {key: cache.key, value: key_TestSetAndGet}
- {key: cache.args, value: 'get key_TestSetAndGet: '}
- operationName: redis/pipeline
parentSpanId: 0
spanId: 3
spanLayer: Cache
startTime: 1691746271518
endTime: 1691746271518
componentId: 5014
isError: false
spanType: Exit
peer: redis-server:6379
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.cmd, value: 'pipeline:set/set'}
- operationName: redis/pipeline
parentSpanId: 0
spanId: 4
spanLayer: Cache
startTime: 1691746271518
endTime: 1691746271518
componentId: 5014
isError: false
spanType: Exit
peer: redis-server:6379
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.cmd, value: 'pipeline:get/get'}
- operationName: redis/set
parentSpanId: 0
spanId: 5
spanLayer: Cache
startTime: 1691746271519
endTime: 1691746271519
componentId: 5014
isError: false
spanType: Exit
peer: 172.18.0.2:7001
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.op, value: write}
- {key: cache.cmd, value: set}
- {key: cache.key, value: key_TestSetAndGet}
- {key: cache.args, value: 'set key_TestSetAndGet value_TestSetAndGet ex 10: '}
- operationName: redis/get
parentSpanId: 0
spanId: 6
spanLayer: Cache
startTime: 1691746271519
endTime: 1691746271519
componentId: 5014
isError: false
spanType: Exit
peer: 172.18.0.2:7001
skipAnalysis: false
tags:
- {key: cache.type, value: redis}
- {key: cache.op, value: read}
- {key: cache.cmd, value: get}
- {key: cache.key, value: key_TestSetAndGet}
- {key: cache.args, value: 'get key_TestSetAndGet: '}
- operationName: GET:/execute
parentSpanId: -1
spanId: 0
spanLayer: Http
startTime: 1691746271515
endTime: 1691746271521
componentId: 5004
isError: false
spanType: Entry
peer: ''
skipAnalysis: false
tags:
- {key: http.method, value: GET}
- {key: url, value: 'service:8080/execute'}
- {key: status_code, value: '200'}
```
The information above consists of actual collected data, and the code logic below is from testing. Why does the pipeline operation on the Redis cluster appear in two different segmentIds? Shouldn't this log data be within the same segmentId?
```
func executeHandler(w http.ResponseWriter, r *http.Request) {
testCases := []struct {
name string
client v9.UniversalClient
fn testFunc
}{
{"set_and_get_single", rdb, TestSetAndGet},
{"pipeline_set_and_get_single", rdb, TestPipelineSetAndGet},
{"set_and_get_cluster", clusterDB, TestSetAndGet},
{"pipeline_set_and_get_cluster", clusterDB, TestPipelineSetAndGet},
}
for _, test := range testCases {
log.Printf("excute test case %s", test.name)
if err := test.fn(r.Context(), test.client); err != nil {
log.Fatalf("test case %s failed: %v", test.name, err)
}
}
_, _ = w.Write([]byte("execute redis op success"))
}
```
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672829807
So, why did you upload the plugin test log? The plugin test code is there, your need to deep look at it why it returns `500`.
--
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-go] JamesSunXX commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672477211
I did run the tests locally and it pass.Should I delete this commit change about the tests?
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672679959
Hi, @jcchavezs . Thanks for your advice.
> - only run tests on the plugin that changed.
> - adds go cache to cache test results
That sounds good. But there maybe have some plugin dependencies another plugin, such as `kratos` dependency on the native `http` and `grpc` frameworks. We cannot make sure all the frameworks/plugins are independent, so we needs to check them all.
> - add ephimeral cache to artifacts that are being downloaded
We don't have so many files that need to be downloaded, so could be ignored.
> - review the usage of `always()` as it will run steps even if previous failed
We already using the `needs` config for some job to make sure only run after the dependency job are successful.
> - verify if you have prerequisites in jobs so you don't run something that you know on beforehand it will fail.
We need the person creating PRs to test the CI locally in order to save 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
[GitHub] [skywalking-go] wu-sheng closed pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng closed pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
URL: https://github.com/apache/skywalking-go/pull/86
--
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-go] wu-sheng commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1689245282
There is no update for one week.
--
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-go] JamesSunXX commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1678346881
In the cluster redis, the hook in the `pipeline` command will be triggered in another goroutine. Is there any way to identify the same segmentId?
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1665180596
You need to keep one PR, please don't re-create 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-go] jcchavezs commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288092636
##########
test/plugins/scenarios/go-redisv9/main.go:
##########
@@ -101,6 +107,19 @@ func TestPipelineSetAndGet(ctx context.Context) error {
}
func main() {
+ connectSingleRedis()
+ connectClusterRedis()
+
+ http.HandleFunc("/execute", executeHandler)
+
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+ writer.WriteHeader(http.StatusOK)
Review Comment:
Is this needed?I believe if you don't do anything it will also return 200.
--
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-go] jcchavezs commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288103098
##########
test/plugins/scenarios/go-redisv9/main.go:
##########
@@ -101,6 +107,19 @@ func TestPipelineSetAndGet(ctx context.Context) error {
}
func main() {
+ connectSingleRedis()
+ connectClusterRedis()
+
+ http.HandleFunc("/execute", executeHandler)
+
+ http.HandleFunc("/health", func(writer http.ResponseWriter, request *http.Request) {
+ writer.WriteHeader(http.StatusOK)
Review Comment:
totally, what I am saying is that you don't need to write 200, it will by default.
--
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-go] JamesSunXX commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "JamesSunXX (via GitHub)" <gi...@apache.org>.
JamesSunXX commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288239287
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,99 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster slots
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: cluster slots }
+ - { key: cache.key, value: slots }
+ - { key: cache.args, value: 'cluster slots: []' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: not null
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: GET:/health
Review Comment:
If I delete `GET:/health` segment, it will validate actual data failed.
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1677101633
This depends on whether your interception point is in the same goroutine (or goroutine created by the current HTTP handler) as the current consumer's access address. If the interception point is in a different goroutine, it will be impossible to determine their correlation.
--
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-go] mrproliu commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1288186111
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,99 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster slots
Review Comment:
Please don't add the blank in the operation name, they should separate by '/'. Also, why do there have so many no-entry segments (only exit span)?
--
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-go] mrproliu commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1671121403
Looks like the CI failure, please re-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
[GitHub] [skywalking-go] jcchavezs commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672687031
Yeah ideally the person writing the PR should verify everything in local. What I am seeking here is to be defensive in the way we incur in CI costs.
About running tests in local. Would it be a good idea to have a pre-commit hook? This would trigger tests before commit which is handy and automated.
--
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-go] mrproliu commented on a diff in pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "mrproliu (via GitHub)" <gi...@apache.org>.
mrproliu commented on code in PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#discussion_r1289621931
##########
test/plugins/scenarios/go-redisv9/config/excepted.yml:
##########
@@ -18,6 +18,82 @@ segmentItems:
- serviceName: go-redisv9
segmentSize: ge 1
segments:
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:6379
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: ping }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: not null }
+ - segmentId: not null
+ spans:
+ - operationName: redis/command
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: command }
+ - { key: cache.key, value: '' }
+ - { key: cache.args, value: 'command: map[]' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/cluster/slots
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: redis-server:7001
+ skipAnalysis: false
+ tags:
+ - { key: cache.type, value: redis }
+ - { key: cache.op, value: '' }
+ - { key: cache.cmd, value: cluster slots }
+ - { key: cache.key, value: slots }
+ - { key: cache.args, value: 'cluster slots: []' }
+ - segmentId: not null
+ spans:
+ - operationName: redis/ping
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Cache
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 5014
+ isError: false
+ spanType: Exit
+ peer: not null
Review Comment:
My intention is not the IP address, after testing it should be the address filled in the program. Isn't 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-go] jcchavezs commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "jcchavezs (via GitHub)" <gi...@apache.org>.
jcchavezs commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672629292
I suggest a few things here:
- only run tests on the plugin that changed.
- adds go cache to cache test results
- add ephimeral cache to artifacts that are being downloaded
- review the usage of `always()` as it will run steps even if previous
failed
- verify if you have prerequisites in jobs so you don't run something that
you know on beforehand it will fail.
On Thu, 10 Aug 2023, 05:00 吴晟 Wu Sheng, ***@***.***> wrote:
> You should pass the plugin tests locally, not just test the codes manually.
>
> All steps on CI are runnable locally.
>
> Your failures show your plugin test codes are not verified by yourself.
> You made us to run those dozens of times, I don't think it makes sense.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/skywalking-go/pull/86#issuecomment-1672479171>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAXOYAQFAQHMBDGVXEBWBILXURFEVANCNFSM6AAAAAA3D2ZWEU>
> .
> You are receiving this because you were mentioned.Message 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-go] wu-sheng commented on pull request #86: Fix redis cluster, ring cluster lead err msg. Add Redis Unit Test
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #86:
URL: https://github.com/apache/skywalking-go/pull/86#issuecomment-1672877305
> > Do all commits made by the user in their own branch automatically trigger CI? Will this result in a significant consumption of GHA resources and frequent receipt of CI emails from GitHub Actions for the user?
>
> Nope. A pre-commit hooks runs locally, run all targets you decide in local before the commit can be made **in local**. See https://pre-commit.com/
pre-hook is local, but with the increment of the plugins(look at Java agent plugin list), every commit will become a pain.
I don't want that burden for everyone. If we look at the history of all language agents contributors, most of them are doing well by following testing locally first, rather than a forcibly pre-commit hook setup.
--
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