Skip to content

feat(tracing): Add initial tracestate header handling #3909

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
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9cfe4c7
add TextEncoder and TextDecoder to jsdom test environment
lobsterkatie Aug 18, 2021
37b4944
add TransactionMetadata to DebugMeta type
lobsterkatie Aug 18, 2021
2ec4722
add segment to user type
lobsterkatie Aug 18, 2021
f9ba96c
add TraceHeaders and SpanContext.getTraceHeaders() to types
lobsterkatie Aug 18, 2021
bbf472d
add base64 string functions
lobsterkatie Aug 18, 2021
9474a4e
fix circular dependency
lobsterkatie Aug 19, 2021
6728b26
move setting default environment to SDK initialization rather than ev…
lobsterkatie Aug 18, 2021
0af6717
remove unused traceHeaders function
lobsterkatie Aug 18, 2021
5e57193
polyfill Object.fromEntries for tracing tests
lobsterkatie Aug 18, 2021
5d53567
s/TRACEPARENT_REGEXP/SENTRY_TRACE_REGEX
lobsterkatie Aug 18, 2021
2a50338
s/extractTraceparentData/extractSentrytraceData
lobsterkatie Aug 18, 2021
54ac0d6
s/traceparent/sentrytrace in various spots
lobsterkatie Aug 19, 2021
58eeda9
add functions for computing tracestate and combined tracing headers
lobsterkatie Aug 19, 2021
a758e16
add tracestate header to outgoing requests
lobsterkatie Aug 19, 2021
a007a83
deprecate toTraceparent
lobsterkatie Aug 19, 2021
1a4336e
add extractTracestateData() function
lobsterkatie Aug 19, 2021
fcb41b3
make transactions accept metadata in their incoming transaction context
lobsterkatie Aug 19, 2021
f0f7ed6
propagate incoming tracestate headers
lobsterkatie Aug 19, 2021
9ab5a8f
extract and propagate tracestate data from <meta> tags
lobsterkatie Aug 19, 2021
08d9ba4
add missing tracestate when capturing transaction, if necessary
lobsterkatie Aug 19, 2021
5c6070e
only deal with envelope header data if we're using an envelope
lobsterkatie Aug 19, 2021
50819b6
add tracestate to envelope header
lobsterkatie Aug 19, 2021
756e855
clean up comments
lobsterkatie Aug 19, 2021
a0b5caf
add http header tests
lobsterkatie Aug 19, 2021
f1a0dbf
random cleanup
lobsterkatie Aug 19, 2021
6415a5a
don't stringify event data until the end
lobsterkatie Aug 19, 2021
15a2e85
rework and add request tests
lobsterkatie Aug 19, 2021
dd76840
remove redundant string test, add browser test for non-base64 string
lobsterkatie Aug 20, 2021
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
17 changes: 17 additions & 0 deletions .jest/dom-environment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const JSDOMEnvironment = require('jest-environment-jsdom');

// TODO Node >= 8.3 includes the same TextEncoder and TextDecoder as exist in the browser, but they haven't yet been
// added to jsdom. Until they are, we can do it ourselves. Once they do, this file can go away.

// see https://github.com/jsdom/jsdom/issues/2524 and https://nodejs.org/api/util.html#util_class_util_textencoder

module.exports = class DOMEnvironment extends JSDOMEnvironment {
async setup() {
await super.setup();
if (typeof this.global.TextEncoder === 'undefined') {
const { TextEncoder, TextDecoder } = require('util');
this.global.TextEncoder = TextEncoder;
this.global.TextDecoder = TextDecoder;
}
}
};
59 changes: 59 additions & 0 deletions packages/browser/test/unit/string.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { base64ToUnicode, unicodeToBase64 } from '@sentry/utils';
import { expect } from 'chai';

// See https://tools.ietf.org/html/rfc4648#section-4 for base64 spec
// eslint-disable-next-line no-useless-escape
const BASE64_REGEX = /([a-zA-Z0-9+/]{4})*(|([a-zA-Z0-9+/]{3}=)|([a-zA-Z0-9+/]{2}==))/;

