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/12/24 10:22:14 UTC
[GitHub] [skywalking-client-js] Fine0830 opened a new pull request #23: feat: trace segments and set http header
Fine0830 opened a new pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23
Trace segments, intercept http traffic and set http header in the browser. Reference https://github.com/apache/skywalking/issues/4150
Screenshots
Signed-off-by: Qiuxia Fan <qi...@apache.org>
----------------------------------------------------------------
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] kezhenxu94 commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549248208
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
Review comment:
> Let's don't share the same traces with different requests of the same page in default. We could keep this as an option. But further more, we really should consider the manual tracingContext link mechanism.
I agree with this. That's why I asked 👇 , in an SPA web app, there may be many many spans in a browser session, 100+ is totally possible, which may cause trouble in skywalking-rocketbot-ui
> After reviewing the codes, one thing I want to confirm is that, all the spans in one "browser session" belong to a trace (same trace id), 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
[GitHub] [skywalking-client-js] wu-sheng commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549245079
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
Review comment:
Let's don't share the same traces with different requests of the same page in default. We could keep this as an option. But further more, we really should consider the manual tracingContext link mechanism.
----------------------------------------------------------------
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 #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-750850057
> If you meant E2E, it's totally possible, from the perspective of the E2E, browser agent has nothing special
A little special is mocking browser, as this should run in a real browser. Could you and @kezhenxu94 help on setting this and agent test tool when this is ready?
----------------------------------------------------------------
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 a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548538681
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
Got 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
[GitHub] [skywalking-client-js] kezhenxu94 commented on pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-750851172
> > If you meant E2E, it's totally possible, from the perspective of the E2E, browser agent has nothing special
>
> A little special is mocking browser, as this should run in a real browser. Could you and @kezhenxu94 help on setting this and agent test tool when this is ready?
Sure I'm glad to help, (btw, "mocking browser" is not that difficult as you might thought)
----------------------------------------------------------------
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] kezhenxu94 commented on a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548493607
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
I believe (from the comment) this is copied from somewhere and we cannot simply add the license header, please remove it and add license to the source and dist license file
----------------------------------------------------------------
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 a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549231990
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
+ const segment = {
+ traceId: uuid(),
+ service: options.service,
+ spans: [],
+ serviceInstance: options.serviceVersion,
+ traceSegmentId: options.segmentId,
+ } as SegmentFeilds;
+ const segCollector: { event: XMLHttpRequest; startTime: number }[] | any = [];
+ // inject interceptor
+ xhrInterceptor();
+ window.addEventListener('xhrReadyStateChange', (event: CustomEvent) => {
+ const xhrState = event.detail.readyState;
+
+ if (xhrState === 1) {
Review comment:
Sure, let me work on it a bit
----------------------------------------------------------------
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] arugal commented on pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
arugal commented on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-750848386
> @Fine0830 @arugal @kezhenxu94 I think we have chance to set up browser tracing in the ci?
Yes, just like in the demo environment :)
----------------------------------------------------------------
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 #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-751601848
Another thing, please update the documentation.
----------------------------------------------------------------
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 a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549228869
##########
File path: src/errors/ajax.ts
##########
@@ -28,7 +28,7 @@ class AjaxErrors extends Base {
const xhrSend = XMLHttpRequest.prototype.send;
const xhrEvent = (event: any) => {
try {
- if (event && event.currentTarget && event.currentTarget.status !== 200) {
+ if (event && event.currentTarget && event.currentTarget.status !== 200 && event.currentTarget.status !== 304) {
Review comment:
All 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
[GitHub] [skywalking-client-js] Fine0830 commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549258494
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
Review comment:
The previous logic has been modified. Please check screenshots https://github.com/apache/skywalking-client-js/pull/23#issue-545250734
----------------------------------------------------------------
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 a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549233788
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
+ const segment = {
+ traceId: uuid(),
+ service: options.service,
+ spans: [],
+ serviceInstance: options.serviceVersion,
+ traceSegmentId: options.segmentId,
+ } as SegmentFeilds;
+ const segCollector: { event: XMLHttpRequest; startTime: number }[] | any = [];
+ // inject interceptor
+ xhrInterceptor();
+ window.addEventListener('xhrReadyStateChange', (event: CustomEvent) => {
+ const xhrState = event.detail.readyState;
+
+ if (xhrState === 1) {
+ segCollector.push({
+ event: event.detail,
+ startTime: new Date().getTime(),
+ });
+ const traceIdStr = String(Base64.encode(segment.traceId));
+ const segmentId = String(Base64.encode(segment.traceSegmentId));
+ const service = String(Base64.encode(segment.service));
+ const instance = String(Base64.encode(segment.serviceInstance));
+ const endpoint = String(Base64.encode(options.pagePath));
+ const url = String(Base64.encode(location.href));
+ const index = segment.spans.length;
+ const values = `${1}-${traceIdStr}-${segmentId}-${index}-${service}-${instance}-${endpoint}-${url}`;
+
+ event.detail.setRequestHeader('sw8', values);
+ }
+ if (xhrState === 4) {
+ const endTime = new Date().getTime();
+ for (let i = 0; i < segCollector.length; i++) {
+ if (segCollector[i].event.status) {
+ const exitSpan: SpanFeilds = {
+ operationName: options.pagePath,
+ startTime: segCollector[i].startTime,
+ endTime,
+ spanId: segment.spans.length - 1 || 0,
+ spanLayer: SpanLayer,
+ spanType: SpanType,
+ isError: event.detail.status === 200 ? false : true,
Review comment:
HaHa....
----------------------------------------------------------------
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] kezhenxu94 commented on pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-750848771
> @Fine0830 @arugal @kezhenxu94 I think we have chance to set up browser tracing in the ci?
If you meant E2E, it's totally possible, from the perspective of the E2E, browser agent has nothing special
----------------------------------------------------------------
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 a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549250523
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
Review comment:
> After reviewing the codes, one thing I want to confirm is that, all the spans in one "browser session" belong to a trace (same trace id), right?
This needs to be fixed :) But this PR is closing to be ready, super cool. We need to consider a new English blog about this.
----------------------------------------------------------------
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 #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-751586078
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
[GitHub] [skywalking-client-js] wu-sheng commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549229726
##########
File path: src/errors/ajax.ts
##########
@@ -28,7 +28,7 @@ class AjaxErrors extends Base {
const xhrSend = XMLHttpRequest.prototype.send;
const xhrEvent = (event: any) => {
try {
- if (event && event.currentTarget && event.currentTarget.status !== 200) {
+ if (event && event.currentTarget && event.currentTarget.status !== 200 && event.currentTarget.status !== 304) {
Review comment:
Yes, `>=400` is correct check. 202 is also correct, 302 is only redirect.
----------------------------------------------------------------
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 a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549230858
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
+ const segment = {
+ traceId: uuid(),
+ service: options.service,
+ spans: [],
+ serviceInstance: options.serviceVersion,
+ traceSegmentId: options.segmentId,
+ } as SegmentFeilds;
+ const segCollector: { event: XMLHttpRequest; startTime: number }[] | any = [];
+ // inject interceptor
+ xhrInterceptor();
+ window.addEventListener('xhrReadyStateChange', (event: CustomEvent) => {
+ const xhrState = event.detail.readyState;
+
+ if (xhrState === 1) {
Review comment:
Could you add comments about the meaning of `xhrState` values? And add constant to represent 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
[GitHub] [skywalking-client-js] wu-sheng commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549234267
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
Review comment:
You haven't provided documentation yet, but from here, I think the trace segment will include one and only one span related to RPC, right?
___
Do you have any design/idea about how to make users to inject some spans related to this RPC? Such as, could user create a tracingContext(unfinished segment), and inject this here.
The segment only finished when the tracingContext finished(no active span).
We could add this later, but we should consider this as another core feature.
----------------------------------------------------------------
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 a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548537605
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
> Could we depend on the 3rd party lib directly rather than copying?
Sure. I think this is lighter than the 3rd party lib. However, It is okay if we depend on the 3rd party lib.
----------------------------------------------------------------
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 a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549238916
##########
File path: src/trace/segment.ts
##########
@@ -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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType, ReadyStatus } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
+ const segment = {
+ traceId: uuid(),
+ service: options.service,
+ spans: [],
+ serviceInstance: options.serviceVersion,
+ traceSegmentId: options.segmentId,
+ } as SegmentFeilds;
+ const segCollector: { event: XMLHttpRequest; startTime: number }[] | any = [];
+ // inject interceptor
+ xhrInterceptor();
+ window.addEventListener('xhrReadyStateChange', (event: CustomEvent) => {
+ const xhrState = event.detail.readyState;
+
+ // The values of xhrState are from https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/readyState
+ if (xhrState === ReadyStatus.OPENED) {
+ segCollector.push({
+ event: event.detail,
+ startTime: new Date().getTime(),
+ });
+ const traceIdStr = String(Base64.encode(segment.traceId));
+ const segmentId = String(Base64.encode(segment.traceSegmentId));
+ const service = String(Base64.encode(segment.service));
+ const instance = String(Base64.encode(segment.serviceInstance));
+ const endpoint = String(Base64.encode(options.pagePath));
+ const url = String(Base64.encode(location.href));
Review comment:
```suggestion
const peer = String(Base64.encode(location.href));
```
I think this name is more accurate?
----------------------------------------------------------------
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] kezhenxu94 commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549226933
##########
File path: src/errors/ajax.ts
##########
@@ -28,7 +28,7 @@ class AjaxErrors extends Base {
const xhrSend = XMLHttpRequest.prototype.send;
const xhrEvent = (event: any) => {
try {
- if (event && event.currentTarget && event.currentTarget.status !== 200) {
+ if (event && event.currentTarget && event.currentTarget.status !== 200 && event.currentTarget.status !== 304) {
Review comment:
Most (if not all) of the other agents use a condition `status >= 400` to determine the status is `error` or not, does it apply to this agent too?
----------------------------------------------------------------
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] kezhenxu94 commented on a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548493607
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
I believe (from the comment) this is copied from somewhere and we cannot simply add the license header, please remove the license header and add it original license to the source and dist license file
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
I believe (from the comment) this is copied from somewhere and we cannot simply add the license header, please remove the license header and add its original license to the source and dist license file
----------------------------------------------------------------
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 a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548538579
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
Copying is fine, if you think this is better, but you need to change the LICENSE in the root, and copy the LICENSE in the `/licenses` folder.
----------------------------------------------------------------
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] kezhenxu94 commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549234142
##########
File path: src/trace/segment.ts
##########
@@ -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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType, ReadyStatus } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
+ const segment = {
+ traceId: uuid(),
+ service: options.service,
+ spans: [],
+ serviceInstance: options.serviceVersion,
+ traceSegmentId: options.segmentId,
+ } as SegmentFeilds;
+ const segCollector: { event: XMLHttpRequest; startTime: number }[] | any = [];
+ // inject interceptor
+ xhrInterceptor();
+ window.addEventListener('xhrReadyStateChange', (event: CustomEvent) => {
+ const xhrState = event.detail.readyState;
+
+ // The values of xhtState are from https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/readyState
+ if (xhrState === ReadyStatus.OPENED) {
+ segCollector.push({
+ event: event.detail,
+ startTime: new Date().getTime(),
+ });
+ const traceIdStr = String(Base64.encode(segment.traceId));
+ const segmentId = String(Base64.encode(segment.traceSegmentId));
+ const service = String(Base64.encode(segment.service));
+ const instance = String(Base64.encode(segment.serviceInstance));
+ const endpoint = String(Base64.encode(options.pagePath));
+ const url = String(Base64.encode(location.href));
+ const index = segment.spans.length;
+ const values = `${1}-${traceIdStr}-${segmentId}-${index}-${service}-${instance}-${endpoint}-${url}`;
+
+ event.detail.setRequestHeader('sw8', values);
+ }
+ if (xhrState === ReadyStatus.DONE) {
+ const endTime = new Date().getTime();
+ for (let i = 0; i < segCollector.length; i++) {
+ if (segCollector[i].event.status) {
+ const exitSpan: SpanFeilds = {
+ operationName: options.pagePath,
+ startTime: segCollector[i].startTime,
+ endTime,
+ spanId: segment.spans.length - 1 || 0,
+ spanLayer: SpanLayer,
+ spanType: SpanType,
+ isError: event.detail.status >= 400 ? false : true,
Review comment:
😉 the logic is reversed?
```suggestion
isError: event.detail.status >= 400,
```
----------------------------------------------------------------
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 merged pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 merged pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23
----------------------------------------------------------------
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 a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548495320
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
Could we depend on the 3rd party lib directly rather than copying?
----------------------------------------------------------------
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 edited a comment on pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng edited a comment on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-750850057
> If you meant E2E, it's totally possible, from the perspective of the E2E, browser agent has nothing special
Not e2e. Just agent test tool for verifying the segment and span. A little special is mocking browser, as this should run in a real browser. Could you and @kezhenxu94 help on setting this and agent test tool when this is ready?
----------------------------------------------------------------
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 a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548538171
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
> I believe (from the comment) this is copied from somewhere and we cannot simply add the license header, please remove the license header and add its original license to the source and dist license file
Let me work on it a bit
----------------------------------------------------------------
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 #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#issuecomment-750846563
@Fine0830 @arugal @kezhenxu94 I think we have chance to set up browser tracing in the ci?
----------------------------------------------------------------
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 a change in pull request #23: [wip]feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r548538397
##########
File path: src/services/base64.ts
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+/**
+ *
+ * Base64 encode / decode
+ * http://www.webtoolkit.info/
+ *
+ **/
+export const Base64 = {
+ // private property
+ _keyStr: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=',
Review comment:
And keep the license as it used to be. This belongs to 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
[GitHub] [skywalking-client-js] kezhenxu94 commented on a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549231231
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
+ const segment = {
+ traceId: uuid(),
+ service: options.service,
+ spans: [],
+ serviceInstance: options.serviceVersion,
+ traceSegmentId: options.segmentId,
+ } as SegmentFeilds;
+ const segCollector: { event: XMLHttpRequest; startTime: number }[] | any = [];
+ // inject interceptor
+ xhrInterceptor();
+ window.addEventListener('xhrReadyStateChange', (event: CustomEvent) => {
+ const xhrState = event.detail.readyState;
+
+ if (xhrState === 1) {
+ segCollector.push({
+ event: event.detail,
+ startTime: new Date().getTime(),
+ });
+ const traceIdStr = String(Base64.encode(segment.traceId));
+ const segmentId = String(Base64.encode(segment.traceSegmentId));
+ const service = String(Base64.encode(segment.service));
+ const instance = String(Base64.encode(segment.serviceInstance));
+ const endpoint = String(Base64.encode(options.pagePath));
+ const url = String(Base64.encode(location.href));
+ const index = segment.spans.length;
+ const values = `${1}-${traceIdStr}-${segmentId}-${index}-${service}-${instance}-${endpoint}-${url}`;
+
+ event.detail.setRequestHeader('sw8', values);
+ }
+ if (xhrState === 4) {
+ const endTime = new Date().getTime();
+ for (let i = 0; i < segCollector.length; i++) {
+ if (segCollector[i].event.status) {
+ const exitSpan: SpanFeilds = {
+ operationName: options.pagePath,
+ startTime: segCollector[i].startTime,
+ endTime,
+ spanId: segment.spans.length - 1 || 0,
+ spanLayer: SpanLayer,
+ spanType: SpanType,
+ isError: event.detail.status === 200 ? false : true,
Review comment:
Forgot to change here as well?
----------------------------------------------------------------
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 a change in pull request #23: feat: trace segments and set http header
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #23:
URL: https://github.com/apache/skywalking-client-js/pull/23#discussion_r549240142
##########
File path: src/trace/segment.ts
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+import { Base64 } from 'js-base64';
+import xhrInterceptor from '../interceptors/xhr';
+import uuid from '../services/uuid';
+import Report from '../services/report';
+import { SegmentFeilds, SpanFeilds } from './type';
+import { SpanLayer, SpanType } from '../services/constant';
+import { CustomOptionsType } from '../types';
+
+export default function traceSegment(options: CustomOptionsType) {
Review comment:
> You haven't provided documentation yet, but from here, I think the trace segment will include one and only one span related to RPC, right?
>
Emm....No, the trace segment include spans of a service.
----------------------------------------------------------------
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