You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/04/13 17:04:44 UTC
[GitHub] [skywalking] arugal opened a new pull request #4647: support `java`
-> `go2sky` -> `java` e2e test, and v3 protocol
arugal opened a new pull request #4647: support `java` -> `go2sky` -> `java` e2e test, and v3 protocol
URL: https://github.com/apache/skywalking/pull/4647
Please answer these questions before submitting pull request
- Why submit this pull request?
- [ ] Bug fix
- [ ] New feature provided
- [ ] Improve performance
- Related issues
https://github.com/apache/skywalking/issues/4618
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io commented on issue #4647: Add `java` ->
`go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#issuecomment-613765234
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4647?src=pr&el=h1) Report
> Merging [#4647](https://codecov.io/gh/apache/skywalking/pull/4647?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/ecd7d997998684512feee92b928cf6b9aab39453&el=desc) will **decrease** coverage by `0.34%`.
> The diff coverage is `n/a`.
[![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4647/graphs/tree.svg?width=650&height=150&src=pr&token=qrILxY5yA8)](https://codecov.io/gh/apache/skywalking/pull/4647?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4647 +/- ##
==========================================
- Coverage 29.06% 28.72% -0.35%
==========================================
Files 1192 1192
Lines 26056 26056
Branches 3480 3480
==========================================
- Hits 7574 7484 -90
- Misses 17798 17890 +92
+ Partials 684 682 -2
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4647?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../apm/agent/core/remote/StandardChannelBuilder.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1N0YW5kYXJkQ2hhbm5lbEJ1aWxkZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../skywalking/apm/agent/core/remote/GRPCChannel.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsLmphdmE=) | `0.00% <0.00%> (-81.25%)` | :arrow_down: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [...lking/apm/agent/core/remote/TLSChannelBuilder.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RMU0NoYW5uZWxCdWlsZGVyLmphdmE=) | `0.00% <0.00%> (-44.45%)` | :arrow_down: |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `34.61% <0.00%> (-29.49%)` | :arrow_down: |
| [...apm/agent/core/remote/AuthenticationDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0F1dGhlbnRpY2F0aW9uRGVjb3JhdG9yLmphdmE=) | `0.00% <0.00%> (-22.23%)` | :arrow_down: |
| [...apm/agent/core/remote/ServiceManagementClient.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VNYW5hZ2VtZW50Q2xpZW50LmphdmE=) | `33.96% <0.00%> (-15.10%)` | :arrow_down: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `47.36% <0.00%> (-3.51%)` | :arrow_down: |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `72.60% <0.00%> (-2.74%)` | :arrow_down: |
| [.../agent/core/profile/ProfileTaskChannelService.java](https://codecov.io/gh/apache/skywalking/pull/4647/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0NoYW5uZWxTZXJ2aWNlLmphdmE=) | `20.87% <0.00%> (-2.20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4647?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4647?src=pr&el=footer). Last update [ecd7d99...5fb4473](https://codecov.io/gh/apache/skywalking/pull/4647?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r407869293
##########
File path: test/e2e/e2e-test/docker/go/go2sky-e2e/go.sum
##########
@@ -0,0 +1,79 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
I know this file is needed, but not hosted
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r407827122
##########
File path: test/e2e/e2e-test/docker/go/go2sky-e2e/go.sum
##########
@@ -0,0 +1,79 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
Do we really need this file? I think you have the go2sky commitid, should have these.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r407867051
##########
File path: test/e2e/e2e-test/docker/go/go2sky-e2e/go.sum
##########
@@ -0,0 +1,79 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
> Do we really need this file?
`go.sum` should be kept.
https://golang.org/cmd/go/#hdr-Module_authentication_using_go_sum
> I think you have the go2sky commitid, should have these already, if we run go vendor or something in the CI process.
I depend on `go2sky` with the [pseudo-version](https://golang.org/cmd/go/#hdr-Pseudo_versions), which is actually the same as the `commit-id` effect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r407871458
##########
File path: test/e2e/e2e-test/docker/go/go2sky-e2e/go.sum
##########
@@ -0,0 +1,79 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
I think you haven't followed my point. Let's talk in this way, one day, user came to the go2sky repo, he didn't need anything rather than standard Go env. Right? He could do git clone, And more commands to make things ready. Now, we have commit ID file, we should not maintain this file in main repo.
Is this clear now?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r408515870
##########
File path: test/e2e/e2e-test/docker/go/Dockerfile.go
##########
@@ -13,16 +13,24 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-FROM golang:1.12
+FROM golang:1.12 AS builder
+
+ARG COMMIT_HASH=fdb185d66faddad1651c18150d63bae32610f3ac
+ARG GO2SKY_REPO=${COMMIT_HASH}.tar.gz
Review comment:
`${COMMIT_HASH}.tar.gz` is not `GO2SKY_REPO`, `https://github.com/SkyAPM/go2sky/archive/${GO2SKY_REPO}` is
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4647: support `java` ->
`go2sky` -> `java` e2e test, and v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4647: support `java` -> `go2sky` -> `java` e2e test, and v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#issuecomment-613171735
You just add our 100th 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r407869189
##########
File path: test/e2e/e2e-test/docker/go/go2sky-e2e/go.sum
##########
@@ -0,0 +1,79 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
My point is, you have the go2sky commit I'd already, you should be able to regenerate all these. Otherwise, how users use our repo?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng merged pull request #4647: Add `java` ->
`go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r407870326
##########
File path: test/e2e/e2e-test/docker/go/go2sky-e2e/go.sum
##########
@@ -0,0 +1,79 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
Users can use it by adding it to [go.mod](https://github.com/arugal/skywalking/blob/5470877696289f77b40d1debb6f454417b003d27/test/e2e/e2e-test/docker/go/go2sky-e2e/go.mod#L5).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request
#4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r408515870
##########
File path: test/e2e/e2e-test/docker/go/Dockerfile.go
##########
@@ -13,16 +13,24 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-FROM golang:1.12
+FROM golang:1.12 AS builder
+
+ARG COMMIT_HASH=fdb185d66faddad1651c18150d63bae32610f3ac
+ARG GO2SKY_REPO=${COMMIT_HASH}.tar.gz
Review comment:
`${COMMIT_HASH}.tar.gz` is not `GO2SKY_REPO`, `https://github.com/SkyAPM/go2sky/archive/${GO2SKY_REPO}` is
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r408520271
##########
File path: test/e2e/e2e-test/docker/go/Dockerfile.go
##########
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+FROM golang:1.12 AS builder
+
+ARG COMMIT_HASH=fdb185d66faddad1651c18150d63bae32610f3ac
Review comment:
We need to update this once go2sky updated, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r408528222
##########
File path: test/e2e/e2e-test/docker/go/Dockerfile.go
##########
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+FROM golang:1.12 AS builder
+
+ARG COMMIT_HASH=fdb185d66faddad1651c18150d63bae32610f3ac
Review comment:
Yes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4647:
Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4647: Add `java` -> `go2sky` -> `java` e2e test case, and adapt v3 protocol
URL: https://github.com/apache/skywalking/pull/4647#discussion_r407827122
##########
File path: test/e2e/e2e-test/docker/go/go2sky-e2e/go.sum
##########
@@ -0,0 +1,79 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
Do we really need this file? I think you have the go2sky commitid, should have these already, if we run go vendor or something in the CI process.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services