Merge branch 'develop' into feat/add-devcontainer

* develop: ref: Add external contributor to CHANGELOG.md (getsentry#13500) test: Avoid race conditions with symlinks (getsentry#13498) fix(node): Suppress tracing for transport request execution rather than transport creation (getsentry#13491) ci: Ensure cache save happens after install step (getsentry#13497) test: Increase timeout for redis-cache test & docker-compose (getsentry#13499) Fix config example in README.md for Nuxt (getsentry#13496)
martinhaintz · Aug 28, 2024 · d0aa570 · d0aa570
2 parents e989e5c + 6479e88
commit d0aa570
Show file tree

Hide file tree

Showing 13 changed files with 122 additions and 130 deletions.
diff --git a/.github/actions/install-playwright/action.yml b/.github/actions/install-playwright/action.yml
@@ -21,15 +21,6 @@ runs:
             ~/.cache/ms-playwright
           key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }}
 
-      # Only store cache on develop branch
-      - name: Store cached playwright binaries
-        uses: actions/cache/save@v4
-        if: github.event_name == 'push' && github.ref == 'refs/heads/develop'
-        with:
-          path: |
-            ~/.cache/ms-playwright
-          key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }}
-
       # We always install all browsers, if uncached
       - name: Install Playwright dependencies (uncached)
         run: npx playwright install chromium webkit firefox --with-deps
@@ -40,3 +31,12 @@ runs:
         run: npx playwright install-deps ${{ inputs.browsers || 'chromium webkit firefox' }}
         if: steps.playwright-cache.outputs.cache-hit == 'true'
         shell: bash
+
+      # Only store cache on develop branch
+      - name: Store cached playwright binaries
+        uses: actions/cache/save@v4
+        if: github.event_name == 'push' && github.ref == 'refs/heads/develop'
+        with:
+          path: |
+            ~/.cache/ms-playwright
+          key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+Work in this release was contributed by @leopoldkristjansson. Thank you for your contribution!
+
 ## 8.27.0
 
 ### Important Changes

diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts
@@ -4,7 +4,7 @@ import type { Package } from '@sentry/types';
 import HtmlWebpackPlugin, { createHtmlTagObject } from 'html-webpack-plugin';
 import type { Compiler } from 'webpack';
 
-import { addStaticAsset, addStaticAssetSymlink } from './staticAssets';
+import { addStaticAsset, symlinkAsset } from './staticAssets';
 
 const LOADER_TEMPLATE = fs.readFileSync(path.join(__dirname, '../fixtures/loader.js'), 'utf-8');
 const PACKAGES_DIR = path.join(__dirname, '..', '..', '..', 'packages');
@@ -214,7 +214,10 @@ class SentryScenarioGenerationPlugin {
                 src: 'cdn.bundle.js',
               });
 
-          addStaticAssetSymlink(this.localOutPath, path.resolve(PACKAGES_DIR, bundleName, bundlePath), 'cdn.bundle.js');
+          symlinkAsset(
+            path.resolve(PACKAGES_DIR, bundleName, bundlePath),
+            path.join(this.localOutPath, 'cdn.bundle.js'),
+          );
 
           if (useLoader) {
             const loaderConfig = LOADER_CONFIGS[bundleKey];
@@ -245,14 +248,13 @@ class SentryScenarioGenerationPlugin {
               const fileName = `${integration}.bundle.js`;
 
               // We add the files, but not a script tag - they are lazy-loaded
-              addStaticAssetSymlink(
-                this.localOutPath,
+              symlinkAsset(
                 path.resolve(
                   PACKAGES_DIR,
                   'feedback',
                   BUNDLE_PATHS['feedback']?.[integrationBundleKey]?.replace('[INTEGRATION_NAME]', integration) || '',
                 ),
-                fileName,
+                path.join(this.localOutPath, fileName),
               );
             });
           }
