Skip to content

Commit 56197af

Browse files
authored
feat(feedback): Add captureFeedback method (#11428)
This PR adds a new `captureFeedback` method which is exported from all SDKs. This method can be used to capture a (new) user feedback from any package. We follow the same semantics as for other capture methods like `captureException()` or `captureMessage()`: The method returns a string, which is the event id. We do not wait for sending to be successful or not, we just try to send it and return. You can both set an `associatedEventId` (which replaces the event_id of the "old" `captureUserFeedback`), and also pass attachments to send along (which for now are sent as a separate envelope). For usage in the modal UI, there is still `sendFeedback` which continues to return a promise that resolves with the event ID, or rejects with a meaningful error message if sending fails. This also deprecates `captureUserFeedback()`, which is only exported in browser. We cannot remove this yet because `captureFeedback` will only work on newer self-hosted instances, so not all users can easily update. We can/should remove this in v9. Includes #11626 Fixes 49946
1 parent 85c754a commit 56197af

File tree

32 files changed

+1094
-174
lines changed

32 files changed

+1094
-174
lines changed

dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => {
5353
source: 'widget',
5454
url: expect.stringContaining('/dist/index.html'),
5555
},
56+
trace: {
57+
trace_id: expect.stringMatching(/\w{32}/),
58+
span_id: expect.stringMatching(/\w{16}/),
59+
},
5660
},
5761
level: 'info',
5862
timestamp: expect.any(Number),

dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat
8989
source: 'widget',
9090
url: expect.stringContaining('/dist/index.html'),
9191
},
92+
trace: {
93+
trace_id: expect.stringMatching(/\w{32}/),
94+
span_id: expect.stringMatching(/\w{16}/),
95+
},
9296
},
9397
level: 'info',
9498
timestamp: expect.any(Number),

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/init.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ window.Sentry = Sentry;
44

55
Sentry.init({
66
dsn: 'https://[email protected]/1337',
7-
integrations: [Sentry.browserTracingIntegration()],
7+
integrations: [Sentry.browserTracingIntegration(), Sentry.feedbackIntegration()],
88
tracePropagationTargets: ['http://example.com'],
99
tracesSampleRate: 1,
1010
});

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { expect } from '@playwright/test';
22
import type { Event, SpanEnvelope } from '@sentry/types';
33
import { sentryTest } from '../../../../utils/fixtures';
44
import type { EventAndTraceHeader } from '../../../../utils/helpers';
5+
import { shouldSkipFeedbackTest } from '../../../../utils/helpers';
56
import {
67
eventAndTraceHeaderRequestParser,
78
getFirstSentryEnvelopeRequest,
@@ -134,7 +135,7 @@ sentryTest('error during navigation has new navigation traceId', async ({ getLoc
134135

135136
const url = await getLocalTestUrl({ testDir: __dirname });
136137

137-
// ensure navigation transaction is finished
138+
// ensure pageload transaction is finished
138139
await getFirstSentryEnvelopeRequest<Event>(page, url);
139140

140141
const envelopeRequestsPromise = getMultipleSentryEnvelopeRequests<EventAndTraceHeader>(
@@ -202,7 +203,7 @@ sentryTest(
202203
});
203204
});
204205

205-
// ensure navigation transaction is finished
206+
// ensure pageload transaction is finished
206207
await getFirstSentryEnvelopeRequest<Event>(page, url);
207208

208209
const [navigationEvent, navigationTraceHeader] = await getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
@@ -276,7 +277,7 @@ sentryTest(
276277
});
277278
});
278279

279-
// ensure navigation transaction is finished
280+
// ensure pageload transaction is finished
280281
await getFirstSentryEnvelopeRequest<Event>(page, url);
281282

