Skip to content

feat: Send transactions in envelopes #2553

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

Merged
merged 6 commits into from
May 11, 2020
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

- [core] feat: Send transactions in envelopes (#2553)

## 5.15.5

- [browser/node] Add missing `BreadcrumbHint` and `EventHint` types exports (#2545)
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export class Breadcrumbs implements Integration {
const client = getCurrentHub().getClient<BrowserClient>();
const dsn = client && client.getDsn();
if (this._options.sentry && dsn) {
const filterUrl = new API(dsn).getStoreEndpoint();
const filterUrl = new API(dsn).getBaseApiEndpoint();
// if Sentry key appears in URL, don't capture it as a request
// but rather as our own 'sentry' type breadcrumb
if (
Expand Down
9 changes: 7 additions & 2 deletions packages/browser/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@ import { PromiseBuffer, SentryError } from '@sentry/utils';
/** Base Transport class implementation */
export abstract class BaseTransport implements Transport {
/**
* @inheritDoc
* @deprecated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends on whether a single transport will eventually have to deal with multiple endpoints like now, or whether we'll have independent transports, each having a fixed target endpoint URL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stick with one transport with multiple methods for getting url. Although I'm not sure if we should deprecate this attribute just yet, as we are not using envelopes for errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should deprecate this attribute just yet, as we are not using envelopes for errors.

The transport now always use the _api attribute to fetch the appropriate endpoint. url is only accessed in tests and possibly user code (e.g. custom transport).

*/
public url: string;

/** Helper to get Sentry API endpoints. */
protected readonly _api: API;

/** A simple buffer holding all requests. */
protected readonly _buffer: PromiseBuffer<Response> = new PromiseBuffer(30);

public constructor(public options: TransportOptions) {
this.url = new API(this.options.dsn).getStoreEndpointWithUrlEncodedAuth();
this._api = new API(this.options.dsn);
// tslint:disable-next-line:deprecation
this.url = this._api.getStoreEndpointWithUrlEncodedAuth();
}

/**
Expand Down
11 changes: 7 additions & 4 deletions packages/browser/src/transports/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { eventToSentryRequest } from '@sentry/core';
import { Event, Response, Status } from '@sentry/types';
import { getGlobalObject, logger, parseRetryAfterHeader, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';

Expand All @@ -22,8 +23,10 @@ export class FetchTransport extends BaseTransport {
});
}

const defaultOptions: RequestInit = {
body: JSON.stringify(event),
const sentryReq = eventToSentryRequest(event, this._api);

const options: RequestInit = {
body: sentryReq.body,
method: 'POST',
// Despite all stars in the sky saying that Edge supports old draft syntax, aka 'never', 'always', 'origin' and 'default
// https://caniuse.com/#feat=referrer-policy
Expand All @@ -33,13 +36,13 @@ export class FetchTransport extends BaseTransport {
};

if (this.options.headers !== undefined) {
defaultOptions.headers = this.options.headers;
options.headers = this.options.headers;
}

return this._buffer.add(
new SyncPromise<Response>((resolve, reject) => {
global
.fetch(this.url, defaultOptions)
.fetch(sentryReq.url, options)
.then(response => {
const status = Status.fromHttpCode(response.status);

Expand Down
7 changes: 5 additions & 2 deletions packages/browser/src/transports/xhr.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { eventToSentryRequest } from '@sentry/core';
import { Event, Response, Status } from '@sentry/types';
import { logger, parseRetryAfterHeader, SyncPromise } from '@sentry/utils';

Expand All @@ -20,6 +21,8 @@ export class XHRTransport extends BaseTransport {
});
}

const sentryReq = eventToSentryRequest(event, this._api);

return this._buffer.add(
new SyncPromise<Response>((resolve, reject) => {
const request = new XMLHttpRequest();
Expand All @@ -45,13 +48,13 @@ export class XHRTransport extends BaseTransport {
reject(request);
};

request.open('POST', this.url);
request.open('POST', sentryReq.url);
for (const header in this.options.headers) {
if (this.options.headers.hasOwnProperty(header)) {
request.setRequestHeader(header, this.options.headers[header]);
}
}
request.send(JSON.stringify(event));
request.send(sentryReq.body);
}),
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/browser/test/unit/transports/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('BaseTransport', () => {

it('has correct endpoint url', () => {
const transport = new SimpleTransport({ dsn: testDsn });
// tslint:disable-next-line:deprecation
expect(transport.url).equal('https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7');
});
});
1 change: 1 addition & 0 deletions packages/browser/test/unit/transports/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('FetchTransport', () => {
});

it('inherits composeEndpointUrl() implementation', () => {
// tslint:disable-next-line:deprecation
expect(transport.url).equal(transportUrl);
});

Expand Down
1 change: 1 addition & 0 deletions packages/browser/test/unit/transports/xhr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('XHRTransport', () => {
});

it('inherits composeEndpointUrl() implementation', () => {
// tslint:disable-next-line:deprecation
expect(transport.url).equal(transportUrl);
});

Expand Down
68 changes: 51 additions & 17 deletions packages/core/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,59 @@ export class API {
return this._dsnObject;
}

/** Returns a string with auth headers in the url to the store endpoint. */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the documentation of getStoreEndpoint! Fixed, it does not include auth.

/** Returns the prefix to construct Sentry ingestion API endpoints. */
public getBaseApiEndpoint(): string {
const dsn = this._dsnObject;
const protocol = dsn.protocol ? `${dsn.protocol}:` : '';
const port = dsn.port ? `:${dsn.port}` : '';
return `${protocol}//${dsn.host}${port}${dsn.path ? `/${dsn.path}` : ''}/api/`;
}

/** Returns the store endpoint URL. */
public getStoreEndpoint(): string {
return `${this._getBaseUrl()}${this.getStoreEndpointPath()}`;
return this._getIngestEndpoint('store');
}

/** Returns the envelope endpoint URL. */
private _getEnvelopeEndpoint(): string {
return this._getIngestEndpoint('envelope');
}

/** Returns the ingest API endpoint for target. */
private _getIngestEndpoint(target: 'store' | 'envelope'): string {
const base = this.getBaseApiEndpoint();
const dsn = this._dsnObject;
return `${base}${dsn.projectId}/${target}/`;
}

/** Returns the store endpoint with auth added in url encoded. */
/**
* Returns the store endpoint URL with auth in the query string.
*
* Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests.
*/
public getStoreEndpointWithUrlEncodedAuth(): string {
return `${this.getStoreEndpoint()}?${this._encodedAuth()}`;
}

/**
* Returns the envelope endpoint URL with auth in the query string.
*
* Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests.
*/
public getEnvelopeEndpointWithUrlEncodedAuth(): string {
return `${this._getEnvelopeEndpoint()}?${this._encodedAuth()}`;
}

/** Returns a URL-encoded string with auth config suitable for a query string. */
private _encodedAuth(): string {
const dsn = this._dsnObject;
const auth = {
sentry_key: dsn.user, // sentry_key is currently used in tracing integration to identify internal sentry requests
// We send only the minimum set of required information. See
// https://github.com/getsentry/sentry-javascript/issues/2572.
sentry_key: dsn.user,
sentry_version: SENTRY_API_VERSION,
};
// Auth is intentionally sent as part of query string (NOT as custom HTTP header)
// to avoid preflight CORS requests
return `${this.getStoreEndpoint()}?${urlEncode(auth)}`;
}

/** Returns the base path of the url including the port. */
private _getBaseUrl(): string {
const dsn = this._dsnObject;
const protocol = dsn.protocol ? `${dsn.protocol}:` : '';
const port = dsn.port ? `:${dsn.port}` : '';
return `${protocol}//${dsn.host}${port}`;
return urlEncode(auth);
}

/** Returns only the path component for the store endpoint. */
Expand All @@ -48,7 +78,11 @@ export class API {
return `${dsn.path ? `/${dsn.path}` : ''}/api/${dsn.projectId}/store/`;
}

/** Returns an object that can be used in request headers. */
/**
* Returns an object that can be used in request headers.
*
* @deprecated in favor of `getStoreEndpointWithUrlEncodedAuth` and `getEnvelopeEndpointWithUrlEncodedAuth`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getRequestHeaders as-is is problematic now because the headers depend on the event type.

*/
public getRequestHeaders(clientName: string, clientVersion: string): { [key: string]: string } {
const dsn = this._dsnObject;
const header = [`Sentry sentry_version=${SENTRY_API_VERSION}`];
Expand All @@ -71,7 +105,7 @@ export class API {
} = {},
): string {
const dsn = this._dsnObject;
const endpoint = `${this._getBaseUrl()}${dsn.path ? `/${dsn.path}` : ''}/api/embed/error-page/`;
const endpoint = `${this.getBaseApiEndpoint()}embed/error-page/`;

const encodedOptions = [];
encodedOptions.push(`dsn=${dsn.toString()}`);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export { addGlobalEventProcessor, getCurrentHub, getHubFromCarrier, Hub, Scope }
export { API } from './api';
export { BaseClient } from './baseclient';
export { BackendClass, BaseBackend } from './basebackend';
export { eventToSentryRequest } from './request';
export { initAndBind, ClientClass } from './sdk';
export { NoopTransport } from './transports/noop';

Expand Down
61 changes: 61 additions & 0 deletions packages/core/src/request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Event } from '@sentry/types';

import { API } from './api';

/** A generic client request. */
interface SentryRequest {
body: string;
url: string;
// headers would contain auth & content-type headers for @sentry/node, but
// since @sentry/browser avoids custom headers to prevent CORS preflight
// requests, we can use the same approach for @sentry/browser and @sentry/node
// for simplicity -- no headers involved.
// headers: { [key: string]: string };
}

/** Creates a SentryRequest from an event. */
export function eventToSentryRequest(event: Event, api: API): SentryRequest {
const useEnvelope = event.type === 'transaction';

const req: SentryRequest = {
body: JSON.stringify(event),
url: useEnvelope ? api.getEnvelopeEndpointWithUrlEncodedAuth() : api.getStoreEndpointWithUrlEncodedAuth(),
};

// https://develop.sentry.dev/sdk/envelopes/

// Since we don't need to manipulate envelopes nor store them, there is no
// exported concept of an Envelope with operations including serialization and
// deserialization. Instead, we only implement a minimal subset of the spec to
// serialize events inline here.
if (useEnvelope) {
const envelopeHeaders = JSON.stringify({
event_id: event.event_id,
sent_at: new Date().toISOString(),
});
const itemHeaders = JSON.stringify({
type: event.type,
// The content-type is assumed to be 'application/json' and not part of
// the current spec for transaction items, so we don't bloat the request
// body with it.
//
// content_type: 'application/json',
//
// The length is optional. It must be the number of bytes in req.Body
// encoded as UTF-8. Since the server can figure this out and would
// otherwise refuse events that report the length incorrectly, we decided
// not to send the length to avoid problems related to reporting the wrong
// size and to reduce request body size.
//
// length: new TextEncoder().encode(req.body).length,
});
// The trailing newline is optional. We intentionally don't send it to avoid
// sending unnecessary bytes.
//
// const envelope = `${envelopeHeaders}\n${itemHeaders}\n${req.body}\n`;
const envelope = `${envelopeHeaders}\n${itemHeaders}\n${req.body}`;
req.body = envelope;
}

return req;
}
2 changes: 2 additions & 0 deletions packages/core/test/lib/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ describe('API', () => {
});

test('getRequestHeaders', () => {
// tslint:disable-next-line:deprecation
expect(new API(dsnPublic).getRequestHeaders('a', '1.0')).toMatchObject({
'Content-Type': 'application/json',
'X-Sentry-Auth': expect.stringMatching(/^Sentry sentry_version=\d, sentry_client=a\/1\.0, sentry_key=abc$/),
});

// tslint:disable-next-line:deprecation
expect(new API(legacyDsn).getRequestHeaders('a', '1.0')).toMatchObject({
'Content-Type': 'application/json',
'X-Sentry-Auth': expect.stringMatching(
Expand Down
Loading