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 2021/04/28 03:56:34 UTC

[GitHub] [skywalking-client-js] rovast opened a new pull request #52: feat: add report interval params to register method

rovast opened a new pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52


   The current `register` method report segment is 5mins, which may be customized by the user.
   
   ```typescript
   ClientMonitor.register({
     collector: 'http://127.0.0.1:8080',
     service: 'test-ui',
     pagePath: '/current/page/name',
     serviceVersion: 'v1.0.0',
     autoReportInterval: 300000,
   });
   ```


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



[GitHub] [skywalking-client-js] Fine0830 commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
Fine0830 commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-828451202


   The current interval is per 5min, if there are segments.


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



[GitHub] [skywalking-client-js] Fine0830 commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
Fine0830 commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-828449355


   From my perspective, user only care whether the data is received or not. On this premise, `client-js` minimizes performance overhead. For the same reason, If this is added, error logs and performance also need to do this. There will be more configurations 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



[GitHub] [skywalking-client-js] Fine0830 commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
Fine0830 commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-828453868


   > @Fine0830 OK, I got it.
   > 
   > But there may still have a problem to solve if we send XHR request on `window.onbeforeunload` event. And that is, we should send XHR **Synchronous** instead of **asynchronous** on unload event, to ensure we report successfully.
   > 
   > So, the `function sendByXhr` @ `src/services/report.ts` may extend a param to control async action.
   > 
   > And here is some linked article
   > 
   > * https://stackoverflow.com/questions/4945932/window-onbeforeunload-ajax-request-in-chrome
   > * https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/Synchronous_and_Asynchronous_Requests
   
   So this async action make serve side doesn't received data? What is your problem?


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



[GitHub] [skywalking-client-js] rovast commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
rovast commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-828253089


   @Fine0830 OK, I got it. 
   
   But there may still have a problem to solve if we send XHR request on `window.onbeforeunload` event. And that is, we should send  XHR **Synchronous** instead of **asynchronous** on unload event, to ensure we report successfully.
   
   So, the `function sendByXhr` @ `src/services/report.ts` may extend a param to control async action.
   
   And here is some linked article
   - https://stackoverflow.com/questions/4945932/window-onbeforeunload-ajax-request-in-chrome
   - https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/Synchronous_and_Asynchronous_Requests


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



[GitHub] [skywalking-client-js] wu-sheng commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-828458586


   > From my perspective, user only care whether the data is received or not. On this premise, `client-js` minimizes performance overhead. For the same reason, If this is added, error logs and performance also need to do this. There will be more configurations here.
   
   Make sense. @rovast I also want to ask, why do you need the data so urgent?


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



[GitHub] [skywalking-client-js] Fine0830 commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
Fine0830 commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-828184946


   There is another way to report segments, except per 5min to report. When pages are closed, all segments would be reported to serve side. I don't think the configuration is necessary. WDYT? @wu-sheng @arugal 


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



[GitHub] [skywalking-client-js] rovast closed pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
rovast closed pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-client-js] wu-sheng commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-828332631


   @Fine0830 What is the current interval? I think configurable is not a bad thing at least. If users want the data reported in higher frequency, I have no concern. Once our default value is good and reasonable, I am good.


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



[GitHub] [skywalking-client-js] eightHundreds commented on pull request #52: feat: add report interval params to register method

Posted by GitBox <gi...@apache.org>.
eightHundreds commented on pull request #52:
URL: https://github.com/apache/skywalking-client-js/pull/52#issuecomment-1085369243


   [    window.onbeforeunload = () => {
       
       ](https://github.com/apache/skywalking-client-js/blob/b32f9f00a84ba0f74faae2165964ec50c61677ea/src/services/task.ts#L38)
       
       
       
   will override user's  event listener


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org