282283
const navigationEventPromise = getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
@@ -456,3 +457,47 @@ sentryTest(
456457
);
457458
},
458459
);
460+
461+
sentryTest(
462+
'user feedback event after navigation has navigation traceId in headers',
463+
async ({ getLocalTestPath, page }) => {
464+
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
465+
sentryTest.skip();
466+
}
467+
468+
const url = await getLocalTestPath({ testDir: __dirname });
469+
470+
// ensure pageload transaction is finished
471+
await getFirstSentryEnvelopeRequest<Event>(page, url);
472+
473+
const navigationEvent = await getFirstSentryEnvelopeRequest<Event>(page, `${url}#foo`);
474+
475+
const navigationTraceContext = navigationEvent.contexts?.trace;
476+
expect(navigationTraceContext).toMatchObject({
477+
op: 'navigation',
478+
trace_id: expect.stringMatching(/^[0-9a-f]{32}$/),
479+
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
480+
});
481+
expect(navigationTraceContext).not.toHaveProperty('parent_span_id');
482+
483+
const feedbackEventPromise = getFirstSentryEnvelopeRequest<Event>(page);
484+
485+
await page.getByText('Report a Bug').click();
486+
expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1);
487+
await page.locator('[name="name"]').fill('Jane Doe');
488+
await page.locator('[name="email"]').fill('[email protected]');
489+
await page.locator('[name="message"]').fill('my example feedback');
490+
await page.locator('[data-sentry-feedback] .btn--primary').click();
491+
492+
const feedbackEvent = await feedbackEventPromise;
493+
494+
expect(feedbackEvent.type).toEqual('feedback');
495+
496+
const feedbackTraceContext = feedbackEvent.contexts?.trace;
497+
498+
expect(feedbackTraceContext).toMatchObject({
499+
trace_id: navigationTraceContext?.trace_id,
500+
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
501+
});
502+
},
503+
);

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { expect } from '@playwright/test';
2-
import type { SpanEnvelope } from '@sentry/types';
2+
import type { Event, SpanEnvelope } from '@sentry/types';
33
import { sentryTest } from '../../../../utils/fixtures';
44
import type { EventAndTraceHeader } from '../../../../utils/helpers';
5+
import { shouldSkipFeedbackTest } from '../../../../utils/helpers';
56
import {
67
eventAndTraceHeaderRequestParser,
78
getFirstSentryEnvelopeRequest,
@@ -429,3 +430,41 @@ sentryTest(
429430
expect(headers['baggage']).toBe(META_TAG_BAGGAGE);
430431
},
431432
);
433+
434+
sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => {
435+
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
436+
sentryTest.skip();
437+
}
438+
439+
const url = await getLocalTestPath({ testDir: __dirname });
440+
441+
const pageloadEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
442+
const pageloadTraceContext = pageloadEvent.contexts?.trace;
443+
444+
expect(pageloadTraceContext).toMatchObject({
445+
op: 'pageload',
446+
trace_id: META_TAG_TRACE_ID,
447+
parent_span_id: META_TAG_PARENT_SPAN_ID,
448+
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
449+
});
450+
451+
const feedbackEventPromise = getFirstSentryEnvelopeRequest<Event>(page);
452+
453+
await page.getByText('Report a Bug').click();
454+
expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1);
455+
await page.locator('[name="name"]').fill('Jane Doe');
456+
await page.locator('[name="email"]').fill('[email protected]');
457+
await page.locator('[name="message"]').fill('my example feedback');
458+
await page.locator('[data-sentry-feedback] .btn--primary').click();
459+
460+
const feedbackEvent = await feedbackEventPromise;
461+
const feedbackTraceContext = feedbackEvent.contexts?.trace;
462+
463+
expect(feedbackEvent.type).toEqual('feedback');
464+
465+
expect(feedbackTraceContext).toMatchObject({
466+
trace_id: META_TAG_TRACE_ID,
467+
parent_span_id: META_TAG_PARENT_SPAN_ID,
468+
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
469+
});
470+
});

dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { expect } from '@playwright/test';
2-
import type { SpanEnvelope } from '@sentry/types';
2+
import type { Event, SpanEnvelope } from '@sentry/types';
33
import { sentryTest } from '../../../../utils/fixtures';
44
import type { EventAndTraceHeader } from '../../../../utils/helpers';
5+
import { shouldSkipFeedbackTest } from '../../../../utils/helpers';
56
import {
67
eventAndTraceHeaderRequestParser,
78
getFirstSentryEnvelopeRequest,
@@ -431,3 +432,41 @@ sentryTest(
431432
);
432433
},
433434
);
435+
436+
sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => {
437+
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
438+
sentryTest.skip();
439+
}
440+
441+
const url = await getLocalTestPath({ testDir: __dirname });
442+
443+
const pageloadEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
444+
const pageloadTraceContext = pageloadEvent.contexts?.trace;
445+
446+
expect(pageloadTraceContext).toMatchObject({
447+
op: 'pageload',
448+
trace_id: expect.stringMatching(/^[0-9a-f]{32}$/),
449+
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
450+
});
451+
expect(pageloadTraceContext).not.toHaveProperty('parent_span_id');
452+
453+
const feedbackEventPromise = getFirstSentryEnvelopeRequest<Event>(page);
454+
455+
await page.getByText('Report a Bug').click();
456+
expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1);
457+
await page.locator('[name="name"]').fill('Jane Doe');
458+
await page.locator('[name="email"]').fill('[email protected]');
459+
await page.locator('[name="message"]').fill('my example feedback');
460+
await page.locator('[data-sentry-feedback] .btn--primary').click();
461+
462+
const feedbackEvent = await feedbackEventPromise;
463+
464+
expect(feedbackEvent.type).toEqual('feedback');
465+
466+
const feedbackTraceContext = feedbackEvent.contexts?.trace;
467+
468+
expect(feedbackTraceContext).toMatchObject({
469+
trace_id: pageloadTraceContext?.trace_id,
470+
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
471+
});
472+
});

