fix: Address PR review

Author: AbhiPrasad

packages/apm/src/integrations/tracing.ts

@@ -660,9 +660,9 @@ export class Tracing implements Integration {
             break;
           case 'resource':
             const resourceName = entry.name.replace(window.location.origin, '');
-            if (entry.initiatorType === 'xmlhttprequest' || entry.initiatorType === 'fetch') {
-              // do nothing, we can't adjust timings just based on urls
-            } else {
+            // we already instrument based on fetch and xhr, so we don't need to
+            // duplicate spans here.
+            if (entry.initiatorType !== 'xmlhttprequest' && entry.initiatorType !== 'fetch') {
               const resource = transactionSpan.startChild({
                 description: `${entry.initiatorType} ${resourceName}`,
                 op: `resource`,

packages/tracing/src/browser/metrics.ts

@@ -214,6 +214,8 @@ function addResourceSpans(
   duration: number,
   timeOrigin: number,
 ): number | undefined {
+  // we already instrument based on fetch and xhr, so we don't need to
+  // duplicate spans here.
   if (entry.initiatorType === 'xmlhttprequest' || entry.initiatorType === 'fetch') {
     return undefined;
   }

packages/tracing/src/browser/request.ts

@@ -104,7 +104,7 @@ export function registerRequestInstrumentation(_options?: Partial<RequestInstrum
   if (traceFetch) {
     addInstrumentationHandler({
       callback: (handlerData: FetchData) => {
-        fetchCallback(handlerData, shouldCreateSpan, spans);
+        _fetchCallback(handlerData, shouldCreateSpan, spans);
       },
       type: 'fetch',
     });
@@ -123,7 +123,7 @@ export function registerRequestInstrumentation(_options?: Partial<RequestInstrum
 /**
  * Create and track fetch request spans
  */
-export function fetchCallback(
+export function _fetchCallback(
   handlerData: FetchData,
   shouldCreateSpan: (url: string) => boolean,
   spans: Record<string, Span>,

packages/tracing/src/idletransaction.ts

@@ -118,9 +118,7 @@ export class IdleTransaction extends Transaction {
     this._prevHeartbeatString = heartbeatString;
 
     if (this._heartbeatCounter >= 3) {
-      logger.log(
-        `[Tracing] Transaction: ${SpanStatus.DeadlineExceeded} -> Heartbeat safeguard kicked in since content hasn't changed for 3 beats`,
-      );
+      logger.log(`[Tracing] Transaction finished because of no change for 3 heart beats`);
       this.setStatus(SpanStatus.DeadlineExceeded);
       this.setTag('heartbeat', 'failed');
       this.finish();

packages/tracing/test/browser/request.test.ts

@@ -2,7 +2,7 @@ import { BrowserClient } from '@sentry/browser';
 import { Hub, makeMain } from '@sentry/hub';
 
 import { Span, Transaction } from '../../src';
-import { fetchCallback, FetchData, registerRequestInstrumentation } from '../../src/browser/request';
+import { _fetchCallback, FetchData, registerRequestInstrumentation } from '../../src/browser/request';
 import { addExtensionMethods } from '../../src/hubextensions';
 
 declare global {
@@ -69,21 +69,17 @@ describe('registerRequestInstrumentation', () => {
   });
 });
 
-describe('fetchCallback', () => {
+describe('_fetchCallback()', () => {
   let hub: Hub;
   let transaction: Transaction;
-  beforeEach(() => {
+  beforeAll(() => {
     hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
     makeMain(hub);
-    transaction = hub.startTransaction({ name: 'organizations/users/:userid', op: 'pageload' }) as Transaction;
-    hub.configureScope(scope => scope.setSpan(transaction));
   });
 
-  afterEach(() => {
-    if (transaction) {
-      transaction.finish();
-    }
-    hub.configureScope(scope => scope.setSpan(undefined));
+  beforeEach(() => {
+    transaction = hub.startTransaction({ name: 'organizations/users/:userid', op: 'pageload' }) as Transaction;
+    hub.configureScope(scope => scope.setSpan(transaction));
   });
 
   it('does not create span if it should not be created', () => {
@@ -98,7 +94,7 @@ describe('fetchCallback', () => {
     };
     const spans = {};
 
-    fetchCallback(data, shouldCreateSpan, spans);
+    _fetchCallback(data, shouldCreateSpan, spans);
     expect(spans).toEqual({});
   });
 
@@ -110,7 +106,7 @@ describe('fetchCallback', () => {
     };
     const spans = {};
 
-    fetchCallback(data, shouldCreateSpan, spans);
+    _fetchCallback(data, shouldCreateSpan, spans);
     expect(spans).toEqual({});
   });
 
@@ -127,7 +123,7 @@ describe('fetchCallback', () => {
     const spans: Record<string, Span> = {};
 
     // Start fetch request
-    fetchCallback(data, shouldCreateSpan, spans);
+    _fetchCallback(data, shouldCreateSpan, spans);
     const spanKey = Object.keys(spans)[0];
 
     const fetchSpan = spans[spanKey];
@@ -151,7 +147,7 @@ describe('fetchCallback', () => {
     };
 
     // End fetch request
-    fetchCallback(newData, shouldCreateSpan, spans);
+    _fetchCallback(newData, shouldCreateSpan, spans);
     expect(spans).toEqual({});
     if (transaction.spanRecorder) {
       expect(transaction.spanRecorder.spans[1].endTimestamp).toBeDefined();