@@ -262,26 +264,23 @@ class SentryScenarioGenerationPlugin {
           if (baseIntegrationFileName) {
             this.requiredIntegrations.forEach(integration => {
               const fileName = `${integration}.bundle.js`;
-              addStaticAssetSymlink(
-                this.localOutPath,
+              symlinkAsset(
                 path.resolve(
                   PACKAGES_DIR,
                   'browser',
                   baseIntegrationFileName.replace('[INTEGRATION_NAME]', integration),
                 ),
-                fileName,
+                path.join(this.localOutPath, fileName),
               );
 
               if (integration === 'feedback') {
-                addStaticAssetSymlink(
-                  this.localOutPath,
+                symlinkAsset(
                   path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-modal.js'),
-                  'feedback-modal.bundle.js',
+                  path.join(this.localOutPath, 'feedback-modal.bundle.js'),
                 );
-                addStaticAssetSymlink(
-                  this.localOutPath,
+                symlinkAsset(
                   path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-screenshot.js'),
-                  'feedback-screenshot.bundle.js',
+                  path.join(this.localOutPath, 'feedback-screenshot.bundle.js'),
                 );
               }
 
@@ -295,10 +294,9 @@ class SentryScenarioGenerationPlugin {
 
           const baseWasmFileName = BUNDLE_PATHS['wasm']?.[integrationBundleKey];
           if (this.requiresWASMIntegration && baseWasmFileName) {
-            addStaticAssetSymlink(
-              this.localOutPath,
+            symlinkAsset(
               path.resolve(PACKAGES_DIR, 'wasm', baseWasmFileName),
-              'wasm.bundle.js',
+              path.join(this.localOutPath, 'wasm.bundle.js'),
             );
 
             const wasmObject = createHtmlTagObject('script', {

diff --git a/dev-packages/browser-integration-tests/utils/staticAssets.ts b/dev-packages/browser-integration-tests/utils/staticAssets.ts
@@ -22,23 +22,11 @@ export function addStaticAsset(localOutPath: string, fileName: string, cb: () =>
   symlinkAsset(newPath, path.join(localOutPath, fileName));
 }
 
-export function addStaticAssetSymlink(localOutPath: string, originalPath: string, fileName: string): void {
-  const newPath = path.join(STATIC_DIR, fileName);
-
-  // Only copy files once
-  if (!fs.existsSync(newPath)) {
-    fs.symlinkSync(originalPath, newPath);
-  }
-
-  symlinkAsset(newPath, path.join(localOutPath, fileName));
-}
-
-function symlinkAsset(originalPath: string, targetPath: string): void {
+export function symlinkAsset(originalPath: string, targetPath: string): void {
   try {
-    fs.unlinkSync(targetPath);
+    fs.linkSync(originalPath, targetPath);
   } catch {
-    // ignore errors here
+    // ignore errors here, probably means the file already exists
+    // Since we always build into a new directory for each test, we can safely ignore this
   }
-
-  fs.linkSync(originalPath, targetPath);
 }
diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts
@@ -1,7 +1,7 @@
 import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
 
 // When running docker compose, we need a larger timeout, as this takes some time...
-jest.setTimeout(75000);
+jest.setTimeout(90000);
 
 describe('redis cache auto instrumentation', () => {
   afterAll(() => {

diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts
@@ -124,7 +124,7 @@ async function runDockerCompose(options: DockerOptions): Promise<VoidFunction> {
     const timeout = setTimeout(() => {
       close();
       reject(new Error('Timed out waiting for docker-compose'));
-    }, 60_000);
+    }, 75_000);
 
     function newData(data: Buffer): void {
       const text = data.toString('utf8');

diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts
@@ -47,6 +47,7 @@ export function makeFetchTransport(
     }
 
     try {
+      // TODO: This may need a `suppresTracing` call in the future when we switch the browser SDK to OTEL
       return nativeFetch(options.url, requestOptions).then(response => {
         pendingBodySize -= requestSize;
         pendingCount--;

diff --git a/packages/cloudflare/src/transport.ts b/packages/cloudflare/src/transport.ts
@@ -1,4 +1,4 @@
-import { createTransport } from '@sentry/core';
+import { createTransport, suppressTracing } from '@sentry/core';
 import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
 import { SentryError } from '@sentry/utils';
 
@@ -89,14 +89,16 @@ export function makeCloudflareTransport(options: CloudflareTransportOptions): Tr
       ...options.fetchOptions,
     };
 
-    return fetch(options.url, requestOptions).then(response => {
-      return {
-        statusCode: response.status,
-        headers: {
-          'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
-          'retry-after': response.headers.get('Retry-After'),
-        },
-      };
+    return suppressTracing(() => {
+      return fetch(options.url, requestOptions).then(response => {
+        return {
+          statusCode: response.status,
+          headers: {
+            'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
+            'retry-after': response.headers.get('Retry-After'),
+          },
+        };
+      });
     });
   }
 

diff --git a/packages/deno/src/transports/index.ts b/packages/deno/src/transports/index.ts
@@ -1,4 +1,4 @@
-import { createTransport } from '@sentry/core';
+import { createTransport, suppressTracing } from '@sentry/core';
 import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
 import { consoleSandbox, logger, rejectedSyncPromise } from '@sentry/utils';
 
@@ -37,14 +37,16 @@ export function makeFetchTransport(options: DenoTransportOptions): Transport {
     };
 
     try {
-      return fetch(options.url, requestOptions).then(response => {
-        return {
-          statusCode: response.status,
-          headers: {
-            'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
-            'retry-after': response.headers.get('Retry-After'),
-          },
-        };
+      return suppressTracing(() => {
+        return fetch(options.url, requestOptions).then(response => {
+          return {
+            statusCode: response.status,
+            headers: {
+              'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
+              'retry-after': response.headers.get('Retry-After'),
+            },
+          };
+        });
       });
     } catch (e) {
       return rejectedSyncPromise(e);

diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -9,7 +9,6 @@ import {
   getCapturedScopesOnSpan,
   getCurrentScope,
   getIsolationScope,
-  isSentryRequestUrl,
   setCapturedScopesOnSpan,
 } from '@sentry/core';
 import { getClient } from '@sentry/opentelemetry';
@@ -102,10 +101,6 @@ export const instrumentHttp = Object.assign(
           return false;
         }
 
-        if (isSentryRequestUrl(url, getClient())) {
-          return true;
-        }
-
         const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests;
         if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) {
           return true;

diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts
@@ -79,11 +79,8 @@ export function makeNodeTransport(options: NodeTransportOptions): Transport {
     ? (new HttpsProxyAgent(proxy) as http.Agent)
     : new nativeHttpModule.Agent({ keepAlive, maxSockets: 30, timeout: 2000 });
 
-  // This ensures we do not generate any spans in OpenTelemetry for the transport
-  return suppressTracing(() => {
-    const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent);
-    return createTransport(options, requestExecutor);
-  });
+  const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent);
+  return createTransport(options, requestExecutor);
 }
 
 /**
@@ -122,54 +119,59 @@ function createRequestExecutor(
   const { hostname, pathname, port, protocol, search } = new URL(options.url);
   return function makeRequest(request: TransportRequest): Promise<TransportMakeRequestResponse> {
     return new Promise((resolve, reject) => {
-      let body = streamFromBody(request.body);
-
-      const headers: Record<string, string> = { ...options.headers };
-
-      if (request.body.length > GZIP_THRESHOLD) {
-        headers['content-encoding'] = 'gzip';
-        body = body.pipe(createGzip());
-      }
-
-      const req = httpModule.request(
-        {
-          method: 'POST',
-          agent,
-          headers,
-          hostname,
-          path: `${pathname}${search}`,
-          port,
-          protocol,
-          ca: options.caCerts,
-        },
-        res => {
-          res.on('data', () => {
-            // Drain socket
-          });
-
-          res.on('end', () => {
-            // Drain socket
-          });
-
-          res.setEncoding('utf8');
-
-          // "Key-value pairs of header names and values. Header names are lower-cased."
-          // https://nodejs.org/api/http.html#http_message_headers
-          const retryAfterHeader = res.headers['retry-after'] ?? null;
-          const rateLimitsHeader = res.headers['x-sentry-rate-limits'] ?? null;
-
-          resolve({
-            statusCode: res.statusCode,
-            headers: {
-              'retry-after': retryAfterHeader,
-              'x-sentry-rate-limits': Array.isArray(rateLimitsHeader) ? rateLimitsHeader[0] || null : rateLimitsHeader,
-            },
-          });
-        },
-      );
-
-      req.on('error', reject);
-      body.pipe(req);
+      // This ensures we do not generate any spans in OpenTelemetry for the transport
+      suppressTracing(() => {
+        let body = streamFromBody(request.body);
+
+        const headers: Record<string, string> = { ...options.headers };
+
+        if (request.body.length > GZIP_THRESHOLD) {
+          headers['content-encoding'] = 'gzip';
+          body = body.pipe(createGzip());
+        }
+
+        const req = httpModule.request(
+          {
+            method: 'POST',
+            agent,
+            headers,
+            hostname,
+            path: `${pathname}${search}`,
+            port,
+            protocol,
+            ca: options.caCerts,
+          },
+          res => {
+            res.on('data', () => {
+              // Drain socket
+            });
+
+            res.on('end', () => {
+              // Drain socket
+            });
+
+            res.setEncoding('utf8');
+
+            // "Key-value pairs of header names and values. Header names are lower-cased."
+            // https://nodejs.org/api/http.html#http_message_headers
+            const retryAfterHeader = res.headers['retry-after'] ?? null;
+            const rateLimitsHeader = res.headers['x-sentry-rate-limits'] ?? null;
+
+            resolve({
+              statusCode: res.statusCode,
+              headers: {
+                'retry-after': retryAfterHeader,
+                'x-sentry-rate-limits': Array.isArray(rateLimitsHeader)
+                  ? rateLimitsHeader[0] || null
+                  : rateLimitsHeader,
+              },
+            });
+          },
+        );
+
+        req.on('error', reject);
+        body.pipe(req);
+      });
     });
   };
 }
diff --git a/packages/nuxt/README.md b/packages/nuxt/README.md
@@ -96,7 +96,7 @@ Add a `sentry.client.config.(js|ts)` file to the root of your project:
 import * as Sentry from '@sentry/nuxt';
 
 Sentry.init({
-  dsn: env.DSN,
+  dsn: process.env.DSN,
 });
 ```