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/02/23 02:59:46 UTC

[GitHub] [skywalking] JaredTan95 opened a new pull request #4399: support http api for upstream trace.

JaredTan95 opened a new pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399
 
 
   Please answer these questions before submitting pull request
   
   - Why submit this pull request?
   - [ ] Bug fix
   - [ ] New feature provided
   - [ ] Improve performance
   
   - Related issues
   
   ___
   ### Bug fix
   - Bug description.
   
   - How to fix?
   
   ___
   ### New feature or improvement
   - Describe the details and related test reports.
   
   ## TODO:
   docs 

----------------------------------------------------------------
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 edited a comment on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@446840c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   24.99%           
   =========================================
     Files             ?     1218           
     Lines             ?    28245           
     Branches          ?     3874           
   =========================================
     Hits              ?     7059           
     Misses            ?    20537           
     Partials          ?      649
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [446840c...3178f7e](https://codecov.io/gh/apache/skywalking/pull/4399?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] codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d714955`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   24.98%           
   =========================================
     Files             ?     1218           
     Lines             ?    28250           
     Branches          ?     3874           
   =========================================
     Hits              ?     7059           
     Misses            ?    20542           
     Partials          ?      649
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [d714955...bd5301a](https://codecov.io/gh/apache/skywalking/pull/4399?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] JaredTan95 edited a comment on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 edited a comment on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-591409979
 
 
   Yes, I rechecked and updated following by https://github.com/apache/skywalking/blob/master/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/pom.xml#L47, I used the `protobuf-java-util` latest version here, cause of there have some bug when dealing with enum type within  `3.5.1` version.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590315491
 
 
   They seem same, I don't know what is the difference.

----------------------------------------------------------------
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 edited a comment on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@ce46725`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   25.42%           
   =========================================
     Files             ?     1216           
     Lines             ?    28064           
     Branches          ?     3859           
   =========================================
     Hits              ?     7134           
     Misses            ?    20279           
     Partials          ?      651
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [ce46725...fa74d0d](https://codecov.io/gh/apache/skywalking/pull/4399?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] JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593739280
 
 
   By following @kezhenxu94 ,  oap and java app are mounted into e2e container. But, nginx could not connect to upstream only when Lua agent registering as the following configuration, and according to log, `upstream:9090/e2e/users` connected successfully.:
   
   ```conf
   http {
       lua_package_path "/usr/share/skywalking-nginx-lua/lib/skywalking/?.lua;;";
       # Buffer represents the register inform and the queue of the finished segment
       lua_shared_dict tracing_buffer 100m;
   
       # Init is the timer setter and keeper
       # Setup an infinite loop timer to do register and trace report.
       init_worker_by_lua_block {
           local metadata_buffer = ngx.shared.tracing_buffer
   
           metadata_buffer:set('serviceName', 'User_Service_Name')
           -- Instance means the number of Nginx deloyment, does not mean the worker instances
           metadata_buffer:set('serviceInstanceName', 'User_Service_Instance_Name')
   
           require("client"):startBackendTimer("http://upstream:12800")
       }
       log_format sw_trace escape=json "$uri $request_body";
   
       server {
           listen 8080;
   
           location /e2e/users {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/users;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
   
           location /health-check {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/health-check;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
       }
   }
   ```
   
   BTW, according to log, `upstream:9090/e2e/users` connected successfully.

----------------------------------------------------------------
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] JaredTan95 commented on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-591409979
 
 
   Yes, I rechecked and following by https://github.com/apache/skywalking/blob/master/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/pom.xml#L47, I used the `protobuf-java-util` latest version here, cause of there have some bug when dealing with enum type within  `3.5.1` version.

----------------------------------------------------------------
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 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d714955`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   24.98%           
   =========================================
     Files             ?     1218           
     Lines             ?    28250           
     Branches          ?     3874           
   =========================================
     Hits              ?     7059           
     Misses            ?    20542           
     Partials          ?      649
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [d714955...4c7b71b](https://codecov.io/gh/apache/skywalking/pull/4399?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] JaredTan95 commented on a change in pull request #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386026435
 
 

 ##########
 File path: tools/dependencies/known-oap-backend-dependencies-es7.txt
 ##########
 @@ -143,6 +143,7 @@ parent-join-client-7.0.0.jar
 perfmark-api-0.19.0.jar
 proto-google-common-protos-1.12.0.jar
 protobuf-java-3.4.0.jar
+protobuf-java-3.11.4.jar
 
 Review comment:
   The old version is still in use. The new `protobuf-java-3.11.4.jar` just used for `protobuf-java-util -3.11.4.jar`, and I used `protobuf-java-util -3.11.4.jar` to dealing with enum type as I mentioned above.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386897600
 
 

 ##########
 File path: .github/workflows/e2e.yaml
 ##########
 @@ -116,6 +116,8 @@ jobs:
           ./mvnw --batch-mode -f test/e2e/pom.xml -pl e2e-base clean install
       - name: 6.x Agents & 7.x Backend
         run: export E2E_VERSION=jdk8-1.5 && bash -x test/e2e/run.sh e2e-6.x-agent-7.x-oap-compatibility
+      - name: Http API with Ningx Lua Tests In Single Node Mode(JDK8)
 
 Review comment:
   ```suggestion
         - name: Http API with Nginx Lua Tests In Single Node Mode(JDK8)
   ```

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r383622020
 
 

 ##########
 File path: tools/dependencies/known-oap-backend-dependencies-es7.txt
 ##########
 @@ -143,6 +143,7 @@ parent-join-client-7.0.0.jar
 perfmark-api-0.19.0.jar
 proto-google-common-protos-1.12.0.jar
 protobuf-java-3.4.0.jar
+protobuf-java-util-3.9.2.jar
 
 Review comment:
   From this, you are not requiring the `maven exclusion`, could you recheck why?

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r384504996
 
 

 ##########
 File path: oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/ProtoBufJsonUtils.java
 ##########
 @@ -0,0 +1,48 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.library.util;
+
+import com.google.protobuf.Message;
+import com.google.protobuf.util.JsonFormat;
+import java.io.IOException;
+
+public class ProtoBufJsonUtils {
+
+    public static String toJSON(Message sourceMessage) throws IOException {
+        return JsonFormat.printer().print(sourceMessage);
+    }
+
+    /**
+     * Extract data from a JSON String and use them to construct a Protocol Buffers Message.
+     *
+     * @param json          A JSON data string to parse
+     * @param targetBuilder A Message builder to use to construct the resulting Message
+     * @return the constructed Message
+     * @throws com.google.protobuf.InvalidProtocolBufferException Thrown in case of invalid Message data
+     */
+    public static Message fromJSON(String json, Message.Builder targetBuilder) throws IOException {
+        JsonFormat.parser()
+                  .usingTypeRegistry(JsonFormat.TypeRegistry.newBuilder()
+                                                            .add(targetBuilder.getDescriptorForType())
+                                                            .build())
+                  .ignoringUnknownFields()
+                  .merge(json, targetBuilder);
+        return targetBuilder.build();
 
 Review comment:
   Seems the returned value is never used, thus `.build()` is unnecessary

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r382959940
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstanceRegisterServletHandler.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInventoryCache;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.HOST_NAME;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.PROCESS_NO;
+
+public class ServiceInstanceRegisterServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstanceRegisterServletHandler.class);
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInventoryCache serviceInventoryCache;
+    private final Gson gson = new Gson();
+
+    private static final String SERVICE_ID = "service_id";
 
 Review comment:
   Why use `service_id` rather than `serviceId`? I thought we keep the same name as gRPC?

----------------------------------------------------------------
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] JaredTan95 commented on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593739280
 
 
   By following @kezhenxu94 , oap and java app are mounted into e2e container. But, nginx could,t connect to upstream only when Lua agent registering as following configuration:
   ![image](https://user-images.githubusercontent.com/12468337/75738092-52651a80-5d3c-11ea-90bb-94ed3f0af097.png)
   
   BTW, according to log, `upstream:9090/e2e/users` connected successfully.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590305927
 
 
   > Yes, I added protobuf-java-util.
   
   I saw you add `exclusion` to guava in the root pom, are you adding it or changing the version? I am a little confused about the change. But anyway, `known-oap-backend-dependencies.txt` and `known-oap-backend-dependencies-es7.txt` files are required to update to follow your change.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r382968362
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-trace-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/trace/provider/handler/v6/rest/TraceSegmentCollectServletHandler.java
 ##########
 @@ -0,0 +1,82 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.trace.provider.handler.v6.rest;
+
+import com.google.gson.JsonElement;
+import com.google.gson.stream.JsonReader;
+import java.io.BufferedReader;
+import java.io.IOException;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.apache.skywalking.oap.server.receiver.trace.provider.handler.v6.rest.reader.TraceSegment;
+import org.apache.skywalking.oap.server.receiver.trace.provider.handler.v6.rest.reader.UpstreamSegmentJsonReader;
+import org.apache.skywalking.oap.server.receiver.trace.provider.parser.SegmentParseV2;
+import org.apache.skywalking.oap.server.receiver.trace.provider.parser.SegmentSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TraceSegmentCollectServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(TraceSegmentCollectServletHandler.class);
+
+    private final SegmentParseV2.Producer segmentProducer;
+
+    public TraceSegmentCollectServletHandler(SegmentParseV2.Producer segmentProducer) {
+        this.segmentProducer = segmentProducer;
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/segments";
+    }
+
+    @Override
+    protected JsonElement doGet(HttpServletRequest req) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected JsonElement doPost(HttpServletRequest req) {
+        if (logger.isDebugEnabled()) {
+            logger.debug("receive stream segment");
+        }
+
+        try {
+            BufferedReader bufferedReader = req.getReader();
+            read(bufferedReader);
+        } catch (IOException e) {
+            logger.error(e.getMessage(), e);
+        }
+
+        return null;
+    }
+
+    private UpstreamSegmentJsonReader jsonReader = new UpstreamSegmentJsonReader();
+
+    private void read(BufferedReader bufferedReader) throws IOException {
+        JsonReader reader = new JsonReader(bufferedReader);
+
+        reader.beginArray();
 
 Review comment:
   Segment should be JSON format directly. There is no way to parse this in proto format. The reason of using HTTP, is there is no protobuf and grpc libs there. Such as Nginx lua. We can't built 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593739280
 
 
   By following @kezhenxu94 ,  oap and java app are mounted into e2e container. But, nginx could,t connect to upstream only when Lua agent registering as the following configuration:
   
   ```conf
   http {
       lua_package_path "/usr/share/skywalking-nginx-lua/lib/skywalking/?.lua;;";
       # Buffer represents the register inform and the queue of the finished segment
       lua_shared_dict tracing_buffer 100m;
   
       # Init is the timer setter and keeper
       # Setup an infinite loop timer to do register and trace report.
       init_worker_by_lua_block {
           local metadata_buffer = ngx.shared.tracing_buffer
   
           metadata_buffer:set('serviceName', 'User_Service_Name')
           -- Instance means the number of Nginx deloyment, does not mean the worker instances
           metadata_buffer:set('serviceInstanceName', 'User_Service_Instance_Name')
   
           require("client"):startBackendTimer("http://upstream:12800")
       }
       log_format sw_trace escape=json "$uri $request_body";
   
       server {
           listen 8080;
   
           location /e2e/users {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/users;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
   
           location /health-check {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/health-check;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
       }
   }
   ```
   
   BTW, according to log, `upstream:9090/e2e/users` connected successfully.

----------------------------------------------------------------
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] aderm commented on a change in pull request #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r383088805
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceRegisterServletHandler.java
 ##########
 @@ -0,0 +1,80 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServiceRegisterServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceRegisterServletHandler.class);
+
+    private final IServiceInventoryRegister serviceInventoryRegister;
+    private final Gson gson = new Gson();
+    private static final String SERVICE_NAME = "service_name";
+    private static final String SERVICE_ID = "service_id";
+
+    public ServiceRegisterServletHandler(ModuleManager moduleManager) {
+        serviceInventoryRegister = moduleManager.find(CoreModule.NAME)
+                                                .provider()
+                                                .getService(IServiceInventoryRegister.class);
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/service/register";
+    }
+
+    @Override
+    protected JsonElement doGet(HttpServletRequest req) throws ArgumentsParseException {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected JsonElement doPost(HttpServletRequest req) throws ArgumentsParseException {
+        JsonArray responseArray = new JsonArray();
+        try {
+            JsonArray serviceCodes = gson.fromJson(req.getReader(), JsonArray.class);
+            for (int i = 0; i < serviceCodes.size(); i++) {
+                JsonObject service = serviceCodes.get(i).getAsJsonObject();
+                String serviceCode = service.get(SERVICE_NAME).getAsString();
+                int serviceId = serviceInventoryRegister.getOrCreate(serviceCode, null);
+                JsonObject mapping = new JsonObject();
+                mapping.addProperty(SERVICE_NAME, serviceCode);
+                mapping.addProperty(SERVICE_ID, serviceId);
+                responseArray.add(mapping);
+                //
 
 Review comment:
   ```suggestion
   ```

----------------------------------------------------------------
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] JaredTan95 commented on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590312480
 
 
   `com.google.protobuf.util.JsonFormat` couldn't be imported from  `guava`. And there may be duplicated dependency if do not `exclusion` 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.
 
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593023594
 
 
   Please fix cI and e2e

----------------------------------------------------------------
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 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d714955`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   24.96%           
   =========================================
     Files             ?     1218           
     Lines             ?    28250           
     Branches          ?     3874           
   =========================================
     Hits              ?     7052           
     Misses            ?    20549           
     Partials          ?      649
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [d714955...c1aae8b](https://codecov.io/gh/apache/skywalking/pull/4399?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] JaredTan95 commented on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-595745933
 
 
   Make sense to me ,  take 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.
 
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386182556
 
 

 ##########
 File path: .github/workflows/e2e.yaml
 ##########
 @@ -44,6 +44,8 @@ jobs:
           ./mvnw --batch-mode -f test/e2e/pom.xml -pl e2e-base clean install
       - name: Single Node Tests(JDK8)
         run: export E2E_VERSION=jdk8-1.5 && bash -x test/e2e/run.sh e2e-single-service
+      - name: Single Http API with Ningx Lua Tests(JDK8)
 
 Review comment:
   And `Single` is not clear in the expression. Please rename this to a more clear name.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590139043
 
 
   @JaredTan95 As described in the #4401, we need an e2e test for HTTP by using LUA sdk here. The expected scenario is
   `Java app with javaagent` -> `OpenResty with lua agent`(tier1 LB) -> `OpenResty with lua agent`(tier2 LB) -> Java app with javaagent`.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386067019
 
 

 ##########
 File path: .gitignore
 ##########
 @@ -20,3 +20,4 @@ OALLexer.tokens
 .externalToolBuilders
 /test/plugin/dist
 /test/plugin/workspace
+e2e-*
 
 Review comment:
   What is this for?

----------------------------------------------------------------
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] JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593739280
 
 
   By following @kezhenxu94 ,  oap and java app are mounted into e2e container. But, nginx could not connect to upstream only when Lua agent registering as the following configuration, and according to log, `upstream:9090/e2e/users` connected successfully.:
   
   ```conf
   http {
       lua_package_path "/usr/share/skywalking-nginx-lua/lib/skywalking/?.lua;;";
       # Buffer represents the register inform and the queue of the finished segment
       lua_shared_dict tracing_buffer 100m;
   
       # Init is the timer setter and keeper
       # Setup an infinite loop timer to do register and trace report.
       init_worker_by_lua_block {
           local metadata_buffer = ngx.shared.tracing_buffer
   
           metadata_buffer:set('serviceName', 'User_Service_Name')
           -- Instance means the number of Nginx deloyment, does not mean the worker instances
           metadata_buffer:set('serviceInstanceName', 'User_Service_Instance_Name')
   
           require("client"):startBackendTimer("http://upstream:12800")
       }
       log_format sw_trace escape=json "$uri $request_body";
   
       server {
           listen 8080;
   
           location /e2e/users {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/users;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
   
           location /health-check {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/health-check;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
       }
   }
   ```

----------------------------------------------------------------
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 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d714955`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   24.98%           
   =========================================
     Files             ?     1218           
     Lines             ?    28250           
     Branches          ?     3874           
   =========================================
     Hits              ?     7059           
     Misses            ?    20542           
     Partials          ?      649
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [d714955...7e04508](https://codecov.io/gh/apache/skywalking/pull/4399?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] aderm commented on a change in pull request #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r383089732
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-trace-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/trace/provider/handler/v6/rest/reader/UpstreamSegmentJsonReader.java
 ##########
 @@ -0,0 +1,63 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.trace.provider.handler.v6.rest.reader;
+
+import com.google.gson.stream.JsonReader;
+import java.io.IOException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class UpstreamSegmentJsonReader implements StreamJsonReader<TraceSegment> {
+
+    private static final Logger logger = LoggerFactory.getLogger(UpstreamSegmentJsonReader.class);
+
+    private UniqueIdJsonReader uniqueIdJsonReader = new UniqueIdJsonReader();
+    private SegmentJsonReader segmentJsonReader = new SegmentJsonReader();
+
+    private static final String GLOBAL_TRACE_IDS = "global_trace_ids";
+    private static final String SEGMENT = "segment";
+
+    @Override
+    public TraceSegment read(JsonReader reader) throws IOException {
+        TraceSegment traceSegment = new TraceSegment();
+
+        reader.beginObject();
+        while (reader.hasNext()) {
+            switch (reader.nextName()) {
+                case GLOBAL_TRACE_IDS:
+                    reader.beginArray();
+                    while (reader.hasNext()) {
+                        traceSegment.addGlobalTraceId(uniqueIdJsonReader.read(reader));
+                    }
+                    reader.endArray();
+
 
 Review comment:
   ```suggestion
   ```

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r384501245
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-trace-receiver-plugin/src/test/java/org/apache/skywalking/oap/server/receiver/trace/provider/parser/listener/segment/ProtoBufJsonUtilsTest.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.trace.provider.parser.listener.segment;
+
+import java.io.IOException;
+import org.apache.skywalking.apm.network.language.agent.UpstreamSegment;
+import org.apache.skywalking.apm.network.language.agent.v2.SegmentObject;
+import org.apache.skywalking.oap.server.library.util.ProtoBufJsonUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ProtoBufJsonUtilsTest {
 
 Review comment:
   Why not put this test into the `library-util` module? If I remember correctly, coverage of unit tests locate in another module won't be counted

----------------------------------------------------------------
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] JaredTan95 commented on a change in pull request #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r383100950
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstanceRegisterServletHandler.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInventoryCache;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.HOST_NAME;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.PROCESS_NO;
+
+public class ServiceInstanceRegisterServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstanceRegisterServletHandler.class);
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInventoryCache serviceInventoryCache;
+    private final Gson gson = new Gson();
+
+    private static final String SERVICE_ID = "service_id";
+    private static final String AGENT_UUID = "agent_uuid";
+    private static final String REGISTER_TIME = "register_time";
+    private static final String INSTANCE_ID = "instance_id";
+    private static final String OS_INFO = "os_info";
+
+    public ServiceInstanceRegisterServletHandler(ModuleManager moduleManager) {
+        this.serviceInventoryCache = moduleManager.find(CoreModule.NAME)
+                                                  .provider()
+                                                  .getService(ServiceInventoryCache.class);
+        this.serviceInstanceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInstanceInventoryRegister.class);
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/instance/register";
 
 Review comment:
   Sure.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386024880
 
 

 ##########
 File path: docs/en/protocols/Trace-Data-Protocol-v2.md
 ##########
 @@ -7,6 +7,9 @@ Trace data protocol is defined and provided in [gRPC format](https://github.com/
 For each agent/SDK, it needs to register service id and service instance id before reporting any kind of trace 
 or metrics data.
 
+Since SkyWalking v7.x, SkyWalking provided register and uplink trace data through HTTP API way.
+[HTTP API Protocol](HTTP-API-Protocol.md) define the API data format.
 
 Review comment:
   ```suggestion
   [HTTP API Protocol](HTTP-API-Protocol.md) defines the API data format.
   ```

----------------------------------------------------------------
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] JaredTan95 commented on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-592847085
 
 
   > Hi @JaredTan95 , I ran into `NoClassDefFoundError`.
   > 
   > ```
   > 2020-02-27 21:50:56,407 - org.eclipse.jetty.server.HttpChannel - 507 [qtp1831717330-79] WARN  [] - /v2/service/register
   > java.lang.NoClassDefFoundError: com/google/protobuf/TypeRegistry
   >         at com.google.protobuf.util.JsonFormat.parser(JsonFormat.java:398) ~[protobuf-java-util-3.11.4.jar:?]
   >         at org.apache.skywalking.oap.server.library.util.ProtoBufJsonUtils.fromJSON(ProtoBufJsonUtils.java:40) ~[library-util-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
   >         at org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest.ServiceRegisterServletHandler.doPost(ServiceRegisterServletHandler.java:70) ~[skywalking-register-receiver-plugin-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
   >         at org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler.doPost(JettyJsonHandler.java:59) ~[library-server-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
   > ```
   
   Thanks to your test, fixed.

----------------------------------------------------------------
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 edited a comment on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@ce46725`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff            @@
   ##             master   #4399   +/-   ##
   ========================================
     Coverage          ?   25.4%           
   ========================================
     Files             ?    1216           
     Lines             ?   28079           
     Branches          ?    3861           
   ========================================
     Hits              ?    7134           
     Misses            ?   20294           
     Partials          ?     651
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   | [.../server/library/server/jetty/JettyJsonHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvc2VydmVyL2pldHR5L0pldHR5SnNvbkhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...handler/v6/rest/ServiceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VSZWdpc3RlclNlcnZsZXRIYW5kbGVyLmphdmE=) | `0% <0%> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/ServiceInstancePingServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVBpbmdTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [ce46725...b922fe3](https://codecov.io/gh/apache/skywalking/pull/4399?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 issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590037930
 
 
   @JaredTan95 The Nginx LUA first version has been finished. Could you try that with a Java App as upstream and downstream?
   https://github.com/apache/skywalking-nginx-lua#setup-doc
   
   This is the segment I reported
   ```json
   {
     "spans": [
       {
         "operationName": "/ingress",
         "startTime": 1582444532567,
         "tags": {},
         "endTime": 1582444532583,
         "spanType": "Exit",
         "logs": {},
         "spanId": 1,
         "isError": false,
         "parentSpanId": 0,
         "componentId": 6000,
         "peer": "User Service Name-nginx:upstream_ip:port",
         "spanLayer": "HTTP"
       },
       {
         "operationName": "/ingress",
         "startTime": 1582444532567,
         "tags": [
           {
             "key": "http.method",
             "value": "GET"
           },
           {
             "key": "http.params",
             "value": "http://127.0.0.1/ingress"
           }
         ],
         "endTime": 1582444532583,
         "spanType": "Entry",
         "spanId": 0,
         "logs": {},
         "parentSpanId": -1,
         "isError": false,
         "spanLayer": "HTTP",
         "componentId": 6000
       }
     ],
     "serviceInstanceId": 1,
     "serviceId": 1,
     "traceSegmentId": "1582444532104.794206293.69887",
     "globalTraceIds": [
       "1582444532104.794206293.69887"
     ]
   }
   ```
   
   And this is the header I injected
   ```
   1-MTU4MjQ0NDUzMjEwNC43OTQyMDYyOTMuNjk4ODc=-MTU4MjQ0NDUzMjEwNC43OTQyMDYyOTMuNjk4ODc=-1-1-1-IyNVc2VyIFNlcnZpY2UgTmFtZS1uZ2lueDp1cHN0cmVhbV9pcDpwb3J0-Iy9pbmdyZXNz-Iy9pbmdyZXNz
   ```

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386019209
 
 

 ##########
 File path: docs/en/protocols/HTTP-API-Protocol.md
 ##########
 @@ -0,0 +1,198 @@
+# HTTP API Protocol
+
+HTTP API Protocol define the API data format, including api request and response data format.
+
+### Do register
+
+For more information about data format can be found in [Register service](https://github.com/apache/skywalking-data-collect-protocol/tree/master/register/Register.proto).
+And register steps followings [SkyWalking Trace Data Protocol v2](Trace-Data-Protocol-v2.md).
+
+- Service Register
+
+> POST http://localhost:12800/v2/service/register
+
+Input:
+
+```json
+{
+  "services": [
+    {
+      "type": "normal",
+      "serviceName": "Service Name"
+    }
+  ]
+}
+```
+
+Output JSON Array:
+
+```json
+[
+    {
+        "key": "Service Name",
+        "value": 2
+    }
+]
+```
+
+- Service instance Register
+
+> POST http://localhost:12800/v2/instance/register
+
+Input:
+
+```json
+{
+  "instances": [
+    {
+      "time": 1582428603392,
+      "instanceUUID": "NAME:Service Instance Name",
+      "properties": [
+        {
+          "key": "language",
+          "value": "Lua"
+        }
+      ],
+      "serviceId": 2
+    }
+  ]
+}
+```
+
+OutPut:
+
+```json
+{
+    "key": "NAME:Service Instance Name",
+    "value": 2
+}
 
 Review comment:
   https://github.com/apache/skywalking-nginx-lua/blob/e284313e9186ad84d9c68c0042aa55139469fa4b/lib/skywalking/client.lua#L146-L153
   
   This is the code that `skywalking-nginx-lua` handles the response.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386067203
 
 

 ##########
 File path: test/e2e/e2e-http-api-with-nginx-lua/pom.xml
 ##########
 @@ -0,0 +1,213 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ 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.
+  ~
+  -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <parent>
+        <artifactId>apache-skywalking-e2e</artifactId>
+        <groupId>org.apache.skywalking</groupId>
+        <version>1.0.0</version>
+    </parent>
+    <modelVersion>4.0.0</modelVersion>
+
+    <artifactId>e2e-http-api-with-nginx-lua</artifactId>
+
+    <properties>
+        <e2e.container.version>1.1</e2e.container.version>
+        <e2e.container.name.prefix>skywalking-e2e-container-${build.id}-http-api-with-nginx-lua</e2e.container.name.prefix>
+        <e2e.nginx.version></e2e.nginx.version>
+    </properties>
+
+    <scm>
+        <connection>scm:git:https://github.com/apache/skywalking-nginx-lua.git</connection>
+        <developerConnection>scm:git:https://github.com/apache/skywalking-nginx-lua.git</developerConnection>
+        <url>https://github.com/apache/skywalking-nginx-lua.git</url>
+    </scm>
 
 Review comment:
   You could add some of these as properties for your startup 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.
 
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590022429
 
 
   - Service register in JSON
   req
   ```json
   {
     "services": [
       {
         "type": "normal",
         "serviceName": "Service Name"
       }
     ]
   }
   ```
   response
   ```json
   [
     {
       "key": "Service Name",
       "value": 1
     }
   ]
   ```
   
   - Service Instance register in JSON
   req
   ```json
   {
     "instances": [
       {
         "time": 1582428603.392,
         "instanceUUID": "name:Service Instance Name",
         "properties": [
           {
             "key": "language",
             "value": "Lua"
           }
         ],
         "serviceId": 1
       }
     ]
   }
   ```
   
   response
   ```json
   [
     {
       "key": "name:Service Instance Name",
       "value": 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d714955`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   24.98%           
   =========================================
     Files             ?     1218           
     Lines             ?    28260           
     Branches          ?     3876           
   =========================================
     Hits              ?     7061           
     Misses            ?    20550           
     Partials          ?      649
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [d714955...8e87fa2](https://codecov.io/gh/apache/skywalking/pull/4399?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] JaredTan95 commented on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593174690
 
 
   Not yet, final assert test haven’t finished.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-591435518
 
 
   I mean `known-oap-backend-dependencies.txt` change, do you notice you add a new dependency rather than changing a version number? That file includes the all dependency libs, so the `protobuf-java-util` doesn't exist before, you could check the previous version, such as the master branch.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386025789
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstanceRegisterServletHandler.java
 ##########
 @@ -0,0 +1,151 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.apm.network.common.KeyStringValuePair;
+import org.apache.skywalking.apm.network.register.v2.ServiceInstance;
+import org.apache.skywalking.apm.network.register.v2.ServiceInstances;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInventoryCache;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.apache.skywalking.oap.server.library.util.ProtoBufJsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.HOST_NAME;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.IPV4S;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.LANGUAGE;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.OS_NAME;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.PROCESS_NO;
+
+public class ServiceInstanceRegisterServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstanceRegisterServletHandler.class);
+    private static final String INSTANCE_CUSTOMIZED_NAME_PREFIX = "NAME:";
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInventoryCache serviceInventoryCache;
+
+    private static final String KEY = "key";
+    private static final String VALUE = "value";
+
+    public ServiceInstanceRegisterServletHandler(ModuleManager moduleManager) {
+        this.serviceInventoryCache = moduleManager.find(CoreModule.NAME)
+                                                  .provider()
+                                                  .getService(ServiceInventoryCache.class);
+        this.serviceInstanceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInstanceInventoryRegister.class);
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/instance/register";
+    }
+
+    @Override
+    protected JsonElement doGet(HttpServletRequest req) throws ArgumentsParseException {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected JsonElement doPost(HttpServletRequest req) throws ArgumentsParseException {
+
+        JsonObject responseJson = new JsonObject();
+
+        try {
+            ServiceInstances.Builder builder = ServiceInstances.newBuilder();
+            ProtoBufJsonUtils.fromJSON(getJsonBody(req), builder);
+            List<ServiceInstance> serviceInstances = builder.build().getInstancesList();
+
+            serviceInstances.forEach(instance -> {
+                long time = instance.getTime();
+                int serviceId = instance.getServiceId();
+                String instanceUUID = instance.getInstanceUUID();
+
+                JsonObject instanceProperties = new JsonObject();
+                List<String> ipv4s = new ArrayList<>();
+
+                for (KeyStringValuePair property : instance.getPropertiesList()) {
+                    String key = property.getKey();
+                    switch (key) {
+                        case HOST_NAME:
+                            instanceProperties.addProperty(HOST_NAME, property.getValue());
+                            break;
+                        case OS_NAME:
+                            instanceProperties.addProperty(OS_NAME, property.getValue());
+                            break;
+                        case LANGUAGE:
+                            instanceProperties.addProperty(LANGUAGE, property.getValue());
+                            break;
+                        case "ipv4":
+                            ipv4s.add(property.getValue());
+                            break;
+                        case PROCESS_NO:
+                            instanceProperties.addProperty(PROCESS_NO, property.getValue());
+                            break;
+                        default:
+                            instanceProperties.addProperty(key, property.getValue());
+                    }
+                }
+                instanceProperties.addProperty(IPV4S, ServiceInstanceInventory.PropertyUtil.ipv4sSerialize(ipv4s));
+
+                String instanceName = null;
+                if (instanceUUID.startsWith(INSTANCE_CUSTOMIZED_NAME_PREFIX)) {
+                    instanceName = instanceUUID.substring(INSTANCE_CUSTOMIZED_NAME_PREFIX.length());
+                }
+
+                ServiceInventory serviceInventory = serviceInventoryCache.get(serviceId);
+
+                if (instanceName == null) {
+                    /**
 
 Review comment:
   Dangling Javadoc comment 

----------------------------------------------------------------
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] dmsolr commented on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593202794
 
 
   > Not yet, final assert test haven’t finished.
   
   Haha, I have the same requirement in the Nginx-Lua agent e2e test. :)

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r382961897
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstanceRegisterServletHandler.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInventoryCache;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.HOST_NAME;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.PROCESS_NO;
+
+public class ServiceInstanceRegisterServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstanceRegisterServletHandler.class);
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInventoryCache serviceInventoryCache;
+    private final Gson gson = new Gson();
+
+    private static final String SERVICE_ID = "service_id";
+    private static final String AGENT_UUID = "agent_uuid";
+    private static final String REGISTER_TIME = "register_time";
+    private static final String INSTANCE_ID = "instance_id";
+    private static final String OS_INFO = "os_info";
+
+    public ServiceInstanceRegisterServletHandler(ModuleManager moduleManager) {
+        this.serviceInventoryCache = moduleManager.find(CoreModule.NAME)
+                                                  .provider()
+                                                  .getService(ServiceInventoryCache.class);
+        this.serviceInstanceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInstanceInventoryRegister.class);
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/instance/register";
 
 Review comment:
   I have begun to adopt your path definitions, please don't change them :)

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386024640
 
 

 ##########
 File path: docs/en/protocols/HTTP-API-Protocol.md
 ##########
 @@ -0,0 +1,198 @@
+# HTTP API Protocol
+
+HTTP API Protocol define the API data format, including api request and response data format.
+
+### Do register
+
+For more information about data format can be found in [Register service](https://github.com/apache/skywalking-data-collect-protocol/tree/master/register/Register.proto).
 
 Review comment:
   Seems to be a grammar error
   
   For more information about the data format, please refer to .....
   
   OR
   
   Detail information about data format can be found in ......

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r384506778
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-trace-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/trace/provider/handler/v6/rest/TraceSegmentCollectServletHandler.java
 ##########
 @@ -0,0 +1,76 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.trace.provider.handler.v6.rest;
+
+import com.google.gson.JsonElement;
+import java.io.BufferedReader;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.apm.network.language.agent.UpstreamSegment;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.apache.skywalking.oap.server.receiver.trace.provider.handler.v6.rest.reader.UpstreamSegmentJsonReader;
+import org.apache.skywalking.oap.server.receiver.trace.provider.parser.SegmentParseV2;
+import org.apache.skywalking.oap.server.receiver.trace.provider.parser.SegmentSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TraceSegmentCollectServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(TraceSegmentCollectServletHandler.class);
+
+    private final SegmentParseV2.Producer segmentProducer;
+
+    private UpstreamSegmentJsonReader upstreamSegmentJsonReader = new UpstreamSegmentJsonReader();
+
+    public TraceSegmentCollectServletHandler(SegmentParseV2.Producer segmentProducer) {
+        this.segmentProducer = segmentProducer;
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/segments";
+    }
+
+    @Override
+    protected JsonElement doGet(HttpServletRequest req) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected JsonElement doPost(HttpServletRequest req) {
+        if (logger.isDebugEnabled()) {
+            logger.debug("receive stream segment");
+        }
+
+        StringBuffer stringBuffer = new StringBuffer();
 
 Review comment:
   'StringBuffer stringBuffer' may be declared as 'StringBuilder' 

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r384572660
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-trace-receiver-plugin/src/test/java/org/apache/skywalking/oap/server/receiver/trace/provider/parser/listener/segment/ProtoBufJsonUtilsTest.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.trace.provider.parser.listener.segment;
+
+import java.io.IOException;
+import org.apache.skywalking.apm.network.language.agent.UpstreamSegment;
+import org.apache.skywalking.apm.network.language.agent.v2.SegmentObject;
+import org.apache.skywalking.oap.server.library.util.ProtoBufJsonUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ProtoBufJsonUtilsTest {
 
 Review comment:
   > If so, `apm-network` module should be added into `library-util` module.
   
   Hum, that's more unacceptable, just ignore it please

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r382961036
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstancePingServletHandler.java
 ##########
 @@ -0,0 +1,107 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.Objects;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.apm.network.common.Command;
+import org.apache.skywalking.apm.network.common.Commands;
+import org.apache.skywalking.apm.network.trace.component.command.ServiceResetCommand;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInstanceInventoryCache;
+import org.apache.skywalking.oap.server.core.command.CommandService;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServiceInstancePingServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstancePingServletHandler.class);
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInstanceInventoryCache serviceInstanceInventoryCache;
+    private final IServiceInventoryRegister serviceInventoryRegister;
+    private final CommandService commandService;
+    private final Gson gson = new Gson();
+
+    private static final String INSTANCE_ID = "instance_id";
+    private static final String TIME = "time";
+    private static final String SERVICE_INSTANCE_UUID = "service_instance_UUID";
+    private static final String COMMANDS = "commands";
+
+    public ServiceInstancePingServletHandler(ModuleManager moduleManager) {
+        this.serviceInstanceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInstanceInventoryRegister.class);
+        this.serviceInstanceInventoryCache = moduleManager.find(CoreModule.NAME).provider().getService(
+            ServiceInstanceInventoryCache.class);
+        this.serviceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInventoryRegister.class);
+        this.commandService = moduleManager.find(CoreModule.NAME).provider().getService(CommandService.class);
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/instance/heartbeat";
+    }
+
+    @Override
+    protected JsonElement doGet(HttpServletRequest req) throws ArgumentsParseException {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected JsonElement doPost(HttpServletRequest req) throws ArgumentsParseException, IOException {
+        JsonObject responseJson = new JsonObject();
+        try {
+            JsonObject heartBeat = gson.fromJson(req.getReader(), JsonObject.class);
+            int instanceId = heartBeat.get(INSTANCE_ID).getAsInt();
+            long heartBeatTime = heartBeat.get(TIME).getAsLong();
+            String serviceInstanceUUID = heartBeat.get(SERVICE_INSTANCE_UUID).getAsString();
+
+            serviceInstanceInventoryRegister.heartbeat(instanceId, heartBeatTime);
+            ServiceInstanceInventory serviceInstanceInventory = serviceInstanceInventoryCache.get(instanceId);
+            if (Objects.nonNull(serviceInstanceInventory)) {
+                serviceInventoryRegister.heartbeat(serviceInstanceInventory.getServiceId(), heartBeatTime);
+            } else {
+                logger.warn(
+                    "Can't found service by service instance id from cache, service instance id is: {}", instanceId);
+
+                final ServiceResetCommand resetCommand = commandService.newResetCommand(
+                    instanceId, heartBeatTime, serviceInstanceUUID);
+                final Command command = resetCommand.serialize().build();
+                final Commands nextCommands = Commands.newBuilder().addCommands(command).build();
+                responseJson.add(COMMANDS, gson.toJsonTree(nextCommands, Commands.class));
+            }
+            responseJson.addProperty(INSTANCE_ID, instanceId);
+            responseJson.addProperty(TIME, heartBeatTime);
+        } catch (IOException e) {
+            logger.error(e.getMessage(), e);
 
 Review comment:
   add some error message to response?

----------------------------------------------------------------
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] JaredTan95 commented on a change in pull request #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r384517226
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-trace-receiver-plugin/src/test/java/org/apache/skywalking/oap/server/receiver/trace/provider/parser/listener/segment/ProtoBufJsonUtilsTest.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.trace.provider.parser.listener.segment;
+
+import java.io.IOException;
+import org.apache.skywalking.apm.network.language.agent.UpstreamSegment;
+import org.apache.skywalking.apm.network.language.agent.v2.SegmentObject;
+import org.apache.skywalking.oap.server.library.util.ProtoBufJsonUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ProtoBufJsonUtilsTest {
 
 Review comment:
   If so, `apm-network` module should be added into `library-util` module. 

----------------------------------------------------------------
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 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > Merging [#4399](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d32450aa7d29a2ccb1b0d48ee251d2d1ef8851af?src=pr&el=desc) will **decrease** coverage by `0.11%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4399      +/-   ##
   ==========================================
   - Coverage    25.1%   24.99%   -0.12%     
   ==========================================
     Files        1211     1230      +19     
     Lines       28096    28465     +369     
     Branches     3868     3900      +32     
   ==========================================
   + Hits         7054     7115      +61     
   - Misses      20393    20693     +300     
   - Partials      649      657       +8
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   | [...dler/v6/rest/reader/UpstreamSegmentJsonReader.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L3JlYWRlci9VcHN0cmVhbVNlZ21lbnRKc29uUmVhZGVyLmphdmE=) | `0% <0%> (ø)` | |
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/ServiceInstancePingServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVBpbmdTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   | [.../server/library/server/jetty/JettyJsonHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvc2VydmVyL2pldHR5L0pldHR5SnNvbkhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...handler/v6/rest/ServiceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VSZWdpc3RlclNlcnZsZXRIYW5kbGVyLmphdmE=) | `0% <0%> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ider/handler/v6/rest/reader/SegmentJsonReader.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L3JlYWRlci9TZWdtZW50SnNvblJlYWRlci5qYXZh) | `0% <0%> (ø)` | |
   | [...r/receiver/trace/provider/TraceModuleProvider.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvVHJhY2VNb2R1bGVQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...iver/register/provider/RegisterModuleProvider.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvUmVnaXN0ZXJNb2R1bGVQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | ... and [20 more](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [d32450a...93b95ee](https://codecov.io/gh/apache/skywalking/pull/4399?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 issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593073397
 
 
   Seems you break the whole CI settings :)

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386025609
 
 

 ##########
 File path: tools/dependencies/known-oap-backend-dependencies-es7.txt
 ##########
 @@ -143,6 +143,7 @@ parent-join-client-7.0.0.jar
 perfmark-api-0.19.0.jar
 proto-google-common-protos-1.12.0.jar
 protobuf-java-3.4.0.jar
+protobuf-java-3.11.4.jar
 
 Review comment:
   Two versions of protobuf-java?  Please check whether the older version is still in use or not, if not, remove it here and update the file below, otherwise consider making the versions consistent
   
   https://github.com/apache/skywalking/blob/29da5738bcea2a55e5d88f4eda9ea9056be38867/dist-material/release-docs/LICENSE#L367-L368

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386181736
 
 

 ##########
 File path: .github/workflows/e2e.yaml
 ##########
 @@ -44,6 +44,8 @@ jobs:
           ./mvnw --batch-mode -f test/e2e/pom.xml -pl e2e-base clean install
       - name: Single Node Tests(JDK8)
         run: export E2E_VERSION=jdk8-1.5 && bash -x test/e2e/run.sh e2e-single-service
+      - name: Single Http API with Ningx Lua Tests(JDK8)
+        run: export E2E_VERSION=jdk8-1.5 && bash -x test/e2e/run.sh e2e-http-api-with-nginx-lua
 
 Review comment:
   Move this to `Compatibilities` section. This is HTTP + LUA compatible test. And we could run test faster there.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590064256
 
 
   By following @JaredTan95 feedback, I just re-format the trace_id and segment_id to adopt the proto
   
   ```json
   {
     "spans": [
       {
         "operationName": "/tier2/lb",
         "startTime": 1582461179910,
         "tags": {},
         "endTime": 1582461179922,
         "spanType": "Exit",
         "logs": {},
         "spanId": 1,
         "isError": false,
         "parentSpanId": 0,
         "componentId": 6000,
         "peer": "User Service Name-nginx:upstream_ip:port",
         "spanLayer": "HTTP"
       },
       {
         "operationName": "/tier2/lb",
         "startTime": 1582461179910,
         "tags": [
           {
             "key": "http.method",
             "value": "GET"
           },
           {
             "key": "http.params",
             "value": "http://127.0.0.1/tier2/lb"
           }
         ],
         "endTime": 1582461179922,
         "spanType": "Entry",
         "logs": {},
         "spanId": 0,
         "isError": false,
         "parentSpanId": -1,
         "componentId": 6000,
         "refs": [
           {
             "parentTraceSegmentId": {
               "idParts": [
                 1582461179038,
                 794206293,
                 69887
               ]
             },
             "parentEndpointId": 0,
             "entryEndpointId": 0,
             "parentServiceInstanceId": 1,
             "parentEndpoint": "/ingress",
             "networkAddress": "#User Service Name-nginx:upstream_ip:port",
             "parentSpanId": 1,
             "entryServiceInstanceId": 1,
             "networkAddressId": 0,
             "entryEndpoint": "/ingress"
           }
         ],
         "spanLayer": "HTTP"
       }
     ],
     "serviceInstanceId": 1,
     "serviceId": 1,
     "traceSegmentId": {
       "idParts": [
         1582461179044,
         794206293,
         69887
       ]
     },
     "globalTraceIds": [
       {
         "idParts": [
           1582461179038,
           794206293,
           69887
         ]
       }
     ]
   }
   ```

----------------------------------------------------------------
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] JaredTan95 commented on a change in pull request #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r382961576
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstanceRegisterServletHandler.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInventoryCache;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.HOST_NAME;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.PROCESS_NO;
+
+public class ServiceInstanceRegisterServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstanceRegisterServletHandler.class);
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInventoryCache serviceInventoryCache;
+    private final Gson gson = new Gson();
+
+    private static final String SERVICE_ID = "service_id";
 
 Review comment:
   Okay

----------------------------------------------------------------
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] JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593739280
 
 
   By following @kezhenxu94 ,  oap and java app are mounted into e2e container. But, nginx could not connect to upstream only when Lua agent registering as the following configuration:
   
   ```conf
   http {
       lua_package_path "/usr/share/skywalking-nginx-lua/lib/skywalking/?.lua;;";
       # Buffer represents the register inform and the queue of the finished segment
       lua_shared_dict tracing_buffer 100m;
   
       # Init is the timer setter and keeper
       # Setup an infinite loop timer to do register and trace report.
       init_worker_by_lua_block {
           local metadata_buffer = ngx.shared.tracing_buffer
   
           metadata_buffer:set('serviceName', 'User_Service_Name')
           -- Instance means the number of Nginx deloyment, does not mean the worker instances
           metadata_buffer:set('serviceInstanceName', 'User_Service_Instance_Name')
   
           require("client"):startBackendTimer("http://upstream:12800")
       }
       log_format sw_trace escape=json "$uri $request_body";
   
       server {
           listen 8080;
   
           location /e2e/users {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/users;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
   
           location /health-check {
   
               rewrite_by_lua_block {
                   require("tracer"):startBackendTimer()
               }
   
               proxy_pass http://upstream:9090/e2e/health-check;
   
               body_filter_by_lua_block {
                   require("tracer"):finish()
               }
   
               log_by_lua_block {
                   require("tracer"):prepareForReport()
               }
           }
       }
   }
   ```
   
   BTW, according to log, `upstream:9090/e2e/users` connected successfully.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > Merging [#4399](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/47bb0c0247290e42923496b2b164eb75ec7cdfe6?src=pr&el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #4399   +/-   ##
   =======================================
     Coverage   25.27%   25.27%           
   =======================================
     Files        1221     1221           
     Lines       28221    28221           
     Branches     3877     3877           
   =======================================
     Hits         7133     7133           
     Misses      20436    20436           
     Partials      652      652
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [47bb0c0...ef86e72](https://codecov.io/gh/apache/skywalking/pull/4399?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 issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-595281040
 
 
   Look like startup sequence is the key block here. @JaredTan95 Please remove the H2 in your case, then the startup seq is certain, as java app starts up before Nginx.

----------------------------------------------------------------
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 issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
arugal commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-591985344
 
 
   Hi @JaredTan95 , I ran into `NoClassDefFoundError`.
   ```
   2020-02-27 21:50:56,407 - org.eclipse.jetty.server.HttpChannel - 507 [qtp1831717330-79] WARN  [] - /v2/service/register
   java.lang.NoClassDefFoundError: com/google/protobuf/TypeRegistry
           at com.google.protobuf.util.JsonFormat.parser(JsonFormat.java:398) ~[protobuf-java-util-3.11.4.jar:?]
           at org.apache.skywalking.oap.server.library.util.ProtoBufJsonUtils.fromJSON(ProtoBufJsonUtils.java:40) ~[library-util-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
           at org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest.ServiceRegisterServletHandler.doPost(ServiceRegisterServletHandler.java:70) ~[skywalking-register-receiver-plugin-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
           at org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler.doPost(JettyJsonHandler.java:59) ~[library-server-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
   ```
   
   

----------------------------------------------------------------
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] JaredTan95 commented on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590305053
 
 
   > Are you adding new dependency? The dependency check fails.
   
   Yes, I added `protobuf-java-util`.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386067192
 
 

 ##########
 File path: test/e2e/e2e-http-api-with-nginx-lua/pom.xml
 ##########
 @@ -0,0 +1,213 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ 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.
+  ~
+  -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <parent>
+        <artifactId>apache-skywalking-e2e</artifactId>
+        <groupId>org.apache.skywalking</groupId>
+        <version>1.0.0</version>
+    </parent>
+    <modelVersion>4.0.0</modelVersion>
+
+    <artifactId>e2e-http-api-with-nginx-lua</artifactId>
+
+    <properties>
+        <e2e.container.version>1.1</e2e.container.version>
+        <e2e.container.name.prefix>skywalking-e2e-container-${build.id}-http-api-with-nginx-lua</e2e.container.name.prefix>
+        <e2e.nginx.version></e2e.nginx.version>
+    </properties>
+
+    <scm>
+        <connection>scm:git:https://github.com/apache/skywalking-nginx-lua.git</connection>
+        <developerConnection>scm:git:https://github.com/apache/skywalking-nginx-lua.git</developerConnection>
+        <url>https://github.com/apache/skywalking-nginx-lua.git</url>
+    </scm>
 
 Review comment:
   This scm section seems not right. SCM in pom means the source code of this module is here. 

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386018844
 
 

 ##########
 File path: docs/en/protocols/HTTP-API-Protocol.md
 ##########
 @@ -0,0 +1,198 @@
+# HTTP API Protocol
+
+HTTP API Protocol define the API data format, including api request and response data format.
+
+### Do register
+
+For more information about data format can be found in [Register service](https://github.com/apache/skywalking-data-collect-protocol/tree/master/register/Register.proto).
+And register steps followings [SkyWalking Trace Data Protocol v2](Trace-Data-Protocol-v2.md).
+
+- Service Register
+
+> POST http://localhost:12800/v2/service/register
+
+Input:
+
+```json
+{
+  "services": [
+    {
+      "type": "normal",
+      "serviceName": "Service Name"
+    }
+  ]
+}
+```
+
+Output JSON Array:
+
+```json
+[
+    {
+        "key": "Service Name",
+        "value": 2
+    }
+]
+```
+
+- Service instance Register
+
+> POST http://localhost:12800/v2/instance/register
+
+Input:
+
+```json
+{
+  "instances": [
+    {
+      "time": 1582428603392,
+      "instanceUUID": "NAME:Service Instance Name",
+      "properties": [
+        {
+          "key": "language",
+          "value": "Lua"
+        }
+      ],
+      "serviceId": 2
+    }
+  ]
+}
+```
+
+OutPut:
+
+```json
+{
+    "key": "NAME:Service Instance Name",
+    "value": 2
+}
 
 Review comment:
   It should be an array, 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] wu-sheng commented on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590192873
 
 
   About tests
   1. A new test e2e case in the `Compatibilities` group should be added, as HTTP API tests.
   1. Use `git submodule` to download Nginx LUA source codes in to `tests/e2e/subprojects/nginx/lua` and use the OpenResty image to load that.
   
   Please you face any issue about the Nginx/OpenResty and LUA agent, please let me know. @dmsolr also has set up locally, seems working as expected.

----------------------------------------------------------------
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 edited a comment on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d714955`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4399   +/-   ##
   =========================================
     Coverage          ?   24.98%           
   =========================================
     Files             ?     1218           
     Lines             ?    28250           
     Branches          ?     3874           
   =========================================
     Hits              ?     7059           
     Misses            ?    20542           
     Partials          ?      649
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ing/oap/server/library/util/ProtoBufJsonUtils.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvUHJvdG9CdWZKc29uVXRpbHMuamF2YQ==) | `0% <ø> (ø)` | |
   | [...v6/rest/ServiceInstanceRegisterServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctcmVnaXN0ZXItcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvcmVnaXN0ZXIvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1NlcnZpY2VJbnN0YW5jZVJlZ2lzdGVyU2VydmxldEhhbmRsZXIuamF2YQ==) | `0% <0%> (ø)` | |
   | [...ler/v6/rest/TraceSegmentCollectServletHandler.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcmVjZWl2ZXItcGx1Z2luL3NreXdhbGtpbmctdHJhY2UtcmVjZWl2ZXItcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvcmVjZWl2ZXIvdHJhY2UvcHJvdmlkZXIvaGFuZGxlci92Ni9yZXN0L1RyYWNlU2VnbWVudENvbGxlY3RTZXJ2bGV0SGFuZGxlci5qYXZh) | `0% <0%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [d714955...045a1eb](https://codecov.io/gh/apache/skywalking/pull/4399?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] kezhenxu94 commented on a change in pull request #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r382961136
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstancePingServletHandler.java
 ##########
 @@ -0,0 +1,107 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.Objects;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.apm.network.common.Command;
+import org.apache.skywalking.apm.network.common.Commands;
+import org.apache.skywalking.apm.network.trace.component.command.ServiceResetCommand;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInstanceInventoryCache;
+import org.apache.skywalking.oap.server.core.command.CommandService;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServiceInstancePingServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstancePingServletHandler.class);
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInstanceInventoryCache serviceInstanceInventoryCache;
+    private final IServiceInventoryRegister serviceInventoryRegister;
+    private final CommandService commandService;
+    private final Gson gson = new Gson();
+
+    private static final String INSTANCE_ID = "instance_id";
+    private static final String TIME = "time";
+    private static final String SERVICE_INSTANCE_UUID = "service_instance_UUID";
+    private static final String COMMANDS = "commands";
+
+    public ServiceInstancePingServletHandler(ModuleManager moduleManager) {
+        this.serviceInstanceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInstanceInventoryRegister.class);
+        this.serviceInstanceInventoryCache = moduleManager.find(CoreModule.NAME).provider().getService(
+            ServiceInstanceInventoryCache.class);
+        this.serviceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInventoryRegister.class);
+        this.commandService = moduleManager.find(CoreModule.NAME).provider().getService(CommandService.class);
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/instance/heartbeat";
+    }
+
+    @Override
+    protected JsonElement doGet(HttpServletRequest req) throws ArgumentsParseException {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected JsonElement doPost(HttpServletRequest req) throws ArgumentsParseException, IOException {
+        JsonObject responseJson = new JsonObject();
+        try {
+            JsonObject heartBeat = gson.fromJson(req.getReader(), JsonObject.class);
+            int instanceId = heartBeat.get(INSTANCE_ID).getAsInt();
+            long heartBeatTime = heartBeat.get(TIME).getAsLong();
+            String serviceInstanceUUID = heartBeat.get(SERVICE_INSTANCE_UUID).getAsString();
+
+            serviceInstanceInventoryRegister.heartbeat(instanceId, heartBeatTime);
+            ServiceInstanceInventory serviceInstanceInventory = serviceInstanceInventoryCache.get(instanceId);
+            if (Objects.nonNull(serviceInstanceInventory)) {
+                serviceInventoryRegister.heartbeat(serviceInstanceInventory.getServiceId(), heartBeatTime);
+            } else {
+                logger.warn(
+                    "Can't found service by service instance id from cache, service instance id is: {}", instanceId);
+
+                final ServiceResetCommand resetCommand = commandService.newResetCommand(
+                    instanceId, heartBeatTime, serviceInstanceUUID);
+                final Command command = resetCommand.serialize().build();
+                final Commands nextCommands = Commands.newBuilder().addCommands(command).build();
+                responseJson.add(COMMANDS, gson.toJsonTree(nextCommands, Commands.class));
 
 Review comment:
   use `org.apache.skywalking.oap.server.library.util.ProtoBufJsonUtils#toJSON`

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590231364
 
 
   Are you adding new dependency? The dependency check fails.

----------------------------------------------------------------
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 edited a comment on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590023100
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=h1) Report
   > Merging [#4399](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/47bb0c0247290e42923496b2b164eb75ec7cdfe6?src=pr&el=desc) will **increase** coverage by `<.01%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4399/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4399      +/-   ##
   ==========================================
   + Coverage   25.27%   25.27%   +<.01%     
   ==========================================
     Files        1221     1221              
     Lines       28221    28221              
     Branches     3877     3877              
   ==========================================
   + Hits         7133     7134       +1     
     Misses      20436    20436              
   + Partials      652      651       -1
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4399?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../apm/network/trace/component/ComponentsDefine.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-YXBtLXByb3RvY29sL2FwbS1uZXR3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9uZXR3b3JrL3RyYWNlL2NvbXBvbmVudC9Db21wb25lbnRzRGVmaW5lLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
   | [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4399/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `60% <0%> (+1.66%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4399?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/4399?src=pr&el=footer). Last update [47bb0c0...ef86e72](https://codecov.io/gh/apache/skywalking/pull/4399?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 merged pull request #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399
 
 
   

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386024503
 
 

 ##########
 File path: docs/en/protocols/HTTP-API-Protocol.md
 ##########
 @@ -0,0 +1,198 @@
+# HTTP API Protocol
+
+HTTP API Protocol define the API data format, including api request and response data format.
 
 Review comment:
   ```suggestion
   HTTP API Protocol defines the API data format, including api request and response data format.
   ```

----------------------------------------------------------------
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 edited a comment on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng edited a comment on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590064256
 
 
   By following @JaredTan95 feedback, I just re-format the trace_id and segment_id to adopt the proto
   
   ```json
   {
     "spans": [
       {
         "operationName": "/tier2/lb",
         "startTime": 1582526028207,
         "endTime": 1582526028221,
         "spanType": "Exit",
         "spanId": 1,
         "isError": false,
         "parentSpanId": 0,
         "componentId": 6000,
         "peer": "User Service Name-nginx:upstream_ip:port",
         "spanLayer": "HTTP"
       },
       {
         "operationName": "/tier2/lb",
         "startTime": 1582526028207,
         "tags": [
           {
             "key": "http.method",
             "value": "GET"
           },
           {
             "key": "http.params",
             "value": "http://127.0.0.1/tier2/lb"
           }
         ],
         "endTime": 1582526028221,
         "spanType": "Entry",
         "spanId": 0,
         "isError": false,
         "parentSpanId": -1,
         "componentId": 6000,
         "refs": [
           {
             "parentTraceSegmentId": {
               "idParts": [
                 1582526028032,
                 794206293,
                 69887
               ]
             },
             "parentEndpointId": 0,
             "entryEndpointId": 0,
             "parentServiceInstanceId": 1,
             "parentEndpoint": "/ingress",
             "networkAddress": "#User Service Name-nginx:upstream_ip:port",
             "parentSpanId": 1,
             "entryServiceInstanceId": 1,
             "networkAddressId": 0,
             "entryEndpoint": "/ingress"
           }
         ],
         "spanLayer": "HTTP"
       }
     ],
     "serviceInstanceId": 1,
     "serviceId": 1,
     "traceSegmentId": {
       "idParts": [
         1582526028040,
         794206293,
         69887
       ]
     },
     "globalTraceIds": [
       {
         "idParts": [
           1582526028032,
           794206293,
           69887
         ]
       }
     ]
   }
   ```

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386025839
 
 

 ##########
 File path: oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/ProtoBufJsonUtils.java
 ##########
 @@ -0,0 +1,47 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.library.util;
+
+import com.google.protobuf.Message;
+import com.google.protobuf.util.JsonFormat;
+import java.io.IOException;
+
+public class ProtoBufJsonUtils {
+
+    public static String toJSON(Message sourceMessage) throws IOException {
+        return JsonFormat.printer().print(sourceMessage);
+    }
+
+    /**
+     * Extract data from a JSON String and use them to construct a Protocol Buffers Message.
+     *
+     * @param json          A JSON data string to parse
+     * @param targetBuilder A Message builder to use to construct the resulting Message
+     * @return the constructed Message
 
 Review comment:
   no `return` 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] wu-sheng edited a comment on issue #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng edited a comment on issue #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-590192873
 
 
   About tests
   1. A new test e2e case in the `Compatibilities` group should be added, as HTTP API tests.
   1. Use `git submodule` to download Nginx LUA source codes in to `tests/e2e/subprojects/nginx-lua` and use the OpenResty image to load that.
   
   Please you face any issue about the Nginx/OpenResty and LUA agent, please let me know. @dmsolr also has set up locally, seems working as expected.

----------------------------------------------------------------
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 #4399: support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r382959959
 
 

 ##########
 File path: oap-server/server-receiver-plugin/skywalking-register-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/register/provider/handler/v6/rest/ServiceInstanceRegisterServletHandler.java
 ##########
 @@ -0,0 +1,121 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.receiver.register.provider.handler.v6.rest;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.cache.ServiceInventoryCache;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.register.service.IServiceInstanceInventoryRegister;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.server.jetty.ArgumentsParseException;
+import org.apache.skywalking.oap.server.library.server.jetty.JettyJsonHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.HOST_NAME;
+import static org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory.PropertyUtil.PROCESS_NO;
+
+public class ServiceInstanceRegisterServletHandler extends JettyJsonHandler {
+
+    private static final Logger logger = LoggerFactory.getLogger(ServiceInstanceRegisterServletHandler.class);
+
+    private final IServiceInstanceInventoryRegister serviceInstanceInventoryRegister;
+    private final ServiceInventoryCache serviceInventoryCache;
+    private final Gson gson = new Gson();
+
+    private static final String SERVICE_ID = "service_id";
+    private static final String AGENT_UUID = "agent_uuid";
+    private static final String REGISTER_TIME = "register_time";
+    private static final String INSTANCE_ID = "instance_id";
+    private static final String OS_INFO = "os_info";
+
+    public ServiceInstanceRegisterServletHandler(ModuleManager moduleManager) {
+        this.serviceInventoryCache = moduleManager.find(CoreModule.NAME)
+                                                  .provider()
+                                                  .getService(ServiceInventoryCache.class);
+        this.serviceInstanceInventoryRegister = moduleManager.find(CoreModule.NAME).provider().getService(
+            IServiceInstanceInventoryRegister.class);
+    }
+
+    @Override
+    public String pathSpec() {
+        return "/v2/instance/register";
+    }
+
+    @Override
+    protected JsonElement doGet(HttpServletRequest req) throws ArgumentsParseException {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected JsonElement doPost(HttpServletRequest req) throws ArgumentsParseException {
+        JsonObject responseJson = new JsonObject();
+        try {
+            JsonObject instance = gson.fromJson(req.getReader(), JsonObject.class);
+            int serviceId = instance.get(SERVICE_ID).getAsInt();
+            String agentUUID = instance.get(AGENT_UUID).getAsString();
+            long registerTime = instance.get(REGISTER_TIME).getAsLong();
+            JsonObject osInfoJson = instance.get(OS_INFO).getAsJsonObject();
 
 Review comment:
   `OS_INFO` could be null, we don't require for that, right? And where is the language?

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386024756
 
 

 ##########
 File path: docs/en/protocols/HTTP-API-Protocol.md
 ##########
 @@ -0,0 +1,198 @@
+# HTTP API Protocol
+
+HTTP API Protocol define the API data format, including api request and response data format.
+
+### Do register
+
+For more information about data format can be found in [Register service](https://github.com/apache/skywalking-data-collect-protocol/tree/master/register/Register.proto).
+And register steps followings [SkyWalking Trace Data Protocol v2](Trace-Data-Protocol-v2.md).
+
+- Service Register
+
+> POST http://localhost:12800/v2/service/register
+
+Input:
+
+```json
+{
+  "services": [
+    {
+      "type": "normal",
+      "serviceName": "Service Name"
+    }
+  ]
+}
+```
+
+Output JSON Array:
+
+```json
+[
+    {
+        "key": "Service Name",
+        "value": 2
+    }
+]
+```
+
+- Service instance Register
+
+> POST http://localhost:12800/v2/instance/register
+
+Input:
+
+```json
+{
+  "instances": [
+    {
+      "time": 1582428603392,
+      "instanceUUID": "NAME:Service Instance Name",
+      "properties": [
+        {
+          "key": "language",
+          "value": "Lua"
+        }
+      ],
+      "serviceId": 2
+    }
+  ]
+}
+```
+
+OutPut:
+
+```json
+{
+    "key": "NAME:Service Instance Name",
+    "value": 2
+}
+```
+
+- Service instance heartbeat
+
+> POST http://localhost:12800/v2/instance/heartbeat
+
+Input:
+
+```json
+{
+  "serviceInstanceId":20,
+  "time": 1582428603392,
+  "serviceInstanceUUID":"NAME:Service Instance Name"
+}
+```
+
+OutPut:
+
+```json
+{}
+```
+If your instance does not exists, and then, you need to clean your local service instance metadata in your application and re-do register:
 
 Review comment:
   ```suggestion
   If your instance does not exist, you need to clean your local service instance metadata in your application and re-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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] arugal commented on issue #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
arugal commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593426809
 
 
   `
   12:12:34.733 skywalking-e2e-container-local-http-api-with-nginx-lua-openresty> 2020/03/02 12:12:34 [error] 830#830: *5 [lua] client.lua:97: registerService(): Service register fails, no resolver defined to resolve "upstream", context: ngx.timer
   `
   
   Hi, according to the above log and the printed log of `SampleVerificationITCase`, `nginx` seems to have failed to register, please recheck.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#issuecomment-593174117
 
 
   Is this PR ready to review? `WIP` is still in the title, just want to confirm.

----------------------------------------------------------------
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 #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386067139
 
 

 ##########
 File path: test/e2e/e2e-http-api-with-nginx-lua/src/docker/rc.d/rc0-prepare.sh
 ##########
 @@ -0,0 +1,27 @@
+#!/usr/bin/env bash
+# Licensed to the SkyAPM 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.
+
+apt-get update && apt-get -y install git
+
+echo 'git clone skyalking-nginx-lua lib from https://github.com/apache/skywalking-nginx-lua.git'
+
+git clone https://github.com/apache/skywalking-nginx-lua.git /usr/share/skywalking-nginx-lua \
+  && cd /usr/share/skywalking-nginx-lua \
+  && git checkout 95684f3dcccab36f6592bf91944cdd5424e0d7a4 \
 
 Review comment:
   Could we set this parameter as a more clear place? Such as in the pom.xml?

----------------------------------------------------------------
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] JaredTan95 commented on a change in pull request #4399: [WIP]support http api for upstream trace.

Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on a change in pull request #4399: [WIP]support http api for upstream trace.
URL: https://github.com/apache/skywalking/pull/4399#discussion_r386086557
 
 

 ##########
 File path: .gitignore
 ##########
 @@ -20,3 +20,4 @@ OALLexer.tokens
 .externalToolBuilders
 /test/plugin/dist
 /test/plugin/workspace
+e2e-*
 
 Review comment:
   Local e2e will generate related files, reduce error commits.

----------------------------------------------------------------
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