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/04 01:54:43 UTC
[GitHub] [skywalking-data-collect-protocol] arugal opened a new pull request
#18: Change BrowserPerfData#logs field
arugal opened a new pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18
According to my discussion with Qiuxia @Fine0830 , there is at most one `BrowserErrorLog`.
ref: [skywalking#issue](https://github.com/apache/skywalking/issues/4166)
----------------------------------------------------------------
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-data-collect-protocol] wu-sheng commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374448548
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
@Fine0830 Based on `BrowserPerfService#collect`, it includes error logs and perf details. For every page, the perf detail should be reported once, right? And at the same time, BrowserErrorLog could be one or more.
Then should we separate these two as two report service? Then the clients-js could choose whether to report one log or multiple logs as streaming.
----------------------------------------------------------------
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-data-collect-protocol] wu-sheng commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374459844
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
Yes. From your description, It seems more reasonable
----------------------------------------------------------------
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-data-collect-protocol] wu-sheng commented on issue #18:
Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#issuecomment-581914775
@Fine0830 Please 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-data-collect-protocol] Fine0830 commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374459384
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
@wu-sheng you mean we separate these two as error and perf report service? If that, it is OK.
----------------------------------------------------------------
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-data-collect-protocol] Fine0830 commented on issue #18:
Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on issue #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#issuecomment-582270415
Thanks. LGTM
----------------------------------------------------------------
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-data-collect-protocol] wu-sheng merged pull request
#18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18
----------------------------------------------------------------
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-data-collect-protocol] arugal commented on issue #18:
Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on issue #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#issuecomment-581910723
Please review @Fine0830 @wu-sheng
----------------------------------------------------------------
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-data-collect-protocol] arugal commented on a change in
pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r375086744
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (BrowserPerfData) returns (Commands) {
+ }
+
+ // report one or more error logs for pages, could report multiple times.
+ rpc collectErrorLogs (stream BrowserErrorLog) returns (Commands) {
}
}
message BrowserPerfData {
+ int32 serviceId = 1;
+ // Service version in browser is the Instance concept in the backend.
+ int32 serviceVersionId = 2;
+ // Perf data time
+ int64 time = 3;
Review comment:
`time` and `pagePathId` need to be used in `browser-receiver`, please keep 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-data-collect-protocol] Fine0830 commented on a change
in pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r375086868
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (BrowserPerfData) returns (Commands) {
+ }
+
+ // report one or more error logs for pages, could report multiple times.
+ rpc collectErrorLogs (stream BrowserErrorLog) returns (Commands) {
}
}
message BrowserPerfData {
+ int32 serviceId = 1;
+ // Service version in browser is the Instance concept in the backend.
+ int32 serviceVersionId = 2;
+ // Perf data time
+ int64 time = 3;
+ // Page path in browser is the endpoint concept in the backend
+ // Page path in the browser, mostly it is URI, without parameter
+ string pagePath = 4;
+ int32 pagePathId = 5;
+ // Unit of all time related field should be `ms`.
+ int32 redirectTime = 6;
+ int32 dnsTime = 7;
+ int32 reqTime = 8;
+ // analysis dom tree time
+ int32 domAnalysisTime = 9;
+ int32 domReadyTime = 10;
+ // page blank time
+ int32 blankTime = 11;
+}
+
+message BrowserErrorLog {
// UUID
string uniqueId = 1;
int32 serviceId = 2;
// Service version in browser is the Instance concept in the backend.
int32 serviceVersionId = 3;
- // Data time
+ // Error log time
int64 time = 4;
Review comment:
As 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-data-collect-protocol] wu-sheng commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374449159
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
Changing the service to
```proto
// Collect performance raw data from the browser.
service BrowserPerfService {
// report one or more error logs for pages, could report multiple times.
rpc collectErrorLogs (stream logs) returns (Commands) {
}
// report once per page
rpc collectPerfData (perfDetail) returns (Commands) {
}
}
```
----------------------------------------------------------------
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-data-collect-protocol] wu-sheng commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374497222
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
@arugal From https://github.com/apache/skywalking-query-protocol/pull/30#issuecomment-581715189, I think this comment also supports this change. Only the error log should be persistent and provide query.
----------------------------------------------------------------
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-data-collect-protocol] arugal commented on a change in
pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374671974
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (stream BrowserPerfData) returns (Commands) {
Review comment:
removed
----------------------------------------------------------------
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-data-collect-protocol] Fine0830 commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374444679
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
When client-js catch an error, it can report log to server at once. if there are multiple errors on client, it can repeatedly report.
----------------------------------------------------------------
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-data-collect-protocol] wu-sheng commented on a change
in pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374671091
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (stream BrowserPerfData) returns (Commands) {
Review comment:
Report once, then no `stream`.
----------------------------------------------------------------
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-data-collect-protocol] wu-sheng commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374438879
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
Why remove `repeated`? There could be multiple error logs for page loading.
----------------------------------------------------------------
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-data-collect-protocol] arugal commented on a change in
pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r375088433
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (BrowserPerfData) returns (Commands) {
+ }
+
+ // report one or more error logs for pages, could report multiple times.
+ rpc collectErrorLogs (stream BrowserErrorLog) returns (Commands) {
}
}
message BrowserPerfData {
+ int32 serviceId = 1;
+ // Service version in browser is the Instance concept in the backend.
+ int32 serviceVersionId = 2;
+ // Perf data time
+ int64 time = 3;
Review comment:
It is generated on the server only when `time` is empty.
----------------------------------------------------------------
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-data-collect-protocol] arugal commented on a change in
pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374460482
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
I'll change the proto.
----------------------------------------------------------------
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-data-collect-protocol] Fine0830 commented on a change
in pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r375087438
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (BrowserPerfData) returns (Commands) {
+ }
+
+ // report one or more error logs for pages, could report multiple times.
+ rpc collectErrorLogs (stream BrowserErrorLog) returns (Commands) {
}
}
message BrowserPerfData {
+ int32 serviceId = 1;
+ // Service version in browser is the Instance concept in the backend.
+ int32 serviceVersionId = 2;
+ // Perf data time
+ int64 time = 3;
Review comment:
OK
----------------------------------------------------------------
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-data-collect-protocol] Fine0830 commented on a change
in pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r375086868
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (BrowserPerfData) returns (Commands) {
+ }
+
+ // report one or more error logs for pages, could report multiple times.
+ rpc collectErrorLogs (stream BrowserErrorLog) returns (Commands) {
}
}
message BrowserPerfData {
+ int32 serviceId = 1;
+ // Service version in browser is the Instance concept in the backend.
+ int32 serviceVersionId = 2;
+ // Perf data time
+ int64 time = 3;
+ // Page path in browser is the endpoint concept in the backend
+ // Page path in the browser, mostly it is URI, without parameter
+ string pagePath = 4;
+ int32 pagePathId = 5;
+ // Unit of all time related field should be `ms`.
+ int32 redirectTime = 6;
+ int32 dnsTime = 7;
+ int32 reqTime = 8;
+ // analysis dom tree time
+ int32 domAnalysisTime = 9;
+ int32 domReadyTime = 10;
+ // page blank time
+ int32 blankTime = 11;
+}
+
+message BrowserErrorLog {
// UUID
string uniqueId = 1;
int32 serviceId = 2;
// Service version in browser is the Instance concept in the backend.
int32 serviceVersionId = 3;
- // Data time
+ // Error log time
int64 time = 4;
Review comment:
As 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-data-collect-protocol] Fine0830 commented on a change
in pull request #18: Enhancement browser protocol
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #18: Enhancement browser protocol
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r375085592
##########
File path: browser/BrowserPerf.proto
##########
@@ -25,36 +25,56 @@ import "common/common.proto";
// Collect performance raw data from browser.
service BrowserPerfService {
- rpc collect (stream BrowserPerfData) returns (Commands) {
+
+ // report once per page
+ rpc collectPerfData (BrowserPerfData) returns (Commands) {
+ }
+
+ // report one or more error logs for pages, could report multiple times.
+ rpc collectErrorLogs (stream BrowserErrorLog) returns (Commands) {
}
}
message BrowserPerfData {
+ int32 serviceId = 1;
+ // Service version in browser is the Instance concept in the backend.
+ int32 serviceVersionId = 2;
+ // Perf data time
+ int64 time = 3;
Review comment:
If `time` field can be generated on server, could we remove `time` field?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] Fine0830 commented on a change
in pull request #18: Change BrowserPerfData#logs field
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #18: Change BrowserPerfData#logs field
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/18#discussion_r374456921
##########
File path: browser/BrowserPerf.proto
##########
@@ -41,7 +41,7 @@ message BrowserPerfData {
// Page path in the browser, mostly it is URI, without parameter
string pagePath = 5;
int32 pagePathId = 6;
- repeated BrowserErrorLog logs = 7;
+ BrowserErrorLog errorLog = 7;
Review comment:
> @Fine0830 Based on `BrowserPerfService#collect`, it includes error logs and perf details. For every page, the perf detail should be reported once, right? And at the same time, BrowserErrorLog could be one or more.
>
> Then should we separate these two as two report service? Then the clients-js could choose whether to report one log or multiple logs as streaming.
Generally, when user open the page, perf data is produced. Error logs are produced in many case, like loading, click, user interactions. Now, there is a list to store error logs and perf logs in client-js, then the list item will report logs to server in turn. Another way, we can regularly report all logs in the list.
----------------------------------------------------------------
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