packages/astro/src/index.server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export {
1414
captureEvent,
1515
captureMessage,
1616
captureCheckIn,
17+
captureFeedback,
1718
withMonitor,
1819
createTransport,
1920
// eslint-disable-next-line deprecation/deprecation

packages/aws-serverless/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export {
66
captureEvent,
77
captureMessage,
88
captureCheckIn,
9+
captureFeedback,
910
startSession,
1011
captureSession,
1112
endSession,

packages/browser/src/client.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
9191

9292
/**
9393
* Sends user feedback to Sentry.
94+
*
95+
* @deprecated Use `captureFeedback` instead.
9496
*/
9597
public captureUserFeedback(feedback: UserFeedback): void {
9698
if (!this._isEnabled()) {

packages/browser/src/exports.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ export {
8888
init,
8989
onLoad,
9090
showReportDialog,
91+
// eslint-disable-next-line deprecation/deprecation
9192
captureUserFeedback,
9293
} from './sdk';
9394

packages/browser/src/index.bundle.feedback.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,5 @@ export {
1111
feedbackAsyncIntegration as feedbackIntegration,
1212
replayIntegrationShim as replayIntegration,
1313
};
14+
15+
export { captureFeedback } from '@sentry/core';

packages/browser/src/index.bundle.tracing.replay.feedback.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export {
1313
withActiveSpan,
1414
getSpanDescendants,
1515
setMeasurement,
16+
captureFeedback,
1617
} from '@sentry/core';
1718

1819
export {

packages/browser/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export {
1010
extraErrorDataIntegration,
1111
rewriteFramesIntegration,
1212
sessionTimingIntegration,
13+
captureFeedback,
1314
} from '@sentry/core';
1415

1516
export {

packages/browser/src/sdk.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,13 @@ function startSessionTracking(): void {
295295

296296
/**
297297
* Captures user feedback and sends it to Sentry.
298+
*
299+
* @deprecated Use `captureFeedback` instead.
298300
*/
299301
export function captureUserFeedback(feedback: UserFeedback): void {
300302
const client = getClient<BrowserClient>();
301303
if (client) {
304+
// eslint-disable-next-line deprecation/deprecation
302305
client.captureUserFeedback(feedback);
303306
}
304307
}

packages/bun/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export {
2626
captureEvent,
2727
captureMessage,
2828
captureCheckIn,
29+
captureFeedback,
2930
startSession,
3031
captureSession,
3132
endSession,

packages/core/src/baseclient.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
addItemToEnvelope,
3636
checkOrSetAlreadyCaught,
3737
createAttachmentEnvelopeItem,
38+
dropUndefinedKeys,
3839
isParameterizedString,
3940
isPlainObject,
4041
isPrimitive,
@@ -663,11 +664,11 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
663664
if (!trace && propagationContext) {
664665
const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext;
665666
evt.contexts = {
666-
trace: {
667+
trace: dropUndefinedKeys({
667668
trace_id,
668669
span_id: spanId,
669670
parent_span_id: parentSpanId,
670-
},
671+
}),
671672
...evt.contexts,
672673
};
673674

packages/core/src/envelope.ts

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
import type {
2-
Attachment,
3-
AttachmentItem,
42
DsnComponents,
53
DynamicSamplingContext,
64
Event,
@@ -15,7 +13,6 @@ import type {
1513
SpanEnvelope,
1614
} from '@sentry/types';
1715
import {
18-
createAttachmentEnvelopeItem,
1916
createEnvelope,
2017
createEventEnvelopeHeaders,
2118
dsnToString,
@@ -95,34 +92,6 @@ export function createEventEnvelope(
9592
return createEnvelope<EventEnvelope>(envelopeHeaders, [eventItem]);
9693
}
9794

98-
/**
99-
* Create an Envelope from an event.
100-
*/
101-
export function createAttachmentEnvelope(
102-
event: Event,
103-
attachments: Attachment[],
104-
dsn?: DsnComponents,
105-
metadata?: SdkMetadata,
106-
tunnel?: string,
107-
): EventEnvelope {
108-
const sdkInfo = getSdkMetadataForEnvelopeHeader(metadata);
109-
enhanceEventWithSdkInfo(event, metadata && metadata.sdk);
110-
111-
const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn);
112-
113-
// Prevent this data (which, if it exists, was used in earlier steps in the processing pipeline) from being sent to
114-
// sentry. (Note: Our use of this property comes and goes with whatever we might be debugging, whatever hacks we may
115-
// have temporarily added, etc. Even if we don't happen to be using it at some point in the future, let's not get rid
116-
// of this `delete`, lest we miss putting it back in the next time the property is in use.)
117-
delete event.sdkProcessingMetadata;
118-
119-
const attachmentItems: AttachmentItem[] = [];
120-
for (const attachment of attachments || []) {
121-
attachmentItems.push(createAttachmentEnvelopeItem(attachment));
122-
}
123-
return createEnvelope<EventEnvelope>(envelopeHeaders, attachmentItems);
124-
}
125-
12695
/**
12796
* Create envelope from Span item.
12897
*/

0 commit comments

Comments
 (0)