// NOTE: These tests are copied (and adapted for chai syntax) from `string.test.ts` in `@sentry/utils`. The
// base64-conversion functions have a different implementation in browser and node, so they're copied here to prove they
// work in a real live browser. If you make changes here, make sure to also port them over to that copy.
describe('base64ToUnicode/unicodeToBase64', () => {
const unicodeString = 'Dogs are great!';
const base64String = 'RG9ncyBhcmUgZ3JlYXQh';

it('converts to valid base64', () => {
expect(BASE64_REGEX.test(unicodeToBase64(unicodeString))).to.be.true;
});

it('works as expected (and conversion functions are inverses)', () => {
expect(unicodeToBase64(unicodeString)).to.equal(base64String);
expect(base64ToUnicode(base64String)).to.equal(unicodeString);
});

it('can handle and preserve multi-byte characters in original string', () => {
['🐶', 'Καλό κορίτσι, Μάιζεϊ!', 'Of margir hundar! Ég geri ráð fyrir að ég þurfi stærra rúm.'].forEach(orig => {
expect(() => {
unicodeToBase64(orig);
}).not.to.throw;
expect(base64ToUnicode(unicodeToBase64(orig))).to.equal(orig);
});
});

it('throws an error when given invalid input', () => {
expect(() => {
unicodeToBase64(null as any);
}).to.throw('Unable to convert to base64');
expect(() => {
unicodeToBase64(undefined as any);
}).to.throw('Unable to convert to base64');
expect(() => {
unicodeToBase64({} as any);
}).to.throw('Unable to convert to base64');

expect(() => {
base64ToUnicode(null as any);
}).to.throw('Unable to convert from base64');
expect(() => {
base64ToUnicode(undefined as any);
}).to.throw('Unable to convert from base64');
expect(() => {
base64ToUnicode({} as any);
}).to.throw('Unable to convert from base64');
expect(() => {
// the exclamation point makes this invalid base64
base64ToUnicode('Dogs are great!');
}).to.throw('Unable to convert from base64');
});
});
4 changes: 2 additions & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
const options = this.getOptions();
const { environment, release, dist, maxValueLength = 250 } = options;

