Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(feedback): Add captureFeedback method #11428

Merged
merged 11 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => {
source: 'widget',
url: expect.stringContaining('/dist/index.html'),
},
trace: {
trace_id: expect.stringMatching(/\w{32}/),
span_id: expect.stringMatching(/\w{16}/),
},
},
level: 'info',
timestamp: expect.any(Number),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPat
source: 'widget',
url: expect.stringContaining('/dist/index.html'),
},
trace: {
trace_id: expect.stringMatching(/\w{32}/),
span_id: expect.stringMatching(/\w{16}/),
},
},
level: 'info',
timestamp: expect.any(Number),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ window.Sentry = Sentry;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [Sentry.browserTracingIntegration()],
integrations: [Sentry.browserTracingIntegration(), Sentry.feedbackIntegration()],
tracePropagationTargets: ['http://example.com'],
tracesSampleRate: 1,
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect } from '@playwright/test';
import type { Event, SpanEnvelope } from '@sentry/types';
import { sentryTest } from '../../../../utils/fixtures';
import type { EventAndTraceHeader } from '../../../../utils/helpers';
import { shouldSkipFeedbackTest } from '../../../../utils/helpers';
import {
eventAndTraceHeaderRequestParser,
getFirstSentryEnvelopeRequest,
Expand Down Expand Up @@ -134,7 +135,7 @@ sentryTest('error during navigation has new navigation traceId', async ({ getLoc

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

// ensure navigation transaction is finished
// ensure pageload transaction is finished
await getFirstSentryEnvelopeRequest<Event>(page, url);

const envelopeRequestsPromise = getMultipleSentryEnvelopeRequests<EventAndTraceHeader>(
Expand Down Expand Up @@ -202,7 +203,7 @@ sentryTest(
});
});

// ensure navigation transaction is finished
// ensure pageload transaction is finished
await getFirstSentryEnvelopeRequest<Event>(page, url);

const [navigationEvent, navigationTraceHeader] = await getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
Expand Down Expand Up @@ -276,7 +277,7 @@ sentryTest(
});
});

// ensure navigation transaction is finished
// ensure pageload transaction is finished
await getFirstSentryEnvelopeRequest<Event>(page, url);

const navigationEventPromise = getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
Expand Down Expand Up @@ -456,3 +457,47 @@ sentryTest(
);
},
);

sentryTest(
'user feedback event after navigation has navigation traceId in headers',
async ({ getLocalTestPath, page }) => {
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
sentryTest.skip();
}

const url = await getLocalTestPath({ testDir: __dirname });

// ensure pageload transaction is finished
await getFirstSentryEnvelopeRequest<Event>(page, url);

const navigationEvent = await getFirstSentryEnvelopeRequest<Event>(page, `${url}#foo`);

const navigationTraceContext = navigationEvent.contexts?.trace;
expect(navigationTraceContext).toMatchObject({
op: 'navigation',
trace_id: expect.stringMatching(/^[0-9a-f]{32}$/),
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
});
expect(navigationTraceContext).not.toHaveProperty('parent_span_id');

const feedbackEventPromise = getFirstSentryEnvelopeRequest<Event>(page);

await page.getByText('Report a Bug').click();
expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1);
await page.locator('[name="name"]').fill('Jane Doe');
await page.locator('[name="email"]').fill('janedoe@example.org');
await page.locator('[name="message"]').fill('my example feedback');
await page.locator('[data-sentry-feedback] .btn--primary').click();

const feedbackEvent = await feedbackEventPromise;

expect(feedbackEvent.type).toEqual('feedback');

const feedbackTraceContext = feedbackEvent.contexts?.trace;

