apply review suggestions

Author: Lms24

packages/tracing/src/browser/request.ts

@@ -10,8 +10,6 @@ import {
 
 import { getActiveTransaction, hasTracingEnabled } from '../utils';
 
-// TODO (v8): Remove `tracingOrigins`
-export const DEFAULT_TRACING_ORIGINS = ['localhost', /^\//];
 export const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\//];
 
 /** Options for Request Instrumentation */
@@ -140,13 +138,16 @@ export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumenta
 /**
  * Creates a function that determines whether to attach tracing headers to a request.
  * This was extracted from `instrumentOutgoingRequests` to make it easier to test shouldAttachHeaders.
+ * We only export this fuction for testing purposes.
  * TODO (v8): Remove `tracingOrigins` which should drastically simplify this function.
  */
 export function makeShouldAttachHeaders(
   tracePropagationTargets: (string | RegExp)[] | undefined,
   tracingOrigins: (string | RegExp)[] | undefined,
 ) {
   return (url: string): boolean => {
+    // TODO (v8): Replace the code below with this one-liner:
+    // return stringMatchesSomePattern(url, tracePropagationTargets || DEFAULT_TRACE_PROPAGATION_TARGETS);
     if (tracePropagationTargets || tracingOrigins) {
       return stringMatchesSomePattern(url, tracePropagationTargets || tracingOrigins);
     }

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

@@ -398,10 +398,12 @@ describe('[pre-v8]: shouldAttachHeaders', () => {
       const shouldAttachHeaders = makeShouldAttachHeaders(['example.com'], undefined);
       expect(shouldAttachHeaders('http://example.com')).toBe(true);
     });
+
     it('should return `false` if tracePropagationTargets array is empty', () => {
       const shouldAttachHeaders = makeShouldAttachHeaders([], ['localhost']);
       expect(shouldAttachHeaders('http://localhost:3000/test')).toBe(false);
     });
+
     it("should return `false` if tracePropagationTargets array doesn't match", () => {
       const shouldAttachHeaders = makeShouldAttachHeaders(['example.com'], ['localhost']);
       expect(shouldAttachHeaders('http://localhost:3000/test')).toBe(false);
@@ -413,10 +415,12 @@ describe('[pre-v8]: shouldAttachHeaders', () => {
       const shouldAttachHeaders = makeShouldAttachHeaders(undefined, ['example.com']);
       expect(shouldAttachHeaders('http://example.com')).toBe(true);
     });
+
     it('should return `false` if tracePropagationTargets array is empty', () => {
       const shouldAttachHeaders = makeShouldAttachHeaders(undefined, []);
       expect(shouldAttachHeaders('http://localhost:3000/test')).toBe(false);
     });
+
     it("should return `false` if tracePropagationTargets array doesn't match", () => {
       const shouldAttachHeaders = makeShouldAttachHeaders(undefined, ['example.com']);
       expect(shouldAttachHeaders('http://localhost:3000/test')).toBe(false);
@@ -433,6 +437,7 @@ describe('[pre-v8]: shouldAttachHeaders', () => {
       const shouldAttachHeaders = makeShouldAttachHeaders(undefined, undefined);
       expect(shouldAttachHeaders(url)).toBe(true);
     });
+
     it.each(['notmydoman/api/test', 'example.com'])('return `false` for urls not matching defaults (%s)', url => {
       const shouldAttachHeaders = makeShouldAttachHeaders(undefined, undefined);
       expect(shouldAttachHeaders(url)).toBe(false);