if (!('environment' in event)) {
event.environment = 'environment' in options ? environment : 'production';
if (event.environment === undefined && environment !== undefined) {
event.environment = environment;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to tracestate?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Environment is part of the data that goes into tracestate, so it needs to be set in time for that to happen. If the user provides an environment, we're already good, but if we need to use the default, at the moment that's not getting added until we're sending the event. This moves it earlier so that it's available when we compute the tracestate value.

See #3242.

}

if (event.release === undefined && release !== undefined) {
Expand Down
116 changes: 69 additions & 47 deletions packages/core/src/request.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Event, SdkInfo, SentryRequest, SentryRequestType, Session, SessionAggregates } from '@sentry/types';
import { base64ToUnicode, logger } from '@sentry/utils';

import { API } from './api';

Expand All @@ -12,19 +13,20 @@ function getSdkMetadataForEnvelopeHeader(api: API): SdkInfo | undefined {
}

/**
* Apply SdkInfo (name, version, packages, integrations) to the corresponding event key.
* Merge with existing data if any.
* Add SDK metadata (name, version, packages, integrations) to the event.
*
* Mutates the object in place. If prior metadata exists, it will be merged with the given metadata.
**/
function enhanceEventWithSdkInfo(event: Event, sdkInfo?: SdkInfo): Event {
function enhanceEventWithSdkInfo(event: Event, sdkInfo?: SdkInfo): void {
if (!sdkInfo) {
return event;
return;
}
event.sdk = event.sdk || {};
event.sdk.name = event.sdk.name || sdkInfo.name;
event.sdk.version = event.sdk.version || sdkInfo.version;
event.sdk.integrations = [...(event.sdk.integrations || []), ...(sdkInfo.integrations || [])];
event.sdk.packages = [...(event.sdk.packages || []), ...(sdkInfo.packages || [])];
return event;
return;
}

/** Creates a SentryRequest from a Session. */
Expand Down Expand Up @@ -54,61 +56,81 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest {
const eventType = event.type || 'event';
const useEnvelope = eventType === 'transaction' || api.forceEnvelope();

const { transactionSampling, ...metadata } = event.debug_meta || {};
const { method: samplingMethod, rate: sampleRate } = transactionSampling || {};
if (Object.keys(metadata).length === 0) {
delete event.debug_meta;
} else {
event.debug_meta = metadata;
}
enhanceEventWithSdkInfo(event, api.metadata.sdk);

const req: SentryRequest = {
body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event),
type: eventType,
url: useEnvelope ? api.getEnvelopeEndpointWithUrlEncodedAuth() : api.getStoreEndpointWithUrlEncodedAuth(),
};
// 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. See https://develop.sentry.dev/sdk/envelopes/.
if (useEnvelope) {
// Extract header information from event
const { transactionSampling, tracestate, ...metadata } = event.debug_meta || {};
if (Object.keys(metadata).length === 0) {
delete event.debug_meta;
} else {
event.debug_meta = metadata;
}

// https://develop.sentry.dev/sdk/envelopes/
// the tracestate is stored in bas64-encoded JSON, but envelope header values are expected to be full JS values,
// so we have to decode and reinflate it
let reinflatedTracestate;
try {
// Because transaction metadata passes through a number of locations (transactionContext, transaction, event during
// processing, event as sent), each with different requirements, all of the parts are typed as optional. That said,
// if we get to this point and either `tracestate` or `tracestate.sentry` are undefined, something's gone very wrong.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const encodedSentryValue = tracestate!.sentry!.replace('sentry=', '');
reinflatedTracestate = JSON.parse(base64ToUnicode(encodedSentryValue));
} catch (err) {
logger.warn(err);
}

// 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(),
...(sdkInfo && { sdk: sdkInfo }),
...(api.forceEnvelope() && { dsn: api.getDsn().toString() }),
...(reinflatedTracestate && { trace: reinflatedTracestate }), // trace context for dynamic sampling on relay
});
const itemHeaders = JSON.stringify({
type: eventType,

// TODO: Right now, sampleRate may or may not be defined (it won't be in the cases of inheritance and
// explicitly-set sampling decisions). Are we good with that?
sample_rates: [{ id: samplingMethod, rate: sampleRate }],
const itemHeaderEntries: { [key: string]: unknown } = {
type: eventType,

// 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.
// Note: as mentioned above, `content_type` and `length` were left out on purpose.
//
// content_type: 'application/json',
// `content_type`:
// Assumed to be 'application/json' and not part of the current spec for transaction items. No point in bloating the
// request body with it. (Would be `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;
// `length`:
// Optional and equal to the number of bytes in `req.Body` encoded as UTF-8. Since the server can figure this out
// and will refuse events that report the length incorrectly, we decided not to send the length to reduce request
// body size and to avoid problems related to reporting the wrong size.(Would be
// `length: new TextEncoder().encode(req.body).length`.)
};

if (eventType === 'transaction') {
// TODO: Right now, `sampleRate` will be undefined in the cases of inheritance and explicitly-set sampling decisions.
itemHeaderEntries.sample_rates = [{ id: transactionSampling?.method, rate: transactionSampling?.rate }];
}

const itemHeaders = JSON.stringify(itemHeaderEntries);

const eventJSON = JSON.stringify(event);

// The trailing newline is optional; leave it off to avoid sending unnecessary bytes. (Would be
// `const envelope = `${envelopeHeaders}\n${itemHeaders}\n${req.body}\n`;`.)
const envelope = `${envelopeHeaders}\n${itemHeaders}\n${eventJSON}`;

return {
body: envelope,
type: eventType,
url: api.getEnvelopeEndpointWithUrlEncodedAuth(),
};
}

return req;
return {
body: JSON.stringify(event),
type: eventType,
url: api.getStoreEndpointWithUrlEncodedAuth(),
};
}
1 change: 1 addition & 0 deletions packages/core/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export function initAndBind<F extends Client, O extends Options>(clientClass: Cl
if (options.debug === true) {
logger.enable();
}
options.environment = options.environment || 'production';
const hub = getCurrentHub();
hub.getScope()?.update(options.initialScope);
const client = new clientClass(options);
Expand Down
26 changes: 0 additions & 26 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ describe('BaseClient', () => {
const client = new TestClient({ dsn: PUBLIC_DSN });
client.captureException(new Error('test exception'));
expect(TestBackend.instance!.event).toEqual({
environment: 'production',
event_id: '42',
exception: {
values: [
Expand Down Expand Up @@ -246,7 +245,6 @@ describe('BaseClient', () => {
const client = new TestClient({ dsn: PUBLIC_DSN });
client.captureMessage('test message');
expect(TestBackend.instance!.event).toEqual({
environment: 'production',
event_id: '42',
level: 'info',
message: 'test message',
Expand Down Expand Up @@ -322,7 +320,6 @@ describe('BaseClient', () => {
client.captureEvent({ message: 'message' }, undefined, scope);
expect(TestBackend.instance!.event!.message).toBe('message');
expect(TestBackend.instance!.event).toEqual({
environment: 'production',
event_id: '42',
message: 'message',
timestamp: 2020,
Expand All @@ -336,7 +333,6 @@ describe('BaseClient', () => {
client.captureEvent({ message: 'message', timestamp: 1234 }, undefined, scope);
expect(TestBackend.instance!.event!.message).toBe('message');
expect(TestBackend.instance!.event).toEqual({
environment: 'production',
event_id: '42',
message: 'message',
timestamp: 1234,
Expand All @@ -349,28 +345,12 @@ describe('BaseClient', () => {
const scope = new Scope();
client.captureEvent({ message: 'message' }, { event_id: 'wat' }, scope);
expect(TestBackend.instance!.event!).toEqual({
environment: 'production',
event_id: 'wat',
message: 'message',
timestamp: 2020,
});
});

test('sets default environment to `production` it none provided', () => {
expect.assertions(1);
const client = new TestClient({
dsn: PUBLIC_DSN,
});
const scope = new Scope();
client.captureEvent({ message: 'message' }, undefined, scope);
expect(TestBackend.instance!.event!).toEqual({
environment: 'production',
event_id: '42',
message: 'message',
timestamp: 2020,
});
});

test('adds the configured environment', () => {
expect.assertions(1);
const client = new TestClient({
Expand Down Expand Up @@ -412,7 +392,6 @@ describe('BaseClient', () => {
const scope = new Scope();
client.captureEvent({ message: 'message' }, undefined, scope);
expect(TestBackend.instance!.event!).toEqual({
environment: 'production',
event_id: '42',
message: 'message',
release: 'v1.0.0',
Expand Down Expand Up @@ -453,7 +432,6 @@ describe('BaseClient', () => {
scope.setUser({ id: 'user' });
client.captureEvent({ message: 'message' }, undefined, scope);
expect(TestBackend.instance!.event!).toEqual({
environment: 'production',
event_id: '42',
extra: { b: 'b' },
message: 'message',
Expand All @@ -470,7 +448,6 @@ describe('BaseClient', () => {
scope.setFingerprint(['abcd']);
client.captureEvent({ message: 'message' }, undefined, scope);
expect(TestBackend.instance!.event!).toEqual({
environment: 'production',
event_id: '42',
fingerprint: ['abcd'],
message: 'message',
Expand Down Expand Up @@ -525,7 +502,6 @@ describe('BaseClient', () => {
expect(TestBackend.instance!.event!).toEqual({
breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb],
contexts: normalizedObject,
environment: 'production',
event_id: '42',
extra: normalizedObject,
timestamp: 2020,
Expand Down Expand Up @@ -571,7 +547,6 @@ describe('BaseClient', () => {
expect(TestBackend.instance!.event!).toEqual({
breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb],
contexts: normalizedObject,
environment: 'production',
Copy link
Contributor

Choose a reason for hiding this comment

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

These test changes seen unrelated to tracestate

Copy link
Member Author

Choose a reason for hiding this comment

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

See my comment above re: setting default environment earlier.

event_id: '42',
extra: normalizedObject,
timestamp: 2020,
Expand Down Expand Up @@ -622,7 +597,6 @@ describe('BaseClient', () => {
expect(TestBackend.instance!.event!).toEqual({
breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb],
contexts: normalizedObject,
environment: 'production',
event_id: '42',
extra: normalizedObject,
timestamp: 2020,
Expand Down
Loading