expect(feedbackTraceContext).toMatchObject({
trace_id: navigationTraceContext?.trace_id,
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
});
},
);
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { expect } from '@playwright/test';
import type { SpanEnvelope } from '@sentry/types';
import type { Event, SpanEnvelope } from '@sentry/types';
import { sentryTest } from '../../../../utils/fixtures';
import type { EventAndTraceHeader } from '../../../../utils/helpers';
import { shouldSkipFeedbackTest } from '../../../../utils/helpers';
import {
eventAndTraceHeaderRequestParser,
getFirstSentryEnvelopeRequest,
Expand Down Expand Up @@ -429,3 +430,41 @@ sentryTest(
expect(headers['baggage']).toBe(META_TAG_BAGGAGE);
},
);

sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => {
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
sentryTest.skip();
}

const url = await getLocalTestPath({ testDir: __dirname });

const pageloadEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
const pageloadTraceContext = pageloadEvent.contexts?.trace;

expect(pageloadTraceContext).toMatchObject({
op: 'pageload',
trace_id: META_TAG_TRACE_ID,
parent_span_id: META_TAG_PARENT_SPAN_ID,
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
});

const feedbackEventPromise = getFirstSentryEnvelopeRequest<Event>(page);

await page.getByText('Report a Bug').click();
expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1);
await page.locator('[name="name"]').fill('Jane Doe');
await page.locator('[name="email"]').fill('janedoe@example.org');
await page.locator('[name="message"]').fill('my example feedback');
await page.locator('[data-sentry-feedback] .btn--primary').click();

const feedbackEvent = await feedbackEventPromise;
const feedbackTraceContext = feedbackEvent.contexts?.trace;

expect(feedbackEvent.type).toEqual('feedback');

