From 12d5acfb89249938ed1503611524f9fdd5ab780d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 6 May 2024 08:05:06 +0200 Subject: [PATCH 01/12] test(feedback): Re-add feedback CDN tests (#11890) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts https://github.com/getsentry/sentry-javascript/pull/11888, and ensures the feedback tests actually work on CDN. For this, we now ensure to serve this locally, so this will work also on release branches. It means you have to use `getLocalTestUrl` instead of `getLocalTestPath` to work. (side note: We can/should probably just remove `getLocalTestPath` overall 🤔 URL based is much more realistic and IMHO better. I'll do that in a follow up, maybe. --- .../suites/feedback/captureFeedback/test.ts | 10 ++++----- .../hasSampling/test.ts | 10 ++++----- .../replay/slowClick/windowOpen/test.ts | 8 +++++++ .../tracing/trace-lifetime/navigation/test.ts | 4 ++-- .../trace-lifetime/pageload-meta/test.ts | 4 ++-- .../tracing/trace-lifetime/pageload/test.ts | 4 ++-- .../utils/fixtures.ts | 12 ++++++++++ .../utils/generatePlugin.ts | 22 +++++++++++++++++++ .../utils/helpers.ts | 4 +--- 9 files changed, 59 insertions(+), 19 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts index 01e56a9c1946..fede6aa2c0e2 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts @@ -1,9 +1,9 @@ import { expect } from '@playwright/test'; -import { sentryTest } from '../../../utils/fixtures'; +import { TEST_HOST, sentryTest } from '../../../utils/fixtures'; import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../utils/helpers'; -sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => { +sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => { if (shouldSkipFeedbackTest()) { sentryTest.skip(); } @@ -31,7 +31,7 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => { }); }); - const url = await getLocalTestPath({ testDir: __dirname }); + const url = await getLocalTestUrl({ testDir: __dirname }); await page.goto(url); await page.getByText('Report a Bug').click(); @@ -51,7 +51,7 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => { message: 'my example feedback', name: 'Jane Doe', source: 'widget', - url: expect.stringContaining('/dist/index.html'), + url: `${TEST_HOST}/index.html`, }, trace: { trace_id: expect.stringMatching(/\w{32}/), @@ -69,7 +69,7 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => { packages: expect.anything(), }, request: { - url: expect.stringContaining('/dist/index.html'), + url: `${TEST_HOST}/index.html`, headers: { 'User-Agent': expect.stringContaining(''), }, diff --git a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts index 3c46d1c79964..4457d64ccd9e 100644 --- a/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts +++ b/dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts @@ -1,6 +1,6 @@ import { expect } from '@playwright/test'; -import { sentryTest } from '../../../../utils/fixtures'; +import { TEST_HOST, sentryTest } from '../../../../utils/fixtures'; import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../../utils/helpers'; import { collectReplayRequests, @@ -9,7 +9,7 @@ import { waitForReplayRequest, } from '../../../../utils/replayHelpers'; -sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPath, page }) => { +sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestUrl, page }) => { if (shouldSkipFeedbackTest() || shouldSkipReplayTest()) { sentryTest.skip(); } @@ -39,7 +39,7 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat }); }); - const url = await getLocalTestPath({ testDir: __dirname }); + const url = await getLocalTestUrl({ testDir: __dirname }); await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]); @@ -87,7 +87,7 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat name: 'Jane Doe', replay_id: replayEvent.event_id, source: 'widget', - url: expect.stringContaining('/dist/index.html'), + url: `${TEST_HOST}/index.html`, }, trace: { trace_id: expect.stringMatching(/\w{32}/), @@ -105,7 +105,7 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat packages: expect.anything(), }, request: { - url: expect.stringContaining('/dist/index.html'), + url: `${TEST_HOST}/index.html`, headers: { 'User-Agent': expect.stringContaining(''), }, diff --git a/dev-packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts b/dev-packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts index bb2c91018d94..d68f27a5f45b 100644 --- a/dev-packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts @@ -16,6 +16,14 @@ sentryTest('window.open() is considered for slow click', async ({ getLocalTestUr }); }); + await page.route('http://example.com/', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}), + }); + }); + const url = await getLocalTestUrl({ testDir: __dirname }); await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts index 461fdc052f5e..b62923be0e9b 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts @@ -311,12 +311,12 @@ sentryTest( sentryTest( 'user feedback event after navigation has navigation traceId in headers', - async ({ getLocalTestPath, page }) => { + async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) { sentryTest.skip(); } - const url = await getLocalTestPath({ testDir: __dirname }); + const url = await getLocalTestUrl({ testDir: __dirname }); // ensure pageload transaction is finished await getFirstSentryEnvelopeRequest(page, url); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts index cfc0fcf8855d..6f94fe7b4fc1 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts @@ -297,12 +297,12 @@ sentryTest( }, ); -sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => { +sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) { sentryTest.skip(); } - const url = await getLocalTestPath({ testDir: __dirname }); + const url = await getLocalTestUrl({ testDir: __dirname }); const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); const pageloadTraceContext = pageloadEvent.contexts?.trace; diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts index 38665b37c5c3..2d0933002e7f 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts @@ -294,12 +294,12 @@ sentryTest( }, ); -sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => { +sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) { sentryTest.skip(); } - const url = await getLocalTestPath({ testDir: __dirname }); + const url = await getLocalTestUrl({ testDir: __dirname }); const pageloadEvent = await getFirstSentryEnvelopeRequest(page, url); const pageloadTraceContext = pageloadEvent.contexts?.trace; diff --git a/dev-packages/browser-integration-tests/utils/fixtures.ts b/dev-packages/browser-integration-tests/utils/fixtures.ts index 0b5943514a3e..f6c989a53778 100644 --- a/dev-packages/browser-integration-tests/utils/fixtures.ts +++ b/dev-packages/browser-integration-tests/utils/fixtures.ts @@ -3,6 +3,7 @@ import path from 'path'; /* eslint-disable no-empty-pattern */ import { test as base } from '@playwright/test'; +import { SDK_VERSION } from '@sentry/browser'; import { generatePage } from './generatePage'; export const TEST_HOST = 'http://sentry-test.io'; @@ -63,6 +64,17 @@ const sentryTest = base.extend({ return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue(); }); + + // Ensure feedback can be lazy loaded + await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback-modal.min.js`, route => { + const filePath = path.resolve(testDir, './dist/feedback-modal.bundle.js'); + return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue(); + }); + + await page.route(`https://browser.sentry-cdn.com/${SDK_VERSION}/feedback-screenshot.min.js`, route => { + const filePath = path.resolve(testDir, './dist/feedback-screenshot.bundle.js'); + return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue(); + }); } return pagePath; diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index 9b967748208e..7b46faa1fd7d 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -63,6 +63,10 @@ const BUNDLE_PATHS: Record> = { bundle: 'build/bundles/[INTEGRATION_NAME].js', bundle_min: 'build/bundles/[INTEGRATION_NAME].min.js', }, + feedback: { + bundle: 'build/bundles/[INTEGRATION_NAME].js', + bundle_min: 'build/bundles/[INTEGRATION_NAME].min.js', + }, wasm: { cjs: 'build/npm/cjs/index.js', esm: 'build/npm/esm/index.js', @@ -225,6 +229,24 @@ class SentryScenarioGenerationPlugin { .replace('_tracing', '') .replace('_feedback', ''); + // For feedback bundle, make sure to add modal & screenshot integrations + if (bundleKey.includes('_feedback')) { + ['feedback-modal', 'feedback-screenshot'].forEach(integration => { + const fileName = `${integration}.bundle.js`; + + // We add the files, but not a script tag - they are lazy-loaded + addStaticAssetSymlink( + this.localOutPath, + path.resolve( + PACKAGES_DIR, + 'feedback', + BUNDLE_PATHS['feedback'][integrationBundleKey].replace('[INTEGRATION_NAME]', integration), + ), + fileName, + ); + }); + } + this.requiredIntegrations.forEach(integration => { const fileName = `${integration}.bundle.js`; addStaticAssetSymlink( diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index c3795496ec64..0e888a708f00 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -248,10 +248,8 @@ export function shouldSkipTracingTest(): boolean { * @returns `true` if we should skip the feedback test */ export function shouldSkipFeedbackTest(): boolean { - // TODO(fn): For now we are skipping feedback tests in all bundles, until we have a proper way to test them. - // We need to make sure lazy loading works on release branches... const bundle = process.env.PW_BUNDLE as string | undefined; - return !!bundle && bundle.startsWith('bundle'); + return bundle != null && !bundle.includes('feedback') && !bundle.includes('esm') && !bundle.includes('cjs'); } /** From 5114e4471abcb2960cafd53ba3cda3d9a18d46aa Mon Sep 17 00:00:00 2001 From: Harshith Mohan Date: Mon, 6 May 2024 11:52:34 +0530 Subject: [PATCH 02/12] docs: fix typo in doc for `reactRouterV6BrowserTracingIntegration` (#11896) --- packages/react/src/reactrouterv6.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index ab67ba44bb5a..fb3f15aae3bf 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -59,8 +59,8 @@ interface ReactRouterOptions { } /** - * A browser tracing integration that uses React Router v3 to instrument navigations. - * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. + * A browser tracing integration that uses React Router v6 to instrument navigations. + * Expects `useEffect`, `useLocation`, `useNavigationType`, `createRoutesFromChildren` and `matchRoutes` to be passed as options. */ export function reactRouterV6BrowserTracingIntegration( options: Parameters[0] & ReactRouterOptions, From 0904ae94a319f57a2ec1bc8739296ec2b372b607 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 6 May 2024 10:18:11 +0200 Subject: [PATCH 03/12] build: Bump `@opentelemetry/instrumentation` and relax to `^` (#11905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As some instrumentation may need higher versions, which we don't want to block there. This shuld be treated more like core etc. Note that deduping this does not really work nicely, sadly, because this is still experimental (`0.x`) so `^0.50.0` will not match `0.51.0` 😞 --- packages/node/package.json | 2 +- packages/opentelemetry/package.json | 2 +- yarn.lock | 88 +++++++---------------------- 3 files changed, 23 insertions(+), 69 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index 2fb985bcbcb2..0c1df0bac414 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -56,7 +56,7 @@ "@opentelemetry/api": "^1.8.0", "@opentelemetry/context-async-hooks": "^1.23.0", "@opentelemetry/core": "^1.23.0", - "@opentelemetry/instrumentation": "0.48.0", + "@opentelemetry/instrumentation": "^0.51.0", "@opentelemetry/instrumentation-connect": "0.35.0", "@opentelemetry/instrumentation-express": "0.35.0", "@opentelemetry/instrumentation-fastify": "0.35.0", diff --git a/packages/opentelemetry/package.json b/packages/opentelemetry/package.json index bca44f020884..c4192a2ddc25 100644 --- a/packages/opentelemetry/package.json +++ b/packages/opentelemetry/package.json @@ -49,7 +49,7 @@ "peerDependencies": { "@opentelemetry/api": "^1.8.0", "@opentelemetry/core": "^1.23.0", - "@opentelemetry/instrumentation": "0.48.0", + "@opentelemetry/instrumentation": "^0.51.0", "@opentelemetry/sdk-trace-base": "^1.23.0", "@opentelemetry/semantic-conventions": "^1.23.0" }, diff --git a/yarn.lock b/yarn.lock index 1c764d790299..3e8835911cdf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6322,17 +6322,6 @@ "@types/pg" "8.6.1" "@types/pg-pool" "2.0.4" -"@opentelemetry/instrumentation@0.48.0", "@opentelemetry/instrumentation@^0.48.0": - version "0.48.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.48.0.tgz#a6dee936e973f1270c464657a55bb570807194aa" - integrity sha512-sjtZQB5PStIdCw5ovVTDGwnmQC+GGYArJNgIcydrDSqUTdYBnMrN9P4pwQZgS3vTGIp+TU1L8vMXGe51NVmIKQ== - dependencies: - "@types/shimmer" "^1.0.2" - import-in-the-middle "1.7.1" - require-in-the-middle "^7.1.1" - semver "^7.5.2" - shimmer "^1.2.1" - "@opentelemetry/instrumentation@0.50.0", "@opentelemetry/instrumentation@^0.50.0": version "0.50.0" resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.50.0.tgz#c558cfc64b84c11d304f31ccdf0de312ec60a2c9" @@ -6368,6 +6357,17 @@ semver "^7.5.2" shimmer "^1.2.1" +"@opentelemetry/instrumentation@^0.48.0": + version "0.48.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.48.0.tgz#a6dee936e973f1270c464657a55bb570807194aa" + integrity sha512-sjtZQB5PStIdCw5ovVTDGwnmQC+GGYArJNgIcydrDSqUTdYBnMrN9P4pwQZgS3vTGIp+TU1L8vMXGe51NVmIKQ== + dependencies: + "@types/shimmer" "^1.0.2" + import-in-the-middle "1.7.1" + require-in-the-middle "^7.1.1" + semver "^7.5.2" + shimmer "^1.2.1" + "@opentelemetry/propagation-utils@^0.30.8": version "0.30.8" resolved "https://registry.yarnpkg.com/@opentelemetry/propagation-utils/-/propagation-utils-0.30.8.tgz#5ae1468250e4f225be98b70aed994586248e2de3" @@ -8382,17 +8382,8 @@ dependencies: "@types/unist" "*" -"@types/history-4@npm:@types/history@4.7.8": - version "4.7.8" - resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" - integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== - -"@types/history-5@npm:@types/history@4.7.8": - version "4.7.8" - resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" - integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== - -"@types/history@*": +"@types/history-4@npm:@types/history@4.7.8", "@types/history-5@npm:@types/history@4.7.8", "@types/history@*": + name "@types/history-4" version "4.7.8" resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== @@ -8756,15 +8747,7 @@ "@types/history" "^3" "@types/react" "*" -"@types/react-router-4@npm:@types/react-router@5.1.14": - version "5.1.14" - resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" - integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== - dependencies: - "@types/history" "*" - "@types/react" "*" - -"@types/react-router-5@npm:@types/react-router@5.1.14": +"@types/react-router-4@npm:@types/react-router@5.1.14", "@types/react-router-5@npm:@types/react-router@5.1.14": version "5.1.14" resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== @@ -25793,7 +25776,8 @@ react-is@^18.0.0: dependencies: "@remix-run/router" "1.0.2" -"react-router-6@npm:react-router@6.3.0": +"react-router-6@npm:react-router@6.3.0", react-router@6.3.0: + name react-router-6 version "6.3.0" resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== @@ -25808,13 +25792,6 @@ react-router-dom@^6.2.2: history "^5.2.0" react-router "6.3.0" -react-router@6.3.0: - version "6.3.0" - resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" - integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== - dependencies: - history "^5.2.0" - react@^18.0.0: version "18.0.0" resolved "https://registry.yarnpkg.com/react/-/react-18.0.0.tgz#b468736d1f4a5891f38585ba8e8fb29f91c3cb96" @@ -28050,7 +28027,8 @@ string-template@~0.2.1: resolved "https://registry.yarnpkg.com/string-template/-/string-template-0.2.1.tgz#42932e598a352d01fc22ec3367d9d84eec6c9add" integrity sha1-QpMuWYo1LQH8IuwzZ9nYTuxsmt0= -"string-width-cjs@npm:string-width@^4.2.0": +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: + name string-width-cjs version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -28076,15 +28054,6 @@ string-width@^2.1.0: is-fullwidth-code-point "^2.0.0" strip-ansi "^4.0.0" -string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - string-width@^5.0.0, string-width@^5.0.1, string-width@^5.1.2: version "5.1.2" resolved "https://registry.yarnpkg.com/string-width/-/string-width-5.1.2.tgz#14f8daec6d81e7221d2a357e668cab73bdbca794" @@ -28180,14 +28149,7 @@ stringify-object@^3.2.1: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -30818,7 +30780,8 @@ workerpool@^6.4.0: resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.4.0.tgz#f8d5cfb45fde32fa3b7af72ad617c3369567a462" integrity sha512-i3KR1mQMNwY2wx20ozq2EjISGtQWDIfV56We+yGJ5yDs8jTwQiLLaqHlkBHITlCuJnYlVRmXegxFxZg7gqI++A== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: + name wrap-ansi-cjs version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -30836,15 +30799,6 @@ wrap-ansi@^6.0.1, wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From 782756b8070796abff8cd2356e5e22504572fff1 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 6 May 2024 11:43:41 +0200 Subject: [PATCH 04/12] fix(node): Ensure prisma integration creates valid DB spans (#11908) Closes https://github.com/getsentry/sentry-javascript/issues/11891 --- .../suites/tracing/prisma-orm/scenario.js | 7 +- .../suites/tracing/prisma-orm/test.ts | 72 ++++++++++--------- .../node/src/integrations/tracing/prisma.ts | 15 +++- 3 files changed, 55 insertions(+), 39 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/prisma-orm/scenario.js b/dev-packages/node-integration-tests/suites/tracing/prisma-orm/scenario.js index 7f291290bea2..82fbc044b973 100644 --- a/dev-packages/node-integration-tests/suites/tracing/prisma-orm/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/prisma-orm/scenario.js @@ -1,6 +1,3 @@ -const { randomBytes } = require('crypto'); -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ -const { PrismaClient } = require('@prisma/client'); const Sentry = require('@sentry/node'); const { loggingTransport } = require('@sentry-internal/node-integration-tests'); @@ -12,6 +9,9 @@ Sentry.init({ integrations: [Sentry.prismaIntegration()], }); +const { randomBytes } = require('crypto'); +const { PrismaClient } = require('@prisma/client'); + // Stop the process from exiting before the transaction is sent setInterval(() => {}, 1000); @@ -49,5 +49,4 @@ async function run() { ); } -// eslint-disable-next-line @typescript-eslint/no-floating-promises run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/prisma-orm/test.ts b/dev-packages/node-integration-tests/suites/tracing/prisma-orm/test.ts index 32bcdf168555..3c96c6d80779 100644 --- a/dev-packages/node-integration-tests/suites/tracing/prisma-orm/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/prisma-orm/test.ts @@ -7,100 +7,104 @@ conditionalTest({ min: 16 })('Prisma ORM Tests', () => { transaction: 'Test Transaction', spans: expect.arrayContaining([ expect.objectContaining({ - data: expect.objectContaining({ + data: { method: 'create', model: 'User', name: 'User.create', 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:client:operation', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:client:serialize', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:client:connect', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:engine', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'db.type': 'postgres', 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:engine:connection', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'db.statement': expect.stringContaining( 'INSERT INTO "public"."User" ("createdAt","email","name") VALUES ($1,$2,$3) RETURNING "public"."User"."id", "public"."User"."createdAt", "public"."User"."email", "public"."User"."name" /* traceparent', ), 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), - description: 'prisma:engine:db_query', + 'sentry.origin': 'auto.db.otel.prisma', + 'db.system': 'prisma', + 'sentry.op': 'db', + }, + description: expect.stringContaining( + 'INSERT INTO "public"."User" ("createdAt","email","name") VALUES ($1,$2,$3) RETURNING "public"."User"."id", "public"."User"."createdAt", "public"."User"."email", "public"."User"."name" /* traceparent', + ), status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:engine:serialize', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:engine:response_json_serialization', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { method: 'findMany', model: 'User', name: 'User.findMany', 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:client:operation', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:client:serialize', status: 'ok', }), expect.objectContaining({ - data: expect.objectContaining({ + data: { 'otel.kind': 'INTERNAL', - 'sentry.origin': 'manual', - }), + 'sentry.origin': 'auto.db.otel.prisma', + }, description: 'prisma:engine', status: 'ok', }), diff --git a/packages/node/src/integrations/tracing/prisma.ts b/packages/node/src/integrations/tracing/prisma.ts index 13e51065dce8..7652ea793530 100644 --- a/packages/node/src/integrations/tracing/prisma.ts +++ b/packages/node/src/integrations/tracing/prisma.ts @@ -1,6 +1,6 @@ // When importing CJS modules into an ESM module, we cannot import the named exports directly. import * as prismaInstrumentation from '@prisma/instrumentation'; -import { defineIntegration } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration, spanToJSON } from '@sentry/core'; import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; @@ -13,6 +13,19 @@ const _prismaIntegration = (() => { new prismaInstrumentation.PrismaInstrumentation({}), ); }, + + setup(client) { + client.on('spanStart', span => { + const spanJSON = spanToJSON(span); + if (spanJSON.description?.startsWith('prisma:')) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.prisma'); + } + + if (spanJSON.description === 'prisma:engine:db_query') { + span.setAttribute('db.system', 'prisma'); + } + }); + }, }; }) satisfies IntegrationFn; From e021c09d469973ecf224a1df89555b594e406aab Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 6 May 2024 11:43:51 +0200 Subject: [PATCH 05/12] feat(node): Support hapi v21 & fix E2E test (#11906) This PR bumps the hapi instrumentation to v0.38.0, which now supports hapi 21: https://github.com/open-telemetry/opentelemetry-js-contrib/releases/tag/instrumentation-hapi-v0.38.0 It also re-adds the hapi E2E test. Closes https://github.com/getsentry/sentry-javascript/issues/11411 --- .github/workflows/build.yml | 2 +- .../node-hapi-app/tests/server.test.ts | 208 ------------------ .../{node-hapi-app => node-hapi}/.gitignore | 0 .../{node-hapi-app => node-hapi}/.npmrc | 0 .../{node-hapi-app => node-hapi}/package.json | 2 +- .../playwright.config.ts | 0 .../{node-hapi-app => node-hapi}/src/app.js | 15 +- .../start-event-proxy.ts | 2 +- .../node-hapi/tests/errors.test.ts | 95 ++++++++ .../node-hapi/tests/transactions.test.ts | 100 +++++++++ .../tsconfig.json | 0 packages/node/package.json | 2 +- yarn.lock | 83 +------ 13 files changed, 216 insertions(+), 293 deletions(-) delete mode 100644 dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts rename dev-packages/e2e-tests/test-applications/{node-hapi-app => node-hapi}/.gitignore (100%) rename dev-packages/e2e-tests/test-applications/{node-hapi-app => node-hapi}/.npmrc (100%) rename dev-packages/e2e-tests/test-applications/{node-hapi-app => node-hapi}/package.json (96%) rename dev-packages/e2e-tests/test-applications/{node-hapi-app => node-hapi}/playwright.config.ts (100%) rename dev-packages/e2e-tests/test-applications/{node-hapi-app => node-hapi}/src/app.js (96%) rename dev-packages/e2e-tests/test-applications/{node-hapi-app => node-hapi}/start-event-proxy.ts (76%) create mode 100644 dev-packages/e2e-tests/test-applications/node-hapi/tests/errors.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-hapi/tests/transactions.test.ts rename dev-packages/e2e-tests/test-applications/{node-hapi-app => node-hapi}/tsconfig.json (100%) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c44609b1e2c8..99ca517863ac 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1017,7 +1017,7 @@ jobs: 'generic-ts3.8', 'node-fastify-app', # TODO(v8): Re-enable hapi tests - # 'node-hapi-app', + 'node-hapi', 'node-nestjs-app', 'node-exports-test-app', 'node-koa-app', diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts b/dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts deleted file mode 100644 index d0c8c5d19fb7..000000000000 --- a/dev-packages/e2e-tests/test-applications/node-hapi-app/tests/server.test.ts +++ /dev/null @@ -1,208 +0,0 @@ -import { expect, test } from '@playwright/test'; -import { waitForError, waitForTransaction } from '@sentry-internal/event-proxy-server'; -import axios, { AxiosError } from 'axios'; - -const authToken = process.env.E2E_TEST_AUTH_TOKEN; -const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; -const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; -const EVENT_POLLING_TIMEOUT = 90_000; - -test('Sends captured exception to Sentry', async ({ baseURL }) => { - const { data } = await axios.get(`${baseURL}/test-error`); - const { exceptionId } = data; - - const url = `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${exceptionId}/`; - - console.log(`Polling for error eventId: ${exceptionId}`); - - await expect - .poll( - async () => { - try { - const response = await axios.get(url, { headers: { Authorization: `Bearer ${authToken}` } }); - - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { timeout: EVENT_POLLING_TIMEOUT }, - ) - .toBe(200); -}); - -test('Sends thrown error to Sentry', async ({ baseURL }) => { - const errorEventPromise = waitForError('node-hapi-app', errorEvent => { - return errorEvent?.exception?.values?.[0]?.value === 'This is an error'; - }); - - try { - await axios.get(`${baseURL}/test-failure`); - } catch (e) {} - - const errorEvent = await errorEventPromise; - const errorEventId = errorEvent.event_id; - - await expect - .poll( - async () => { - try { - const response = await axios.get( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${errorEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); -}); - -test('Sends successful transactions to Sentry', async ({ baseURL }) => { - const pageloadTransactionEventPromise = waitForTransaction('node-hapi-app', transactionEvent => { - return ( - transactionEvent?.contexts?.trace?.op === 'hapi.request' && transactionEvent?.transaction === '/test-success' - ); - }); - - await axios.get(`${baseURL}/test-success`); - - const transactionEvent = await pageloadTransactionEventPromise; - const transactionEventId = transactionEvent.event_id; - - await expect - .poll( - async () => { - try { - const response = await axios.get( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); -}); - -test('sends error with parameterized transaction name', async ({ baseURL }) => { - const errorEventPromise = waitForError('node-hapi-app', errorEvent => { - return errorEvent?.exception?.values?.[0]?.value === 'This is an error with id 123'; - }); - - try { - await axios.get(`${baseURL}/test-error/123`); - } catch {} - - const errorEvent = await errorEventPromise; - - expect(errorEvent?.transaction).toBe('GET /test-error/{id}'); -}); - -test('Sends parameterized transactions to Sentry', async ({ baseURL }) => { - const pageloadTransactionEventPromise = waitForTransaction('node-hapi-app', transactionEvent => { - return ( - transactionEvent?.contexts?.trace?.op === 'hapi.request' && - transactionEvent?.transaction === '/test-param/{param}' - ); - }); - - await axios.get(`${baseURL}/test-param/123`); - - const transactionEvent = await pageloadTransactionEventPromise; - const transactionEventId = transactionEvent.event_id; - - expect(transactionEvent?.contexts?.trace?.op).toBe('hapi.request'); - expect(transactionEvent?.transaction).toBe('/test-param/{param}'); - - await expect - .poll( - async () => { - try { - const response = await axios.get( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - return response.status; - } catch (e) { - if (e instanceof AxiosError && e.response) { - if (e.response.status !== 404) { - throw e; - } else { - return e.response.status; - } - } else { - throw e; - } - } - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); -}); - -test('Sends sentry-trace and baggage as response headers', async ({ baseURL }) => { - const data = await axios.get(`${baseURL}/test-success`); - - expect(data.headers).toHaveProperty('sentry-trace'); - expect(data.headers).toHaveProperty('baggage'); -}); - -test('Continues trace and baggage from incoming headers', async ({ baseURL }) => { - const traceContent = '12312012123120121231201212312012-1121201211212012-0'; - const baggageContent = 'sentry-release=2.0.0,sentry-environment=myEnv'; - - await axios.get(`${baseURL}/test-success`); - - const data = await axios.get(`${baseURL}/test-success`, { - headers: { - 'sentry-trace': traceContent, - baggage: baggageContent, - }, - }); - - expect(data.headers).toHaveProperty('sentry-trace'); - expect(data.headers).toHaveProperty('baggage'); - - expect(data.headers['sentry-trace']).toContain('12312012123120121231201212312012-'); - expect(data.headers['baggage']).toContain(baggageContent); -}); diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/.gitignore b/dev-packages/e2e-tests/test-applications/node-hapi/.gitignore similarity index 100% rename from dev-packages/e2e-tests/test-applications/node-hapi-app/.gitignore rename to dev-packages/e2e-tests/test-applications/node-hapi/.gitignore diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/.npmrc b/dev-packages/e2e-tests/test-applications/node-hapi/.npmrc similarity index 100% rename from dev-packages/e2e-tests/test-applications/node-hapi-app/.npmrc rename to dev-packages/e2e-tests/test-applications/node-hapi/.npmrc diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/package.json b/dev-packages/e2e-tests/test-applications/node-hapi/package.json similarity index 96% rename from dev-packages/e2e-tests/test-applications/node-hapi-app/package.json rename to dev-packages/e2e-tests/test-applications/node-hapi/package.json index d63a3a6c457f..d0355205826c 100644 --- a/dev-packages/e2e-tests/test-applications/node-hapi-app/package.json +++ b/dev-packages/e2e-tests/test-applications/node-hapi/package.json @@ -1,5 +1,5 @@ { - "name": "node-hapi-app", + "name": "node-hapi", "version": "1.0.0", "private": true, "scripts": { diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/playwright.config.ts b/dev-packages/e2e-tests/test-applications/node-hapi/playwright.config.ts similarity index 100% rename from dev-packages/e2e-tests/test-applications/node-hapi-app/playwright.config.ts rename to dev-packages/e2e-tests/test-applications/node-hapi/playwright.config.ts diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/src/app.js b/dev-packages/e2e-tests/test-applications/node-hapi/src/app.js similarity index 96% rename from dev-packages/e2e-tests/test-applications/node-hapi-app/src/app.js rename to dev-packages/e2e-tests/test-applications/node-hapi/src/app.js index 6d6f959410dd..772847392e43 100644 --- a/dev-packages/e2e-tests/test-applications/node-hapi-app/src/app.js +++ b/dev-packages/e2e-tests/test-applications/node-hapi/src/app.js @@ -1,10 +1,4 @@ const Sentry = require('@sentry/node'); -const Hapi = require('@hapi/hapi'); - -const server = Hapi.server({ - port: 3030, - host: 'localhost', -}); Sentry.init({ environment: 'qa', // dynamic sampling bias to keep transactions @@ -15,6 +9,13 @@ Sentry.init({ tracesSampleRate: 1, }); +const Hapi = require('@hapi/hapi'); + +const server = Hapi.server({ + port: 3030, + host: 'localhost', +}); + const init = async () => { server.route({ method: 'GET', @@ -28,6 +29,8 @@ const init = async () => { method: 'GET', path: '/test-param/{param}', handler: function (request, h) { + Sentry.setTag(`param-${request.params.param}`, 'yes'); + return { paramWas: request.params.param }; }, }); diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/start-event-proxy.ts b/dev-packages/e2e-tests/test-applications/node-hapi/start-event-proxy.ts similarity index 76% rename from dev-packages/e2e-tests/test-applications/node-hapi-app/start-event-proxy.ts rename to dev-packages/e2e-tests/test-applications/node-hapi/start-event-proxy.ts index f1f5cf4b3316..25464165b311 100644 --- a/dev-packages/e2e-tests/test-applications/node-hapi-app/start-event-proxy.ts +++ b/dev-packages/e2e-tests/test-applications/node-hapi/start-event-proxy.ts @@ -2,5 +2,5 @@ import { startEventProxyServer } from '@sentry-internal/event-proxy-server'; startEventProxyServer({ port: 3031, - proxyServerName: 'node-hapi-app', + proxyServerName: 'node-hapi', }); diff --git a/dev-packages/e2e-tests/test-applications/node-hapi/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-hapi/tests/errors.test.ts new file mode 100644 index 000000000000..8782e8bb42a9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-hapi/tests/errors.test.ts @@ -0,0 +1,95 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/event-proxy-server'; +import axios, { AxiosError } from 'axios'; + +const authToken = process.env.E2E_TEST_AUTH_TOKEN; +const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; +const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; +const EVENT_POLLING_TIMEOUT = 90_000; + +test('Sends captured exception to Sentry', async ({ baseURL }) => { + const { data } = await axios.get(`${baseURL}/test-error`); + const { exceptionId } = data; + + const url = `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${exceptionId}/`; + + console.log(`Polling for error eventId: ${exceptionId}`); + + await expect + .poll( + async () => { + try { + const response = await axios.get(url, { headers: { Authorization: `Bearer ${authToken}` } }); + + return response.status; + } catch (e) { + if (e instanceof AxiosError && e.response) { + if (e.response.status !== 404) { + throw e; + } else { + return e.response.status; + } + } else { + throw e; + } + } + }, + { timeout: EVENT_POLLING_TIMEOUT }, + ) + .toBe(200); +}); + +test('Sends thrown error to Sentry', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-hapi', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'This is an error'; + }); + + try { + await axios.get(`${baseURL}/test-failure`); + } catch (e) {} + + const errorEvent = await errorEventPromise; + const errorEventId = errorEvent.event_id; + + await expect + .poll( + async () => { + try { + const response = await axios.get( + `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${errorEventId}/`, + { headers: { Authorization: `Bearer ${authToken}` } }, + ); + + return response.status; + } catch (e) { + if (e instanceof AxiosError && e.response) { + if (e.response.status !== 404) { + throw e; + } else { + return e.response.status; + } + } else { + throw e; + } + } + }, + { + timeout: EVENT_POLLING_TIMEOUT, + }, + ) + .toBe(200); +}); + +test('sends error with parameterized transaction name', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-hapi', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'This is an error with id 123'; + }); + + try { + await axios.get(`${baseURL}/test-error/123`); + } catch {} + + const errorEvent = await errorEventPromise; + + expect(errorEvent?.transaction).toBe('GET /test-error/{id}'); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-hapi/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-hapi/tests/transactions.test.ts new file mode 100644 index 000000000000..2c07128b6561 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-hapi/tests/transactions.test.ts @@ -0,0 +1,100 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/event-proxy-server'; +import axios from 'axios'; + +test('Sends successful transaction', async ({ baseURL }) => { + const pageloadTransactionEventPromise = waitForTransaction('node-hapi', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /test-success' + ); + }); + + await axios.get(`${baseURL}/test-success`); + + const transactionEvent = await pageloadTransactionEventPromise; + const transactionEventId = transactionEvent.event_id; + + expect(transactionEvent.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/test-success', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/test-success', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/test-success', + 'http.user_agent': 'axios/1.6.7', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': expect.any(String), + 'net.host.port': expect.any(Number), + 'net.peer.ip': expect.any(String), + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-success', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + trace_id: expect.any(String), + origin: 'auto.http.otel.http', + }); + + expect(transactionEvent).toEqual( + expect.objectContaining({ + transaction: 'GET /test-success', + type: 'transaction', + transaction_info: { + source: 'route', + }, + }), + ); +}); + +test('Sends parameterized transactions to Sentry', async ({ baseURL }) => { + const pageloadTransactionEventPromise = waitForTransaction('node-hapi', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /test-param/{param}' + ); + }); + + await axios.get(`${baseURL}/test-param/123`); + + const transactionEvent = await pageloadTransactionEventPromise; + const transactionEventId = transactionEvent.event_id; + + expect(transactionEvent?.contexts?.trace?.op).toBe('http.server'); + expect(transactionEvent?.contexts?.trace?.data?.['http.route']).toBe('/test-param/{param}'); + expect(transactionEvent?.transaction).toBe('GET /test-param/{param}'); +}); + +test('Isolates requests', async ({ baseURL }) => { + const transaction1Promise = waitForTransaction('node-hapi', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.contexts?.trace?.data?.['http.target'] === '/test-param/888' + ); + }); + const transaction2Promise = waitForTransaction('node-hapi', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.contexts?.trace?.data?.['http.target'] === '/test-param/999' + ); + }); + + await Promise.all([axios.get(`${baseURL}/test-param/888`), axios.get(`${baseURL}/test-param/999`)]); + + const transaction1 = await transaction1Promise; + const transaction2 = await transaction2Promise; + + expect(transaction1.tags).toEqual({ 'param-888': 'yes' }); + expect(transaction2.tags).toEqual({ 'param-999': 'yes' }); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-hapi-app/tsconfig.json b/dev-packages/e2e-tests/test-applications/node-hapi/tsconfig.json similarity index 100% rename from dev-packages/e2e-tests/test-applications/node-hapi-app/tsconfig.json rename to dev-packages/e2e-tests/test-applications/node-hapi/tsconfig.json diff --git a/packages/node/package.json b/packages/node/package.json index 0c1df0bac414..18c447165676 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -61,7 +61,7 @@ "@opentelemetry/instrumentation-express": "0.35.0", "@opentelemetry/instrumentation-fastify": "0.35.0", "@opentelemetry/instrumentation-graphql": "0.39.0", - "@opentelemetry/instrumentation-hapi": "0.36.0", + "@opentelemetry/instrumentation-hapi": "0.38.0", "@opentelemetry/instrumentation-http": "0.51.0", "@opentelemetry/instrumentation-ioredis": "0.40.0", "@opentelemetry/instrumentation-koa": "0.39.0", diff --git a/yarn.lock b/yarn.lock index 3e8835911cdf..d8b5738feba5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4825,7 +4825,7 @@ dependencies: "@hapi/hoek" "9.x.x" -"@hapi/boom@^9.0.0", "@hapi/boom@^9.1.0": +"@hapi/boom@^9.1.0": version "9.1.4" resolved "https://registry.yarnpkg.com/@hapi/boom/-/boom-9.1.4.tgz#1f9dad367c6a7da9f8def24b4a986fc5a7bd9db6" integrity sha512-Ls1oH8jaN1vNsqcaHVYJrKmgMcKsC1wcp8bujvXrHaAqD2iDYq3HoOwsxwo09Cuda5R5nC0o0IxlrlTuvPuzSw== @@ -4933,7 +4933,7 @@ resolved "https://registry.yarnpkg.com/@hapi/hoek/-/hoek-9.3.0.tgz#8368869dcb735be2e7f5cb7647de78e167a251fb" integrity sha512-/c6rf4UJlmHlC9b5BaNvzAcFv7HZ2QHaV0D4/HNlBdvFnvQq8RI4kYdhyPCl7Xj+oWvTWQ8ujhqS53LIgAe6KQ== -"@hapi/iron@6.x.x", "@hapi/iron@^6.0.0": +"@hapi/iron@6.x.x": version "6.0.0" resolved "https://registry.yarnpkg.com/@hapi/iron/-/iron-6.0.0.tgz#ca3f9136cda655bdd6028de0045da0de3d14436f" integrity sha512-zvGvWDufiTGpTJPG1Y/McN8UqWBu0k/xs/7l++HVU535NLHXsHhy54cfEMdW7EjwKfbBfM9Xy25FmTiobb7Hvw== @@ -4971,7 +4971,7 @@ "@hapi/hoek" "9.x.x" "@hapi/nigel" "4.x.x" -"@hapi/podium@4.x.x", "@hapi/podium@^4.1.1", "@hapi/podium@^4.1.3": +"@hapi/podium@4.x.x", "@hapi/podium@^4.1.1": version "4.1.3" resolved "https://registry.yarnpkg.com/@hapi/podium/-/podium-4.1.3.tgz#91e20838fc2b5437f511d664aabebbb393578a26" integrity sha512-ljsKGQzLkFqnQxE7qeanvgGj4dejnciErYd30dbrYzUOF/FyS/DOF97qcrT3bhoVwCYmxa6PEMhxfCPlnUcD2g== @@ -6227,15 +6227,14 @@ dependencies: "@opentelemetry/instrumentation" "^0.50.0" -"@opentelemetry/instrumentation-hapi@0.36.0": - version "0.36.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-hapi/-/instrumentation-hapi-0.36.0.tgz#ce3ed250ee0006f4335bda4cc06bc2d531307d4c" - integrity sha512-0NnXuRF89Windosn+iUpq5Fn/Foy8PMJxtLfe6CakDJIUGPj/g1+erz5irqSOc0P5mM3rEqKC/cYCoSIMKo/eA== +"@opentelemetry/instrumentation-hapi@0.38.0": + version "0.38.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-hapi/-/instrumentation-hapi-0.38.0.tgz#2913263248c190638aaed921b1f272af0b830a2b" + integrity sha512-ZcOqEuwuutTDYIjhDIStix22ECblG/i9pHje23QGs4Q4YS4RMaZ5hKCoQJxW88Z4K7T53rQkdISmoXFKDV8xMg== dependencies: "@opentelemetry/core" "^1.8.0" - "@opentelemetry/instrumentation" "^0.50.0" + "@opentelemetry/instrumentation" "^0.51.0" "@opentelemetry/semantic-conventions" "^1.0.0" - "@types/hapi__hapi" "20.0.13" "@opentelemetry/instrumentation-http@0.51.0": version "0.51.0" @@ -7082,23 +7081,6 @@ unplugin "1.0.1" uuid "^9.0.0" -"@sideway/address@^4.1.3": - version "4.1.4" - resolved "https://registry.yarnpkg.com/@sideway/address/-/address-4.1.4.tgz#03dccebc6ea47fdc226f7d3d1ad512955d4783f0" - integrity sha512-7vwq+rOHVWjyXxVlR76Agnvhy8I9rpzjosTESvmhNeXOXdZZB15Fl+TI9x1SiHZH5Jv2wTGduSxFDIaq0m3DUw== - dependencies: - "@hapi/hoek" "^9.0.0" - -"@sideway/formula@^3.0.1": - version "3.0.1" - resolved "https://registry.yarnpkg.com/@sideway/formula/-/formula-3.0.1.tgz#80fcbcbaf7ce031e0ef2dd29b1bfc7c3f583611f" - integrity sha512-/poHZJJVjx3L+zVD6g9KgHfYnb443oi7wLu/XKojDviHy6HOEOA6z1Trk5aR1dGcmPenJEgb2sK2I80LeS3MIg== - -"@sideway/pinpoint@^2.0.0": - version "2.0.0" - resolved "https://registry.yarnpkg.com/@sideway/pinpoint/-/pinpoint-2.0.0.tgz#cff8ffadc372ad29fd3f78277aeb29e632cc70df" - integrity sha512-RNiOoTPkptFtSVzQevY/yWtZwf/RxyVnPy/OcA9HBM3MlGDnBEYL5B41H0MTn0Uec8Hi+2qUtTfG2WWZBmMejQ== - "@sigstore/protobuf-specs@^0.1.0": version "0.1.0" resolved "https://registry.yarnpkg.com/@sigstore/protobuf-specs/-/protobuf-specs-0.1.0.tgz#957cb64ea2f5ce527cc9cf02a096baeb0d2b99b4" @@ -8335,39 +8317,6 @@ dependencies: "@types/node" "*" -"@types/hapi__catbox@*": - version "10.2.5" - resolved "https://registry.yarnpkg.com/@types/hapi__catbox/-/hapi__catbox-10.2.5.tgz#4eb5e2fbb966acd2a3c2e86f935b7de95aceb7dd" - integrity sha512-vomIMP6dUDSbiasbPglH5LJvnnl8jFmRTjPgPl4l9Vi1L9fto3VXJQZtl8LzyIQUUoocyT5bmvWeYWsVgxAHQg== - -"@types/hapi__hapi@20.0.13": - version "20.0.13" - resolved "https://registry.yarnpkg.com/@types/hapi__hapi/-/hapi__hapi-20.0.13.tgz#ea8ce83c192f6e8106f6e76e40f795e7e36d0615" - integrity sha512-LP4IPfhIO5ZPVOrJo7H8c8Slc0WYTFAUNQX1U0LBPKyXioXhH5H2TawIgxKujIyOhbwoBbpvOsBf6o5+ToJIrQ== - dependencies: - "@hapi/boom" "^9.0.0" - "@hapi/iron" "^6.0.0" - "@hapi/podium" "^4.1.3" - "@types/hapi__catbox" "*" - "@types/hapi__mimos" "*" - "@types/hapi__shot" "*" - "@types/node" "*" - joi "^17.3.0" - -"@types/hapi__mimos@*": - version "4.1.4" - resolved "https://registry.yarnpkg.com/@types/hapi__mimos/-/hapi__mimos-4.1.4.tgz#4f8a1c58345fc468553708d3cb508724aa081bd9" - integrity sha512-i9hvJpFYTT/qzB5xKWvDYaSXrIiNqi4ephi+5Lo6+DoQdwqPXQgmVVOZR+s3MBiHoFqsCZCX9TmVWG3HczmTEQ== - dependencies: - "@types/mime-db" "*" - -"@types/hapi__shot@*": - version "4.1.4" - resolved "https://registry.yarnpkg.com/@types/hapi__shot/-/hapi__shot-4.1.4.tgz#17d418537bc180ac042361d9a331892a31a61583" - integrity sha512-AhEirOGy2ajtdV9WE/JqPkGeCH8lpgcSEQxn0ZNJkTvxkOv5DfZEXGit3l5J9P1VoQFAAHGNLVGWI5IWCiQf9A== - dependencies: - "@types/node" "*" - "@types/hast@^2.0.0": version "2.3.6" resolved "https://registry.yarnpkg.com/@types/hast/-/hast-2.3.6.tgz#bb8b05602112a26d22868acb70c4b20984ec7086" @@ -8550,11 +8499,6 @@ dependencies: "@types/unist" "*" -"@types/mime-db@*": - version "1.43.3" - resolved "https://registry.yarnpkg.com/@types/mime-db/-/mime-db-1.43.3.tgz#f7bec64a9a62ddded3371e82862c0516539710e8" - integrity sha512-vg0UsF1p1Qi/8iCARoie7F/Ng92zo7tQlL+sqE15GonkKVl55n/0vB6jSbrYTgDO0PSx9pKfGG1iZg9gJum3wA== - "@types/mime@*": version "3.0.4" resolved "https://registry.yarnpkg.com/@types/mime/-/mime-3.0.4.tgz#2198ac274de6017b44d941e00261d5bc6a0e0a45" @@ -20127,17 +20071,6 @@ jiti@^1.21.0: resolved "https://registry.yarnpkg.com/jiti/-/jiti-1.21.0.tgz#7c97f8fe045724e136a397f7340475244156105d" integrity sha512-gFqAIbuKyyso/3G2qhiO2OM6shY6EPP/R0+mkDbyspxKazh8BXDC5FiFsUjlczgdNz/vfra0da2y+aHrusLG/Q== -joi@^17.3.0: - version "17.11.0" - resolved "https://registry.yarnpkg.com/joi/-/joi-17.11.0.tgz#aa9da753578ec7720e6f0ca2c7046996ed04fc1a" - integrity sha512-NgB+lZLNoqISVy1rZocE9PZI36bL/77ie924Ri43yEvi9GUUMPeyVIr8KdFTMUlby1p0PBYMk9spIxEUQYqrJQ== - dependencies: - "@hapi/hoek" "^9.0.0" - "@hapi/topo" "^5.0.0" - "@sideway/address" "^4.1.3" - "@sideway/formula" "^3.0.1" - "@sideway/pinpoint" "^2.0.0" - js-cleanup@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/js-cleanup/-/js-cleanup-1.2.0.tgz#8dbc65954b1d38b255f1e8cf02cd17b3f7a053f9" From 2c161f6caef9dee777e5de95bc17883858388526 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 6 May 2024 14:12:35 +0200 Subject: [PATCH 06/12] fix(node): Include loader hook files in package.json (#11911) --- packages/astro/package.json | 3 ++- packages/aws-serverless/package.json | 3 ++- packages/google-cloud-serverless/package.json | 3 ++- packages/node/package.json | 3 ++- packages/remix/package.json | 3 ++- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/astro/package.json b/packages/astro/package.json index 697e00d621e5..c9584605901f 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -22,7 +22,8 @@ "esm", "types", "types-ts3.8", - "register.mjs" + "import-hook.mjs", + "loader-hook.mjs" ], "main": "build/cjs/index.client.js", "module": "build/esm/index.server.js", diff --git a/packages/aws-serverless/package.json b/packages/aws-serverless/package.json index 87f608e7be54..8f2a82359084 100644 --- a/packages/aws-serverless/package.json +++ b/packages/aws-serverless/package.json @@ -13,7 +13,8 @@ "cjs", "types", "types-ts3.8", - "register.mjs" + "import-hook.mjs", + "loader-hook.mjs" ], "main": "build/npm/cjs/index.js", "types": "build/npm/types/index.d.ts", diff --git a/packages/google-cloud-serverless/package.json b/packages/google-cloud-serverless/package.json index 732f65ca4428..831caf672465 100644 --- a/packages/google-cloud-serverless/package.json +++ b/packages/google-cloud-serverless/package.json @@ -14,7 +14,8 @@ "esm", "types", "types-ts3.8", - "register.mjs" + "import-hook.mjs", + "loader-hook.mjs" ], "main": "build/cjs/index.js", "types": "build/types/index.d.ts", diff --git a/packages/node/package.json b/packages/node/package.json index 18c447165676..ba9e8f704e3d 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -14,7 +14,8 @@ "esm", "types", "types-ts3.8", - "register.mjs" + "import-hook.mjs", + "loader-hook.mjs" ], "main": "build/cjs/index.js", "module": "build/esm/index.js", diff --git a/packages/remix/package.json b/packages/remix/package.json index 013b2b1db887..cca07be1fd05 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -18,7 +18,8 @@ "types", "types-ts3.8", "scripts", - "register.mjs" + "import-hook.mjs", + "loader-hook.mjs" ], "main": "build/cjs/index.server.js", "module": "build/esm/index.server.js", From 78804ae8c92ed614b483c902a32ca82859e5826c Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Mon, 6 May 2024 06:27:37 -0700 Subject: [PATCH 07/12] feat(feedback): Iterate on css for better scrolling & resizing when browser is small (#11893) --- packages/feedback/src/constants/theme.ts | 26 ++-- .../feedback/src/core/components/Actor.css.ts | 43 +++---- .../feedback/src/core/components/Actor.ts | 11 +- .../src/core/components/FeedbackIcon.ts | 3 +- .../feedback/src/core/createMainStyles.ts | 22 +++- packages/feedback/src/core/integration.ts | 6 +- .../src/modal/components/Dialog.css.ts | 117 ++++++++++------- .../{DialogContainer.tsx => Dialog.tsx} | 13 +- .../src/modal/components/DialogContent.tsx | 21 ---- .../src/modal/components/DialogHeader.tsx | 7 +- .../feedback/src/modal/components/Form.tsx | 16 ++- .../feedback/src/modal/components/Icon.ts | 49 -------- .../src/modal/components/SentryLogo.ts | 31 +---- .../src/modal/components/SuccessIcon.ts | 2 +- packages/feedback/src/modal/integration.tsx | 9 +- .../components/ScreenshotInput.css.ts | 5 - packages/types/src/feedback/theme.ts | 119 ++++++++++++------ 17 files changed, 244 insertions(+), 256 deletions(-) rename packages/feedback/src/modal/components/{DialogContainer.tsx => Dialog.tsx} (83%) delete mode 100644 packages/feedback/src/modal/components/DialogContent.tsx delete mode 100644 packages/feedback/src/modal/components/Icon.ts diff --git a/packages/feedback/src/constants/theme.ts b/packages/feedback/src/constants/theme.ts index 127cc4616062..26398b74d1b2 100644 --- a/packages/feedback/src/constants/theme.ts +++ b/packages/feedback/src/constants/theme.ts @@ -6,37 +6,39 @@ export const LIGHT_THEME = { fontFamily: "system-ui, 'Helvetica Neue', Arial, sans-serif", fontSize: '14px', - background: LIGHT_BACKGROUND, - backgroundHover: '#f6f6f7', foreground: '#2b2233', + background: LIGHT_BACKGROUND, + success: '#268d75', + error: '#df3338', + + zIndex: 100000, border: '1.5px solid rgba(41, 35, 47, 0.13)', - borderRadius: '25px', boxShadow: '0px 4px 24px 0px rgba(43, 34, 51, 0.12)', - success: '#268d75', - error: '#df3338', + backgroundHover: '#f6f6f7', + borderRadius: '25px', + formBorderRadius: '20px', + formContentBorderRadius: '6px', + + submitForeground: LIGHT_BACKGROUND, submitBackground: 'rgba(88, 74, 192, 1)', + submitForegroundHover: LIGHT_BACKGROUND, submitBackgroundHover: SUBMIT_COLOR, submitBorder: SUBMIT_COLOR, submitOutlineFocus: '#29232f', - submitForeground: LIGHT_BACKGROUND, - submitForegroundHover: LIGHT_BACKGROUND, + cancelForeground: 'var(--foreground)', cancelBackground: 'transparent', + cancelForegroundHover: 'var(--foreground)', cancelBackgroundHover: 'var(--background-hover)', cancelBorder: 'var(--border)', cancelOutlineFocus: 'var(--input-outline-focus)', - cancelForeground: 'var(--foreground)', - cancelForegroundHover: 'var(--foreground)', inputBackground: INHERIT, inputForeground: INHERIT, inputBorder: 'var(--border)', inputOutlineFocus: SUBMIT_COLOR, - - formBorderRadius: '20px', - formContentBorderRadius: '6px', }; export const DEFAULT_THEME = { diff --git a/packages/feedback/src/core/components/Actor.css.ts b/packages/feedback/src/core/components/Actor.css.ts index bfc135a7c9a0..ea80921b3744 100644 --- a/packages/feedback/src/core/components/Actor.css.ts +++ b/packages/feedback/src/core/components/Actor.css.ts @@ -8,34 +8,36 @@ export function createActorStyles(): HTMLStyleElement { style.textContent = ` .widget__actor { position: fixed; - left: var(--left); - right: var(--right); - bottom: var(--bottom); - top: var(--top); z-index: var(--z-index); - - line-height: 16px; + margin: 0; + inset: var(--actor-inset); display: flex; align-items: center; gap: 8px; + padding: 16px; - - border-radius: var(--border-radius); - cursor: pointer; font-family: inherit; font-size: var(--font-size); font-weight: 600; - padding: 16px; + line-height: 16px; text-decoration: none; - z-index: 9000; - color: var(--foreground); background-color: var(--background); + border-radius: var(--border-radius); border: var(--border); box-shadow: var(--box-shadow); + color: var(--foreground); + cursor: pointer; opacity: 1; - transition: opacity 0.1s ease-in-out; + transition: transform 0.2s ease-in-out; + transform: translate(0, 0) scale(1); +} +.widget__actor[aria-hidden="true"] { + opacity: 0; + pointer-events: none; + visibility: hidden; + transform: translate(0, 16px) scale(0.98); } .widget__actor:hover { @@ -47,24 +49,11 @@ export function createActorStyles(): HTMLStyleElement { height: 16px; } -.widget__actor--hidden { - opacity: 0; - pointer-events: none; - visibility: hidden; -} - -.widget__actor__text { -} - @media (max-width: 600px) { - .widget__actor__text { + .widget__actor span { display: none; } } - -.feedback-icon path { - fill: var(--foreground); -} `; return style; diff --git a/packages/feedback/src/core/components/Actor.ts b/packages/feedback/src/core/components/Actor.ts index fb49d5d1b7ee..ac62fad4957e 100644 --- a/packages/feedback/src/core/components/Actor.ts +++ b/packages/feedback/src/core/components/Actor.ts @@ -13,6 +13,10 @@ export interface ActorComponent { appendToDom: () => void; removeFromDom: () => void; + + show: () => void; + + hide: () => void; } /** @@ -27,7 +31,6 @@ export function Actor({ buttonLabel, shadow }: ActorProps): ActorComponent { el.appendChild(FeedbackIcon()); if (buttonLabel) { const label = DOCUMENT.createElement('span'); - label.className = 'widget__actor__text'; label.appendChild(DOCUMENT.createTextNode(buttonLabel)); el.appendChild(label); } @@ -44,5 +47,11 @@ export function Actor({ buttonLabel, shadow }: ActorProps): ActorComponent { shadow.removeChild(el); shadow.removeChild(style); }, + show(): void { + el.ariaHidden = 'false'; + }, + hide(): void { + el.ariaHidden = 'true'; + }, }; } diff --git a/packages/feedback/src/core/components/FeedbackIcon.ts b/packages/feedback/src/core/components/FeedbackIcon.ts index a6145ebf8900..d4ca23d9349a 100644 --- a/packages/feedback/src/core/components/FeedbackIcon.ts +++ b/packages/feedback/src/core/components/FeedbackIcon.ts @@ -11,11 +11,10 @@ export function FeedbackIcon(): SVGElement { const createElementNS = (tagName: K): SVGElementTagNameMap[K] => WINDOW.document.createElementNS(XMLNS, tagName); const svg = setAttributesNS(createElementNS('svg'), { - class: 'feedback-icon', width: `${SIZE}`, height: `${SIZE}`, viewBox: `0 0 ${SIZE} ${SIZE}`, - fill: 'none', + fill: 'var(--foreground)', }); const g = setAttributesNS(createElementNS('g'), { diff --git a/packages/feedback/src/core/createMainStyles.ts b/packages/feedback/src/core/createMainStyles.ts index ca5f138cb960..92763cacdd93 100644 --- a/packages/feedback/src/core/createMainStyles.ts +++ b/packages/feedback/src/core/createMainStyles.ts @@ -46,17 +46,29 @@ export function createMainStyles( const style = DOCUMENT.createElement('style'); style.textContent = ` :host { - --bottom: 1rem; - --right: 1rem; - --top: auto; - --left: auto; - --z-index: 100000; + --z-index: ${themes.themeLight.zIndex}; --font-family: ${themes.themeLight.fontFamily}; --font-size: ${themes.themeLight.fontSize}; font-family: var(--font-family); font-size: var(--font-size); + --page-margin: 16px; + --actor-inset: auto var(--page-margin) var(--page-margin) auto; + + --dialog-inset: auto var(--page-margin) var(--page-margin) auto; + --dialog-padding: 24px; + + .brand-link path { + fill: ${colorScheme === 'dark' ? '#fff' : '#362d59'}; + } + @media (prefers-color-scheme: dark) + { + path: { + fill: '#fff'; + } + } + ${getThemedCssVariables(colorScheme === 'dark' ? themes.themeDark : themes.themeLight)} } diff --git a/packages/feedback/src/core/integration.ts b/packages/feedback/src/core/integration.ts index fe5eceee4508..32cce8b83fe7 100644 --- a/packages/feedback/src/core/integration.ts +++ b/packages/feedback/src/core/integration.ts @@ -254,13 +254,13 @@ export const buildFeedbackIntegration = ({ const mergedOptions = mergeOptions(_options, { ...optionOverrides, onFormOpen() { - actor.removeFromDom(); + actor.hide(); }, onFormClose() { - actor.appendToDom(); + actor.show(); }, onFormSubmitted() { - actor.appendToDom(); + actor.show(); }, }); _attachTo(actor.el, mergedOptions); diff --git a/packages/feedback/src/modal/components/Dialog.css.ts b/packages/feedback/src/modal/components/Dialog.css.ts index 037f64310120..4d01aae84309 100644 --- a/packages/feedback/src/modal/components/Dialog.css.ts +++ b/packages/feedback/src/modal/components/Dialog.css.ts @@ -1,24 +1,23 @@ import { DOCUMENT } from '../../constants'; -/** - * Creates