expect(feedbackTraceContext).toMatchObject({
trace_id: META_TAG_TRACE_ID,
parent_span_id: META_TAG_PARENT_SPAN_ID,
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { expect } from '@playwright/test';
import type { SpanEnvelope } from '@sentry/types';
import type { Event, SpanEnvelope } from '@sentry/types';
import { sentryTest } from '../../../../utils/fixtures';
import type { EventAndTraceHeader } from '../../../../utils/helpers';
import { shouldSkipFeedbackTest } from '../../../../utils/helpers';
import {
eventAndTraceHeaderRequestParser,
getFirstSentryEnvelopeRequest,
Expand Down Expand Up @@ -431,3 +432,41 @@ sentryTest(
);
},
);

sentryTest('user feedback event after pageload has pageload traceId in headers', async ({ getLocalTestPath, page }) => {
if (shouldSkipTracingTest() || shouldSkipFeedbackTest()) {
sentryTest.skip();
}

const url = await getLocalTestPath({ testDir: __dirname });

const pageloadEvent = await getFirstSentryEnvelopeRequest<Event>(page, url);
const pageloadTraceContext = pageloadEvent.contexts?.trace;

expect(pageloadTraceContext).toMatchObject({
op: 'pageload',
trace_id: expect.stringMatching(/^[0-9a-f]{32}$/),
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
});
expect(pageloadTraceContext).not.toHaveProperty('parent_span_id');

const feedbackEventPromise = getFirstSentryEnvelopeRequest<Event>(page);

await page.getByText('Report a Bug').click();
expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1);
await page.locator('[name="name"]').fill('Jane Doe');
await page.locator('[name="email"]').fill('janedoe@example.org');
await page.locator('[name="message"]').fill('my example feedback');
await page.locator('[data-sentry-feedback] .btn--primary').click();

const feedbackEvent = await feedbackEventPromise;

expect(feedbackEvent.type).toEqual('feedback');

const feedbackTraceContext = feedbackEvent.contexts?.trace;

expect(feedbackTraceContext).toMatchObject({
trace_id: pageloadTraceContext?.trace_id,
span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
});
});
1 change: 1 addition & 0 deletions packages/astro/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export {
captureEvent,
captureMessage,
captureCheckIn,
captureFeedback,
withMonitor,
createTransport,
// eslint-disable-next-line deprecation/deprecation
Expand Down
1 change: 1 addition & 0 deletions packages/aws-serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export {
captureEvent,
captureMessage,
captureCheckIn,
captureFeedback,
startSession,
captureSession,
endSession,
Expand Down
2 changes: 2 additions & 0 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {

/**
* Sends user feedback to Sentry.
*
* @deprecated Use `captureFeedback` instead.
*/
public captureUserFeedback(feedback: UserFeedback): void {
if (!this._isEnabled()) {
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export {
init,
onLoad,
showReportDialog,
// eslint-disable-next-line deprecation/deprecation
captureUserFeedback,
} from './sdk';

Expand Down
2 changes: 2 additions & 0 deletions packages/browser/src/index.bundle.feedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export {
feedbackAsyncIntegration as feedbackIntegration,
replayIntegrationShim as replayIntegration,
};

export { captureFeedback } from '@sentry/core';
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export {
withActiveSpan,
getSpanDescendants,
setMeasurement,
captureFeedback,
} from '@sentry/core';

export {
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export {
extraErrorDataIntegration,
rewriteFramesIntegration,
sessionTimingIntegration,
captureFeedback,
} from '@sentry/core';

export {
Expand Down
3 changes: 3 additions & 0 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,13 @@ function startSessionTracking(): void {

/**
* Captures user feedback and sends it to Sentry.
*
* @deprecated Use `captureFeedback` instead.
*/
export function captureUserFeedback(feedback: UserFeedback): void {
const client = getClient<BrowserClient>();
if (client) {
// eslint-disable-next-line deprecation/deprecation
client.captureUserFeedback(feedback);
}
}
1 change: 1 addition & 0 deletions packages/bun/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export {
captureEvent,
captureMessage,
captureCheckIn,
captureFeedback,
startSession,
captureSession,
endSession,
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
addItemToEnvelope,
checkOrSetAlreadyCaught,
createAttachmentEnvelopeItem,
dropUndefinedKeys,
isParameterizedString,
isPlainObject,
isPrimitive,
Expand Down Expand Up @@ -663,11 +664,11 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
if (!trace && propagationContext) {
const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext;
evt.contexts = {
trace: {
trace: dropUndefinedKeys({
trace_id,
span_id: spanId,
parent_span_id: parentSpanId,
},
}),
...evt.contexts,
};

Expand Down
31 changes: 0 additions & 31 deletions packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import type {
Attachment,
AttachmentItem,
DsnComponents,
DynamicSamplingContext,
Event,
Expand All @@ -15,7 +13,6 @@ import type {
SpanEnvelope,
} from '@sentry/types';
import {
createAttachmentEnvelopeItem,
createEnvelope,
createEventEnvelopeHeaders,
dsnToString,
Expand Down Expand Up @@ -95,34 +92,6 @@ export function createEventEnvelope(
return createEnvelope<EventEnvelope>(envelopeHeaders, [eventItem]);
}

/**
* Create an Envelope from an event.
*/
export function createAttachmentEnvelope(
event: Event,
attachments: Attachment[],
dsn?: DsnComponents,
metadata?: SdkMetadata,
tunnel?: string,
): EventEnvelope {
const sdkInfo = getSdkMetadataForEnvelopeHeader(metadata);
enhanceEventWithSdkInfo(event, metadata && metadata.sdk);

const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn);

// Prevent this data (which, if it exists, was used in earlier steps in the processing pipeline) from being sent to
// sentry. (Note: Our use of this property comes and goes with whatever we might be debugging, whatever hacks we may
// 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
// of this `delete`, lest we miss putting it back in the next time the property is in use.)
delete event.sdkProcessingMetadata;

const attachmentItems: AttachmentItem[] = [];
for (const attachment of attachments || []) {
attachmentItems.push(createAttachmentEnvelopeItem(attachment));
}
return createEnvelope<EventEnvelope>(envelopeHeaders, attachmentItems);
}

/**
* Create envelope from Span item.
*/
Expand Down
Loading
Loading