From 9cfe4c7eb1a5478fea7a990d71fc7b1936a597b8 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 15:53:17 -0700 Subject: [PATCH 01/28] add TextEncoder and TextDecoder to jsdom test environment --- .jest/dom-environment.js | 17 +++++++++++++++++ packages/gatsby/package.json | 2 +- packages/react/package.json | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 .jest/dom-environment.js diff --git a/.jest/dom-environment.js b/.jest/dom-environment.js new file mode 100644 index 000000000000..b8b45d1e59f2 --- /dev/null +++ b/.jest/dom-environment.js @@ -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; + } + } +}; diff --git a/packages/gatsby/package.json b/packages/gatsby/package.json index 3982b4dee6c2..a9d4af12ee8e 100644 --- a/packages/gatsby/package.json +++ b/packages/gatsby/package.json @@ -76,7 +76,7 @@ "ts", "tsx" ], - "testEnvironment": "jsdom", + "testEnvironment": "../../.jest/dom-environment", "testMatch": [ "**/*.test.ts", "**/*.test.tsx" diff --git a/packages/react/package.json b/packages/react/package.json index 3c1dca3de811..db4ba6f6e74f 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -89,7 +89,7 @@ "ts", "tsx" ], - "testEnvironment": "jsdom", + "testEnvironment": "../../.jest/dom-environment", "testMatch": [ "**/*.test.ts", "**/*.test.tsx" From 37b49440df86d49232d0bc00c48a6589e43daa88 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 15:59:28 -0700 Subject: [PATCH 02/28] add TransactionMetadata to DebugMeta type --- packages/types/src/debugMeta.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/types/src/debugMeta.ts b/packages/types/src/debugMeta.ts index 0c00304dc636..d9851ae11edd 100644 --- a/packages/types/src/debugMeta.ts +++ b/packages/types/src/debugMeta.ts @@ -1,10 +1,11 @@ +import { TransactionMetadata } from './transaction'; + /** * Holds meta information to customize the behavior of sentry's event processing. **/ -export interface DebugMeta { +export type DebugMeta = { images?: Array; - transactionSampling?: { rate?: number; method?: string }; -} +} & TransactionMetadata; /** * Possible choices for debug images. From 2ec472261123fac5ebe84615113601ecb187952a Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 15:59:49 -0700 Subject: [PATCH 03/28] add segment to user type --- packages/types/src/user.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/types/src/user.ts b/packages/types/src/user.ts index eae46b008b3e..3964096aec77 100644 --- a/packages/types/src/user.ts +++ b/packages/types/src/user.ts @@ -5,4 +5,5 @@ export interface User { ip_address?: string; email?: string; username?: string; + segment?: string; } From f9ba96c55c9017b3b4d46bb63fc855cbb8187485 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 16:01:43 -0700 Subject: [PATCH 04/28] add TraceHeaders and SpanContext.getTraceHeaders() to types --- packages/types/src/index.ts | 2 +- packages/types/src/span.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 9674be34db9b..505b558bba6a 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -32,7 +32,7 @@ export { SessionFlusherLike, } from './session'; export { Severity } from './severity'; -export { Span, SpanContext } from './span'; +export { Span, SpanContext, TraceHeaders } from './span'; export { StackFrame } from './stackframe'; export { Stacktrace } from './stacktrace'; export { Status } from './status'; diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 09b69454835e..9ac73425639a 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -158,6 +158,13 @@ export interface Span extends SpanContext { /** Updates the current span with a new `SpanContext` */ updateWithContext(spanContext: SpanContext): this; + /** + * Get headers to attach to any outgoing requests made by the operation corresponding to the current span + * + * @returns An object containing the headers + */ + getTraceHeaders(): TraceHeaders; + /** Convert the object to JSON for w. spans array info only */ getTraceContext(): { data?: { [key: string]: any }; @@ -169,6 +176,7 @@ export interface Span extends SpanContext { tags?: { [key: string]: Primitive }; trace_id: string; }; + /** Convert the object to JSON */ toJSON(): { data?: { [key: string]: any }; @@ -183,3 +191,8 @@ export interface Span extends SpanContext { trace_id: string; }; } + +export type TraceHeaders = { + 'sentry-trace': string; + tracestate?: string; +}; From bbf472de29585bf823aad2104fcceed60e291ccc Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 16:07:29 -0700 Subject: [PATCH 05/28] add base64 string functions --- packages/browser/test/unit/string.test.ts | 64 ++++++++++++ packages/utils/src/string.ts | 116 ++++++++++++++++++++++ packages/utils/test/string.test.ts | 63 +++++++++++- 3 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 packages/browser/test/unit/string.test.ts diff --git a/packages/browser/test/unit/string.test.ts b/packages/browser/test/unit/string.test.ts new file mode 100644 index 000000000000..56119c19a906 --- /dev/null +++ b/packages/browser/test/unit/string.test.ts @@ -0,0 +1,64 @@ +/* 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', () => { + expect(unicodeToBase64(unicodeString)).to.equal(base64String); + expect(base64ToUnicode(base64String)).to.equal(unicodeString); + }); + + it('conversion functions are inverses', () => { + expect(base64ToUnicode(unicodeToBase64(unicodeString))).to.equal(unicodeString); + expect(unicodeToBase64(base64ToUnicode(base64String))).to.equal(base64String); + }); + + 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'); + + // Note that by design, in node base64 encoding and decoding will accept any string, whether or not it's valid + // base64, by ignoring all invalid characters, including whitespace. Therefore, no wacky strings have been included + // here because they don't actually error. + }); +}); diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index 4722ad4f172a..e2b3243da3fc 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -1,3 +1,5 @@ +import { getGlobalObject } from './misc'; +import { SentryError } from './error'; import { isRegExp, isString } from './is'; /** @@ -101,3 +103,117 @@ export function isMatchingPattern(value: string, pattern: RegExp | string): bool } return false; } + +type GlobalWithBase64Helpers = { + // browser + atob?: (base64String: string) => string; + btoa?: (utf8String: string) => string; + // Node + Buffer?: { from: (input: string, encoding: string) => { toString: (encoding: string) => string } }; +}; + +/** + * Convert a Unicode string to a base64 string. + * + * @param plaintext The string to base64-encode + * @throws SentryError (because using the logger creates a circular dependency) + * @returns A base64-encoded version of the string + */ +export function unicodeToBase64(plaintext: string): string { + const globalObject = getGlobalObject(); + + // To account for the fact that different platforms use different character encodings natively, our `tracestate` + // spec calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. + try { + if (typeof plaintext !== 'string') { + throw new Error(`Input must be a string. Received input of type '${typeof plaintext}'.`); + } + + // browser + if ('btoa' in globalObject) { + // encode using UTF-8 + const bytes = new TextEncoder().encode(plaintext); + + // decode using UTF-16 (JS's native encoding) since `btoa` requires string input + const bytesAsString = String.fromCharCode(...bytes); + + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return globalObject.btoa!(bytesAsString); + } + + // Node + if ('Buffer' in globalObject) { + // encode using UTF-8 + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const bytes = globalObject.Buffer!.from(plaintext, 'utf-8'); + + // unlike the browser, Node can go straight from bytes to base64 + return bytes.toString('base64'); + } + + // we shouldn't ever get here, because one of `btoa` and `Buffer` should exist, but just in case... + throw new SentryError('Neither `window.btoa` nor `global.Buffer` is defined.'); + } catch (err) { + // Cast to a string just in case we're given something else + const stringifiedInput = JSON.stringify(plaintext); + const errMsg = `Unable to convert to base64: ${ + stringifiedInput?.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput + }.`; + throw new SentryError(`${errMsg}\nGot error: ${err}`); + } +} + +/** + * Convert a base64 string to a Unicode string. + * + * @param base64String The string to decode + * @throws SentryError (because using the logger creates a circular dependency) + * @returns A Unicode string + */ +export function base64ToUnicode(base64String: string): string { + const globalObject = getGlobalObject(); + + // To account for the fact that different platforms use different character encodings natively, our `tracestate` spec + // calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. So to reverse + // the process, decode from base64 to bytes, then feed those bytes to a UTF-8 decoder. + try { + if (typeof base64String !== 'string') { + throw new Error(`Input must be a string. Received input of type '${typeof base64String}'.`); + } + + // browser + if ('atob' in globalObject) { + // `atob` returns a string rather than bytes, so we first need to encode using the native encoding (UTF-16) + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const bytesAsString = globalObject.atob!(base64String); + const bytes = [...bytesAsString].map(char => char.charCodeAt(0)); + + // decode using UTF-8 (cast the `bytes` arry to a Uint8Array just because that's the format `decode()` expects) + return new TextDecoder().decode(Uint8Array.from(bytes)); + } + + // Node + if ('Buffer' in globalObject) { + // unlike the browser, Node can go straight from base64 to bytes + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const bytes = globalObject.Buffer!.from(base64String, 'base64'); + + // decode using UTF-8 + return bytes.toString('utf-8'); + } + + // we shouldn't ever get here, because one of `atob` and `Buffer` should exist, but just in case... + throw new SentryError('Neither `window.atob` nor `global.Buffer` is defined.'); + } catch (err) { + // we cast to a string just in case we're given something else + const stringifiedInput = JSON.stringify(base64String); + const errMsg = `Unable to convert from base64: ${ + stringifiedInput?.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput + }.`; + throw new SentryError(`${errMsg}\nGot error: ${err}`); + } +} diff --git a/packages/utils/test/string.test.ts b/packages/utils/test/string.test.ts index 316328d9ee13..fe8bfda35481 100644 --- a/packages/utils/test/string.test.ts +++ b/packages/utils/test/string.test.ts @@ -1,4 +1,8 @@ -import { isMatchingPattern, truncate } from '../src/string'; +import { base64ToUnicode, isMatchingPattern, truncate, unicodeToBase64 } from '../src/string'; + +// 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}==))/; describe('truncate()', () => { test('it works as expected', () => { @@ -45,3 +49,60 @@ describe('isMatchingPattern()', () => { expect(isMatchingPattern([] as any, 'foo')).toEqual(false); }); }); + +// NOTE: These tests are copied (and adapted for chai syntax) to `string.test.ts` in `@sentry/browser`. The +// base64-conversion functions have a different implementation in browser and node, so they're copied there 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'; + + test('converts to valid base64', () => { + expect(BASE64_REGEX.test(unicodeToBase64(unicodeString))).toBe(true); + }); + + test('works as expected', () => { + expect(unicodeToBase64(unicodeString)).toEqual(base64String); + expect(base64ToUnicode(base64String)).toEqual(unicodeString); + }); + + test('conversion functions are inverses', () => { + expect(base64ToUnicode(unicodeToBase64(unicodeString))).toEqual(unicodeString); + expect(unicodeToBase64(base64ToUnicode(base64String))).toEqual(base64String); + }); + + test('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.toThrowError(); + expect(base64ToUnicode(unicodeToBase64(orig))).toEqual(orig); + }); + }); + + test('throws an error when given invalid input', () => { + expect(() => { + unicodeToBase64(null as any); + }).toThrowError('Unable to convert to base64'); + expect(() => { + unicodeToBase64(undefined as any); + }).toThrowError('Unable to convert to base64'); + expect(() => { + unicodeToBase64({} as any); + }).toThrowError('Unable to convert to base64'); + + expect(() => { + base64ToUnicode(null as any); + }).toThrowError('Unable to convert from base64'); + expect(() => { + base64ToUnicode(undefined as any); + }).toThrowError('Unable to convert from base64'); + expect(() => { + base64ToUnicode({} as any); + }).toThrowError('Unable to convert from base64'); + + // Note that by design, in node base64 encoding and decoding will accept any string, whether or not it's valid + // base64, by ignoring all invalid characters, including whitespace. Therefore, no wacky strings have been included + // here because they don't actually error. + }); +}); From 9474a4e9fb9338bfec51fdf5e19df331b914eb41 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 19:07:17 -0700 Subject: [PATCH 06/28] fix circular dependency --- packages/utils/src/crossplatform.ts | 44 +++++++++++++++++++++++++++++ packages/utils/src/index.ts | 1 + packages/utils/src/instrument.ts | 2 +- packages/utils/src/logger.ts | 3 +- packages/utils/src/misc.ts | 38 ++----------------------- packages/utils/src/node.ts | 9 ------ packages/utils/src/string.ts | 2 +- packages/utils/src/supports.ts | 2 +- packages/utils/src/time.ts | 4 +-- packages/utils/test/misc.test.ts | 9 ++---- 10 files changed, 56 insertions(+), 58 deletions(-) create mode 100644 packages/utils/src/crossplatform.ts diff --git a/packages/utils/src/crossplatform.ts b/packages/utils/src/crossplatform.ts new file mode 100644 index 000000000000..fbc42fc31843 --- /dev/null +++ b/packages/utils/src/crossplatform.ts @@ -0,0 +1,44 @@ +import { Integration } from '@sentry/types'; + +/** + * Checks whether we're in the Node.js or Browser environment + * + * @returns Answer to given question + */ +export function isNodeEnv(): boolean { + return Object.prototype.toString.call(typeof process !== 'undefined' ? process : 0) === '[object process]'; +} + +/** Internal */ +interface SentryGlobal { + Sentry?: { + Integrations?: Integration[]; + }; + SENTRY_ENVIRONMENT?: string; + SENTRY_DSN?: string; + SENTRY_RELEASE?: { + id?: string; + }; + __SENTRY__: { + globalEventProcessors: any; + hub: any; + logger: any; + }; +} + +const fallbackGlobalObject = {}; + +/** + * Safely get global scope object + * + * @returns Global scope object + */ +export function getGlobalObject(): T & SentryGlobal { + return (isNodeEnv() + ? global + : typeof window !== 'undefined' + ? window + : typeof self !== 'undefined' + ? self + : fallbackGlobalObject) as T & SentryGlobal; +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index da1983b5b7b6..71fd03d63652 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -1,5 +1,6 @@ export * from './async'; export * from './browser'; +export * from './crossplatform'; export * from './dsn'; export * from './error'; export * from './instrument'; diff --git a/packages/utils/src/instrument.ts b/packages/utils/src/instrument.ts index 4079c4afbebc..0e9ed80cf683 100644 --- a/packages/utils/src/instrument.ts +++ b/packages/utils/src/instrument.ts @@ -2,9 +2,9 @@ /* eslint-disable @typescript-eslint/ban-types */ import { WrappedFunction } from '@sentry/types'; +import { getGlobalObject } from './crossplatform'; import { isInstanceOf, isString } from './is'; import { logger } from './logger'; -import { getGlobalObject } from './misc'; import { fill } from './object'; import { getFunctionName } from './stacktrace'; import { supportsHistory, supportsNativeFetch } from './supports'; diff --git a/packages/utils/src/logger.ts b/packages/utils/src/logger.ts index ba804d860991..293491f3ab41 100644 --- a/packages/utils/src/logger.ts +++ b/packages/utils/src/logger.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { consoleSandbox, getGlobalObject } from './misc'; +import { getGlobalObject } from './crossplatform'; +import { consoleSandbox } from './misc'; // TODO: Implement different loggers for different environments const global = getGlobalObject(); diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index 7c7a568db51d..766f9231af67 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -1,43 +1,9 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { Event, Integration, StackFrame, WrappedFunction } from '@sentry/types'; +import { Event, StackFrame, WrappedFunction } from '@sentry/types'; -import { isNodeEnv } from './node'; +import { getGlobalObject } from './crossplatform'; import { snipLine } from './string'; -/** Internal */ -interface SentryGlobal { - Sentry?: { - Integrations?: Integration[]; - }; - SENTRY_ENVIRONMENT?: string; - SENTRY_DSN?: string; - SENTRY_RELEASE?: { - id?: string; - }; - __SENTRY__: { - globalEventProcessors: any; - hub: any; - logger: any; - }; -} - -const fallbackGlobalObject = {}; - -/** - * Safely get global scope object - * - * @returns Global scope object - */ -export function getGlobalObject(): T & SentryGlobal { - return (isNodeEnv() - ? global - : typeof window !== 'undefined' - ? window - : typeof self !== 'undefined' - ? self - : fallbackGlobalObject) as T & SentryGlobal; -} - /** * Extended Window interface that allows for Crypto API usage in IE browsers */ diff --git a/packages/utils/src/node.ts b/packages/utils/src/node.ts index 1b37e0867c07..8e06c586abed 100644 --- a/packages/utils/src/node.ts +++ b/packages/utils/src/node.ts @@ -1,12 +1,3 @@ -/** - * Checks whether we're in the Node.js or Browser environment - * - * @returns Answer to given question - */ -export function isNodeEnv(): boolean { - return Object.prototype.toString.call(typeof process !== 'undefined' ? process : 0) === '[object process]'; -} - /** * Requires a module which is protected against bundler minification. * diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index e2b3243da3fc..cb00adb45622 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -1,4 +1,4 @@ -import { getGlobalObject } from './misc'; +import { getGlobalObject } from './crossplatform'; import { SentryError } from './error'; import { isRegExp, isString } from './is'; diff --git a/packages/utils/src/supports.ts b/packages/utils/src/supports.ts index 88e519b77a24..e695e8f0627d 100644 --- a/packages/utils/src/supports.ts +++ b/packages/utils/src/supports.ts @@ -1,5 +1,5 @@ +import { getGlobalObject } from './crossplatform'; import { logger } from './logger'; -import { getGlobalObject } from './misc'; /** * Tells whether current environment supports ErrorEvent objects diff --git a/packages/utils/src/time.ts b/packages/utils/src/time.ts index 62beac716f02..c76863b06ac1 100644 --- a/packages/utils/src/time.ts +++ b/packages/utils/src/time.ts @@ -1,5 +1,5 @@ -import { getGlobalObject } from './misc'; -import { dynamicRequire, isNodeEnv } from './node'; +import { getGlobalObject, isNodeEnv } from './crossplatform'; +import { dynamicRequire } from './node'; /** * An object that can return the current timestamp in seconds since the UNIX epoch. diff --git a/packages/utils/test/misc.test.ts b/packages/utils/test/misc.test.ts index 72425032a06f..46f4759c137b 100644 --- a/packages/utils/test/misc.test.ts +++ b/packages/utils/test/misc.test.ts @@ -1,12 +1,7 @@ import { StackFrame } from '@sentry/types'; -import { - addContextToFrame, - getEventDescription, - getGlobalObject, - parseRetryAfterHeader, - stripUrlQueryAndFragment, -} from '../src/misc'; +import { getGlobalObject } from '../src/crossplatform'; +import { addContextToFrame, getEventDescription, parseRetryAfterHeader, stripUrlQueryAndFragment } from '../src/misc'; describe('getEventDescription()', () => { test('message event', () => { From 6728b261ebc6eb4e5a0c682352f2d64a7a2d1b8d Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 16:27:28 -0700 Subject: [PATCH 07/28] move setting default environment to SDK initialization rather than event processing --- packages/core/src/baseclient.ts | 4 +-- packages/core/src/sdk.ts | 1 + packages/core/test/lib/base.test.ts | 26 ------------------- packages/core/test/lib/sdk.test.ts | 16 +++++++++++- .../aggregates-disable-single-session.js | 2 +- 5 files changed, 19 insertions(+), 30 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 83aeb475a390..0893046ca132 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -417,8 +417,8 @@ export abstract class BaseClient 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; } if (event.release === undefined && release !== undefined) { diff --git a/packages/core/src/sdk.ts b/packages/core/src/sdk.ts index f674a3e900f4..82640ebd7657 100644 --- a/packages/core/src/sdk.ts +++ b/packages/core/src/sdk.ts @@ -16,6 +16,7 @@ export function initAndBind(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); diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index db9fbeb09664..2e08fb0357a2 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -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: [ @@ -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', @@ -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, @@ -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, @@ -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({ @@ -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', @@ -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', @@ -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', @@ -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, @@ -571,7 +547,6 @@ describe('BaseClient', () => { expect(TestBackend.instance!.event!).toEqual({ breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb], contexts: normalizedObject, - environment: 'production', event_id: '42', extra: normalizedObject, timestamp: 2020, @@ -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, diff --git a/packages/core/test/lib/sdk.test.ts b/packages/core/test/lib/sdk.test.ts index 04f704d0698e..ed1aba49bcee 100644 --- a/packages/core/test/lib/sdk.test.ts +++ b/packages/core/test/lib/sdk.test.ts @@ -19,7 +19,8 @@ jest.mock('@sentry/hub', () => { getClient(): boolean; getScope(): Scope; } { - return { + const mockHub = { + _stack: [], getClient(): boolean { return false; }, @@ -27,10 +28,13 @@ jest.mock('@sentry/hub', () => { return new Scope(); }, bindClient(client: Client): boolean { + (this._stack as any[]).push({ client }); client.setupIntegrations(); return true; }, }; + global.__SENTRY__.hub = mockHub; + return mockHub; }, }; }); @@ -50,6 +54,16 @@ describe('SDK', () => { }); describe('initAndBind', () => { + test("sets environment to 'production' if none is provided", () => { + initAndBind(TestClient, { dsn: PUBLIC_DSN }); + expect(global.__SENTRY__.hub._stack[0].client.getOptions().environment).toEqual('production'); + }); + + test("doesn't overwrite given environment", () => { + initAndBind(TestClient, { dsn: PUBLIC_DSN, environment: 'dogpark' }); + expect(global.__SENTRY__.hub._stack[0].client.getOptions().environment).toEqual('dogpark'); + }); + test('installs default integrations', () => { const DEFAULT_INTEGRATIONS: Integration[] = [ new MockIntegration('MockIntegration 1'), diff --git a/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js b/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js index 4a78c7d2ea94..eb8fb82f0047 100644 --- a/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js +++ b/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js @@ -30,7 +30,7 @@ function assertSessionAggregates(session, expected) { class DummyTransport extends BaseDummyTransport { sendSession(session) { assertSessionAggregates(session, { - attrs: { release: '1.1' }, + attrs: { release: '1.1', environment: 'production' }, aggregates: [{ crashed: 2, errored: 1, exited: 1 }], }); From 0af671729eb6da9976cf5c20abbe22b9ca47c6d5 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 16:28:52 -0700 Subject: [PATCH 08/28] remove unused traceHeaders function --- packages/hub/src/hub.ts | 7 ------- packages/tracing/src/hubextensions.ts | 17 ----------------- packages/types/src/hub.ts | 3 --- 3 files changed, 27 deletions(-) diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index 84c2650c5311..29ceada8b767 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -387,13 +387,6 @@ export class Hub implements HubInterface { return this._callExtensionMethod('startTransaction', context, customSamplingContext); } - /** - * @inheritDoc - */ - public traceHeaders(): { [key: string]: string } { - return this._callExtensionMethod<{ [key: string]: string }>('traceHeaders'); - } - /** * @inheritDoc */ diff --git a/packages/tracing/src/hubextensions.ts b/packages/tracing/src/hubextensions.ts index 081b9ba8b5b3..85edc4cdef1b 100644 --- a/packages/tracing/src/hubextensions.ts +++ b/packages/tracing/src/hubextensions.ts @@ -15,20 +15,6 @@ import { IdleTransaction } from './idletransaction'; import { Transaction } from './transaction'; import { hasTracingEnabled } from './utils'; -/** Returns all trace headers that are currently on the top scope. */ -function traceHeaders(this: Hub): { [key: string]: string } { - const scope = this.getScope(); - if (scope) { - const span = scope.getSpan(); - if (span) { - return { - 'sentry-trace': span.toTraceparent(), - }; - } - } - return {}; -} - /** * Makes a sampling decision for the given transaction and stores it on the transaction. * @@ -216,9 +202,6 @@ export function _addTracingExtensions(): void { if (!carrier.__SENTRY__.extensions.startTransaction) { carrier.__SENTRY__.extensions.startTransaction = _startTransaction; } - if (!carrier.__SENTRY__.extensions.traceHeaders) { - carrier.__SENTRY__.extensions.traceHeaders = traceHeaders; - } } /** diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts index a6be3e203db6..713c200ff0e0 100644 --- a/packages/types/src/hub.ts +++ b/packages/types/src/hub.ts @@ -177,9 +177,6 @@ export interface Hub { /** Returns the integration if installed on the current client. */ getIntegration(integration: IntegrationClass): T | null; - /** Returns all trace headers that are currently on the top scope. */ - traceHeaders(): { [key: string]: string }; - /** * @deprecated No longer does anything. Use use {@link Transaction.startChild} instead. */ From 5e5719317ff474e3539bf05622fce3e845d4f4e2 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 16:37:33 -0700 Subject: [PATCH 09/28] polyfill Object.fromEntries for tracing tests --- packages/tracing/test/testutils.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/tracing/test/testutils.ts b/packages/tracing/test/testutils.ts index a8f42f84b9b8..a7697bdeb577 100644 --- a/packages/tracing/test/testutils.ts +++ b/packages/tracing/test/testutils.ts @@ -56,3 +56,16 @@ export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { return it; }; + +/** Polyfill for `Object.fromEntries`, for Node < 12 */ +export const objectFromEntries = + 'fromEntries' in Object + ? (Object as { fromEntries: any }).fromEntries + : (entries: Array<[string, any]>): Record => { + const result: Record = {}; + entries.forEach(entry => { + const [key, value] = entry; + result[key] = value; + }); + return result; + }; From 5d5356776fdec888be564ddbd1c5e8832e5f7dd9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 16:42:01 -0700 Subject: [PATCH 10/28] s/TRACEPARENT_REGEXP/SENTRY_TRACE_REGEX --- packages/node/test/integrations/http.test.ts | 4 ++-- packages/tracing/src/index.ts | 2 +- packages/tracing/src/utils.ts | 4 ++-- packages/tracing/test/browser/request.test.ts | 2 +- packages/tracing/test/hub.test.ts | 6 +++--- packages/tracing/test/span.test.ts | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 38370650daac..5f0f76799cf6 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -1,7 +1,7 @@ import * as sentryCore from '@sentry/core'; import { Hub } from '@sentry/hub'; import * as hubModule from '@sentry/hub'; -import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sentry/tracing'; +import { addExtensionMethods, SENTRY_TRACE_REGEX, Span, Transaction } from '@sentry/tracing'; import * as http from 'http'; import * as nock from 'nock'; @@ -75,7 +75,7 @@ describe('tracing', () => { const sentryTraceHeader = request.getHeader('sentry-trace') as string; expect(sentryTraceHeader).toBeDefined(); - expect(TRACEPARENT_REGEXP.test(sentryTraceHeader)).toBe(true); + expect(SENTRY_TRACE_REGEX.test(sentryTraceHeader)).toBe(true); }); it("doesn't attach the sentry-trace header to outgoing sentry requests", () => { diff --git a/packages/tracing/src/index.ts b/packages/tracing/src/index.ts index 608bd0630810..9cf5a2f4265e 100644 --- a/packages/tracing/src/index.ts +++ b/packages/tracing/src/index.ts @@ -26,6 +26,6 @@ export { extractTraceparentData, getActiveTransaction, hasTracingEnabled, + SENTRY_TRACE_REGEX, stripUrlQueryAndFragment, - TRACEPARENT_REGEXP, } from './utils'; diff --git a/packages/tracing/src/utils.ts b/packages/tracing/src/utils.ts index 340bc00cf0b5..c6ef3101f885 100644 --- a/packages/tracing/src/utils.ts +++ b/packages/tracing/src/utils.ts @@ -1,7 +1,7 @@ import { getCurrentHub, Hub } from '@sentry/hub'; import { Options, TraceparentData, Transaction } from '@sentry/types'; -export const TRACEPARENT_REGEXP = new RegExp( +export const SENTRY_TRACE_REGEX = new RegExp( '^[ \\t]*' + // whitespace '([0-9a-f]{32})?' + // trace_id '-?([0-9a-f]{16})?' + // span_id @@ -33,7 +33,7 @@ export function hasTracingEnabled( * @returns Object containing data from the header, or undefined if traceparent string is malformed */ export function extractTraceparentData(traceparent: string): TraceparentData | undefined { - const matches = traceparent.match(TRACEPARENT_REGEXP); + const matches = traceparent.match(SENTRY_TRACE_REGEX); if (matches) { let parentSampled: boolean | undefined; if (matches[3] === '1') { diff --git a/packages/tracing/test/browser/request.test.ts b/packages/tracing/test/browser/request.test.ts index 4c2a18788131..f721e2b70939 100644 --- a/packages/tracing/test/browser/request.test.ts +++ b/packages/tracing/test/browser/request.test.ts @@ -214,7 +214,7 @@ describe('callbacks', () => { expect(setRequestHeader).toHaveBeenCalledWith( 'sentry-trace', - expect.stringMatching(tracingUtils.TRACEPARENT_REGEXP), + expect.stringMatching(tracingUtils.SENTRY_TRACE_REGEX), ); }); diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index 0890240fae8c..6a7ffc7aa562 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -8,7 +8,7 @@ import { logger } from '@sentry/utils'; import { BrowserTracing } from '../src/browser/browsertracing'; import { addExtensionMethods } from '../src/hubextensions'; import { Transaction } from '../src/transaction'; -import { extractTraceparentData, TRACEPARENT_REGEXP } from '../src/utils'; +import { extractTraceparentData, SENTRY_TRACE_REGEX } from '../src/utils'; import { addDOMPropertiesToGlobal, getSymbolObjectKeyByName, testOnlyIfNodeVersionAtLeast } from './testutils'; addExtensionMethods(); @@ -371,7 +371,7 @@ describe('Hub', () => { // check that sentry-trace header is added to request expect(headers).toEqual( - expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) }), + expect.objectContaining({ 'sentry-trace': expect.stringMatching(SENTRY_TRACE_REGEX) }), ); // check that sampling decision is passed down correctly @@ -413,7 +413,7 @@ describe('Hub', () => { // check that sentry-trace header is added to request expect(headers).toEqual( - expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) }), + expect.objectContaining({ 'sentry-trace': expect.stringMatching(SENTRY_TRACE_REGEX) }), ); // check that sampling decision is passed down correctly diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 0db4b205a28b..43d7a6a30c4e 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -2,7 +2,7 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain, Scope } from '@sentry/hub'; import { Span, SpanStatus, Transaction } from '../src'; -import { TRACEPARENT_REGEXP } from '../src/utils'; +import { SENTRY_TRACE_REGEX } from '../src/utils'; describe('Span', () => { let hub: Hub; @@ -95,10 +95,10 @@ describe('Span', () => { describe('toTraceparent', () => { test('simple', () => { - expect(new Span().toTraceparent()).toMatch(TRACEPARENT_REGEXP); + expect(new Span().toTraceparent()).toMatch(SENTRY_TRACE_REGEX); }); test('with sample', () => { - expect(new Span({ sampled: true }).toTraceparent()).toMatch(TRACEPARENT_REGEXP); + expect(new Span({ sampled: true }).toTraceparent()).toMatch(SENTRY_TRACE_REGEX); }); }); From 2a50338fc3d1faadb8f477ad109e31ecd27e1cea Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 16:51:14 -0700 Subject: [PATCH 11/28] s/extractTraceparentData/extractSentrytraceData --- packages/nextjs/src/utils/instrumentServer.ts | 4 +-- packages/nextjs/src/utils/withSentry.ts | 4 +-- packages/node/src/handlers.ts | 4 +-- packages/serverless/src/awslambda.ts | 4 +-- packages/serverless/src/gcpfunction/http.ts | 4 +-- .../tracing/src/browser/browsertracing.ts | 4 +-- packages/tracing/src/index.ts | 2 +- packages/tracing/src/utils.ts | 2 +- packages/tracing/test/hub.test.ts | 6 ++--- packages/tracing/test/utils.test.ts | 26 +++++++++---------- 10 files changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index eee338cde538..91d76f3e7b0b 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -1,5 +1,5 @@ import { captureException, deepReadDirSync, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; +import { extractSentrytraceData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; import { fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import * as domain from 'domain'; import * as http from 'http'; @@ -201,7 +201,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string); + traceparentData = extractSentrytraceData(req.headers['sentry-trace'] as string); logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index dbccfceb1724..ffb8ad260cd1 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -1,5 +1,5 @@ import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { extractTraceparentData, hasTracingEnabled } from '@sentry/tracing'; +import { extractSentrytraceData, hasTracingEnabled } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; import { addExceptionMechanism, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import * as domain from 'domain'; @@ -39,7 +39,7 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string); + traceparentData = extractSentrytraceData(req.headers['sentry-trace'] as string); logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index c8156896acc8..a552271fe24b 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ /* eslint-disable @typescript-eslint/no-explicit-any */ import { captureException, getCurrentHub, startTransaction, withScope } from '@sentry/core'; -import { extractTraceparentData, Span } from '@sentry/tracing'; +import { extractSentrytraceData, Span } from '@sentry/tracing'; import { Event, ExtractedNodeRequestData, RequestSessionStatus, Transaction } from '@sentry/types'; import { isPlainObject, isString, logger, normalize, stripUrlQueryAndFragment } from '@sentry/utils'; import * as cookie from 'cookie'; @@ -56,7 +56,7 @@ export function tracingHandler(): ( // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string); + traceparentData = extractSentrytraceData(req.headers['sentry-trace'] as string); } const transaction = startTransaction( diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 79c662d0505c..7a870000a3bf 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -9,7 +9,7 @@ import { withScope, } from '@sentry/node'; import * as Sentry from '@sentry/node'; -import { extractTraceparentData } from '@sentry/tracing'; +import { extractSentrytraceData } from '@sentry/tracing'; import { Integration } from '@sentry/types'; import { isString, logger } from '@sentry/utils'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil @@ -256,7 +256,7 @@ export function wrapHandler( let traceparentData; const eventWithHeaders = event as { headers?: { [key: string]: string } }; if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace'] as string); + traceparentData = extractSentrytraceData(eventWithHeaders.headers['sentry-trace'] as string); } const transaction = startTransaction({ name: context.functionName, diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index d9996842b18c..2ec97e835192 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -1,5 +1,5 @@ import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { extractTraceparentData } from '@sentry/tracing'; +import { extractSentrytraceData } from '@sentry/tracing'; import { isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import { domainify, getActiveDomain, proxyFunction } from './../utils'; @@ -53,7 +53,7 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial | undefined { const header = getMetaContent('sentry-trace'); if (header) { - return extractTraceparentData(header); + return extractSentrytraceData(header); } return undefined; diff --git a/packages/tracing/src/index.ts b/packages/tracing/src/index.ts index 9cf5a2f4265e..a2370873d477 100644 --- a/packages/tracing/src/index.ts +++ b/packages/tracing/src/index.ts @@ -23,7 +23,7 @@ addExtensionMethods(); export { addExtensionMethods }; export { - extractTraceparentData, + extractSentrytraceData, getActiveTransaction, hasTracingEnabled, SENTRY_TRACE_REGEX, diff --git a/packages/tracing/src/utils.ts b/packages/tracing/src/utils.ts index c6ef3101f885..477283b2fef3 100644 --- a/packages/tracing/src/utils.ts +++ b/packages/tracing/src/utils.ts @@ -32,7 +32,7 @@ export function hasTracingEnabled( * * @returns Object containing data from the header, or undefined if traceparent string is malformed */ -export function extractTraceparentData(traceparent: string): TraceparentData | undefined { +export function extractSentrytraceData(traceparent: string): TraceparentData | undefined { const matches = traceparent.match(SENTRY_TRACE_REGEX); if (matches) { let parentSampled: boolean | undefined; diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index 6a7ffc7aa562..691b6123978a 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -8,7 +8,7 @@ import { logger } from '@sentry/utils'; import { BrowserTracing } from '../src/browser/browsertracing'; import { addExtensionMethods } from '../src/hubextensions'; import { Transaction } from '../src/transaction'; -import { extractTraceparentData, SENTRY_TRACE_REGEX } from '../src/utils'; +import { extractSentrytraceData, SENTRY_TRACE_REGEX } from '../src/utils'; import { addDOMPropertiesToGlobal, getSymbolObjectKeyByName, testOnlyIfNodeVersionAtLeast } from './testutils'; addExtensionMethods(); @@ -376,7 +376,7 @@ describe('Hub', () => { // check that sampling decision is passed down correctly expect(transaction.sampled).toBe(true); - expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(true); + expect(extractSentrytraceData(headers['sentry-trace'])!.parentSampled).toBe(true); }, ); @@ -418,7 +418,7 @@ describe('Hub', () => { // check that sampling decision is passed down correctly expect(transaction.sampled).toBe(false); - expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(false); + expect(extractSentrytraceData(headers['sentry-trace'])!.parentSampled).toBe(false); }, ); diff --git a/packages/tracing/test/utils.test.ts b/packages/tracing/test/utils.test.ts index cda8fbbb062f..3615730641a4 100644 --- a/packages/tracing/test/utils.test.ts +++ b/packages/tracing/test/utils.test.ts @@ -1,8 +1,8 @@ -import { extractTraceparentData } from '../src/utils'; +import { extractSentrytraceData } from '../src/utils'; -describe('extractTraceparentData', () => { +describe('extractSentrytraceData', () => { test('no sample', () => { - const data = extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb') as any; + const data = extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb') as any; expect(data).toBeDefined(); expect(data.parentSpanId).toEqual('bbbbbbbbbbbbbbbb'); @@ -11,21 +11,21 @@ describe('extractTraceparentData', () => { }); test('sample true', () => { - const data = extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-1') as any; + const data = extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-1') as any; expect(data).toBeDefined(); expect(data.parentSampled).toBeTruthy(); }); test('sample false', () => { - const data = extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-0') as any; + const data = extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-0') as any; expect(data).toBeDefined(); expect(data.parentSampled).toBeFalsy(); }); test('just sample decision - false', () => { - const data = extractTraceparentData('0') as any; + const data = extractSentrytraceData('0') as any; expect(data).toBeDefined(); expect(data.traceId).toBeUndefined(); @@ -34,7 +34,7 @@ describe('extractTraceparentData', () => { }); test('just sample decision - true', () => { - const data = extractTraceparentData('1') as any; + const data = extractSentrytraceData('1') as any; expect(data).toBeDefined(); expect(data.traceId).toBeUndefined(); @@ -44,21 +44,21 @@ describe('extractTraceparentData', () => { test('invalid', () => { // trace id wrong length - expect(extractTraceparentData('a-bbbbbbbbbbbbbbbb-1')).toBeUndefined(); + expect(extractSentrytraceData('a-bbbbbbbbbbbbbbbb-1')).toBeUndefined(); // parent span id wrong length - expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-b-1')).toBeUndefined(); + expect(extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-b-1')).toBeUndefined(); // parent sampling decision wrong length - expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-11')).toBeUndefined(); + expect(extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-11')).toBeUndefined(); // trace id invalid hex value - expect(extractTraceparentData('someStuffHereWhichIsNotAtAllHexy-bbbbbbbbbbbbbbbb-1')).toBeUndefined(); + expect(extractSentrytraceData('someStuffHereWhichIsNotAtAllHexy-bbbbbbbbbbbbbbbb-1')).toBeUndefined(); // parent span id invalid hex value - expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-alsoNotSuperHexy-1')).toBeUndefined(); + expect(extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-alsoNotSuperHexy-1')).toBeUndefined(); // bogus sampling decision - expect(extractTraceparentData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-x')).toBeUndefined(); + expect(extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-x')).toBeUndefined(); }); }); From 54ac0d6e1c522385cdb1244b1d0047e4374f98b6 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 17:06:37 -0700 Subject: [PATCH 12/28] s/traceparent/sentrytrace in various spots --- packages/node/src/handlers.ts | 6 +++--- packages/serverless/src/awslambda.ts | 6 +++--- packages/serverless/src/gcpfunction/http.ts | 6 +++--- packages/tracing/src/utils.ts | 8 +++++--- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index a552271fe24b..61bf6ab7db5b 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -54,16 +54,16 @@ export function tracingHandler(): ( next: (error?: any) => void, ): void { // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) - let traceparentData; + let sentrytraceData; if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractSentrytraceData(req.headers['sentry-trace'] as string); + sentrytraceData = extractSentrytraceData(req.headers['sentry-trace'] as string); } const transaction = startTransaction( { name: extractExpressTransactionName(req, { path: true, method: true }), op: 'http.server', - ...traceparentData, + ...sentrytraceData, }, // extra context passed to the tracesSampler { request: extractRequestData(req) }, diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 7a870000a3bf..fa0a7bb9ec2f 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -253,15 +253,15 @@ export function wrapHandler( } // Applying `sentry-trace` to context - let traceparentData; + let sentrytraceData; const eventWithHeaders = event as { headers?: { [key: string]: string } }; if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) { - traceparentData = extractSentrytraceData(eventWithHeaders.headers['sentry-trace'] as string); + sentrytraceData = extractSentrytraceData(eventWithHeaders.headers['sentry-trace'] as string); } const transaction = startTransaction({ name: context.functionName, op: 'awslambda.handler', - ...traceparentData, + ...sentrytraceData, }); const hub = getCurrentHub(); diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 2ec97e835192..b6412db1e182 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -50,15 +50,15 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial Date: Wed, 18 Aug 2021 17:16:46 -0700 Subject: [PATCH 13/28] add functions for computing tracestate and combined tracing headers --- packages/tracing/src/span.ts | 89 +++++++++++++++++++++++++++++- packages/tracing/src/utils.ts | 34 ++++++++++++ packages/tracing/test/span.test.ts | 81 ++++++++++++++++++++++++++- 3 files changed, 199 insertions(+), 5 deletions(-) diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts index ed3c03651ba0..1b248f66aa32 100644 --- a/packages/tracing/src/span.ts +++ b/packages/tracing/src/span.ts @@ -1,8 +1,10 @@ /* eslint-disable max-lines */ -import { Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types'; -import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils'; +import { getCurrentHub, Hub } from '@sentry/hub'; +import { Primitive, Span as SpanInterface, SpanContext, TraceHeaders, Transaction } from '@sentry/types'; +import { dropUndefinedKeys, logger, timestampWithMs, uuid4 } from '@sentry/utils'; import { SpanStatus } from './spanstatus'; +import { computeTracestateValue } from './utils'; /** * Keeps track of finished spans for a given transaction @@ -284,6 +286,26 @@ export class Span implements SpanInterface { return this; } + /** + * @inheritDoc + */ + public getTraceHeaders(): TraceHeaders { + // if this span is part of a transaction, but that transaction doesn't yet have a tracestate value, create one + if (this.transaction && !this.transaction?.metadata.tracestate?.sentry) { + this.transaction.metadata.tracestate = { + ...this.transaction.metadata.tracestate, + sentry: this._getNewTracestate(), + }; + } + + const tracestate = this._toTracestate(); + + return { + 'sentry-trace': this._toSentrytrace(), + ...(tracestate && { tracestate }), + }; + } + /** * @inheritDoc */ @@ -339,4 +361,67 @@ export class Span implements SpanInterface { trace_id: this.traceId, }); } + + /** + * Create a new Sentry tracestate header entry (i.e. `sentry=xxxxxx`) + * + * @returns The new Sentry tracestate entry, or undefined if there's no client or no dsn + */ + protected _getNewTracestate(): string | undefined { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any + const hub = ((this.transaction as any)?._hub as Hub) || getCurrentHub(); + const client = hub.getClient(); + const { id: userId, segment: userSegment } = hub.getScope()?.getUser() || {}; + const dsn = client?.getDsn(); + + if (!client || !dsn) { + return; + } + + const { environment, release } = client.getOptions() || {}; + + // only define a `user` object if there's going to be something in it (note: prettier insists on removing the + // parentheses, but since it's easy to misinterpret this, imagine `()` around `userId || userSegment`) + const user = userId || userSegment ? { id: userId, segment: userSegment } : undefined; + + // TODO - the only reason we need the non-null assertion on `dsn.publicKey` (below) is because `dsn.publicKey` has + // to be optional while we transition from `dsn.user` -> `dsn.publicKey`. Once `dsn.user` is removed, we can make + // `dsn.publicKey` required and remove the `!`. + + return `sentry=${computeTracestateValue({ + trace_id: this.traceId, + environment, + release, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + public_key: dsn.publicKey!, + user, + })}`; + } + + /** + * Return a traceparent-compatible header string. + */ + private _toSentrytrace(): string { + let sampledString = ''; + if (this.sampled !== undefined) { + sampledString = this.sampled ? '-1' : '-0'; + } + return `${this.traceId}-${this.spanId}${sampledString}`; + } + + /** + * Return a tracestate-compatible header string, including both sentry and third-party data (if any). Returns + * undefined if there is no client or no DSN. + */ + private _toTracestate(): string | undefined { + // if this is an orphan span, create a new tracestate value + const sentryTracestate = this.transaction?.metadata?.tracestate?.sentry || this._getNewTracestate(); + let thirdpartyTracestate = this.transaction?.metadata?.tracestate?.thirdparty; + + // if there's third-party data, add a leading comma; otherwise, convert from `undefined` to the empty string, so the + // end result doesn’t come out as `sentry=xxxxxundefined` + thirdpartyTracestate = thirdpartyTracestate ? `,${thirdpartyTracestate}` : ''; + + return `${sentryTracestate}${thirdpartyTracestate}`; + } } diff --git a/packages/tracing/src/utils.ts b/packages/tracing/src/utils.ts index 886df02ed251..078c4cac2301 100644 --- a/packages/tracing/src/utils.ts +++ b/packages/tracing/src/utils.ts @@ -1,5 +1,6 @@ import { getCurrentHub, Hub } from '@sentry/hub'; import { Options, TraceparentData, Transaction } from '@sentry/types'; +import { dropUndefinedKeys, SentryError, unicodeToBase64 } from '@sentry/utils'; export const SENTRY_TRACE_REGEX = new RegExp( '^[ \\t]*' + // whitespace @@ -75,3 +76,36 @@ export function secToMs(time: number): number { // so it can be used in manual instrumentation without necessitating a hard dependency on @sentry/utils export { stripUrlQueryAndFragment } from '@sentry/utils'; + +type SentryTracestateData = { + trace_id: string; + environment?: string; + release?: string; + public_key: string; + user?: { id?: string; segment?: string }; +}; + +/** + * Compute the value of a Sentry tracestate header. + * + * @throws SentryError (because using the logger creates a circular dependency) + * @returns the base64-encoded header value + */ +export function computeTracestateValue(data: SentryTracestateData): string { + // `JSON.stringify` will drop keys with undefined values, but not ones with null values, so this prevents + // these values from being dropped if they haven't been set by `Sentry.init` + + // See https://www.w3.org/TR/trace-context/#tracestate-header-field-values + // The spec for tracestate header values calls for a string of the form + // + // identifier1=value1,identifier2=value2,... + // + // which means the value can't include any equals signs, since they already have meaning. Equals signs are commonly + // used to pad the end of base64 values though, so to avoid confusion, we strip them off. (Most languages' base64 + // decoding functions (including those in JS) are able to function without the padding.) + try { + return unicodeToBase64(JSON.stringify(dropUndefinedKeys(data))).replace(/={1,2}$/, ''); + } catch (err) { + throw new SentryError(`[Tracing] Error computing tracestate value from data: ${err}\nData: ${data}`); + } +} diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 43d7a6a30c4e..a5959d2a9801 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -1,8 +1,9 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain, Scope } from '@sentry/hub'; +import * as hubPackage from '@sentry/hub'; import { Span, SpanStatus, Transaction } from '../src'; -import { SENTRY_TRACE_REGEX } from '../src/utils'; +import { computeTracestateValue, SENTRY_TRACE_REGEX } from '../src/utils'; describe('Span', () => { let hub: Hub; @@ -93,12 +94,86 @@ describe('Span', () => { }); }); + // TODO Once toTraceparent is removed, we obv don't need these tests anymore describe('toTraceparent', () => { test('simple', () => { - expect(new Span().toTraceparent()).toMatch(SENTRY_TRACE_REGEX); + expect(new Span().getTraceHeaders()['sentry-trace']).toMatch(SENTRY_TRACE_REGEX); }); test('with sample', () => { - expect(new Span({ sampled: true }).toTraceparent()).toMatch(SENTRY_TRACE_REGEX); + expect(new Span({ sampled: true }).getTraceHeaders()['sentry-trace']).toMatch(SENTRY_TRACE_REGEX); + }); + }); + + describe('toTracestate', () => { + const publicKey = 'dogsarebadatkeepingsecrets'; + const release = 'off.leash.trail'; + const environment = 'dogpark'; + const traceId = '12312012123120121231201212312012'; + const user = { id: '1121', segment: 'bigs' }; + + const computedTracestate = `sentry=${computeTracestateValue({ + trace_id: traceId, + environment, + release, + public_key: publicKey, + user, + })}`; + const thirdpartyData = 'maisey=silly,charlie=goofy'; + + const hub = new Hub( + new BrowserClient({ + dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', + tracesSampleRate: 1, + release, + environment, + }), + ); + + hub.configureScope(scope => { + scope.setUser(user); + }); + + test('no third-party data', () => { + const transaction = new Transaction({ name: 'FETCH /ball', traceId }, hub); + const span = transaction.startChild({ op: 'dig.hole' }); + + expect(span.getTraceHeaders().tracestate).toEqual(computedTracestate); + }); + + test('third-party data', () => { + const transaction = new Transaction({ name: 'FETCH /ball' }, hub); + transaction.setMetadata({ tracestate: { sentry: computedTracestate, thirdparty: thirdpartyData } }); + const span = transaction.startChild({ op: 'dig.hole' }); + + expect(span.getTraceHeaders().tracestate).toEqual(`${computedTracestate},${thirdpartyData}`); + }); + + test('orphan span', () => { + jest.spyOn(hubPackage, 'getCurrentHub').mockReturnValueOnce(hub); + const span = new Span({ op: 'dig.hole' }); + span.traceId = traceId; + + expect(span.getTraceHeaders().tracestate).toEqual(computedTracestate); + }); + }); + + describe('getTraceHeaders', () => { + it('returns correct headers', () => { + const hub = new Hub( + new BrowserClient({ + dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', + tracesSampleRate: 1, + release: 'off.leash.park', + environment: 'dogpark', + }), + ); + const transaction = hub.startTransaction({ name: 'FETCH /ball' }); + const span = transaction.startChild({ op: 'dig.hole' }); + + const headers = span.getTraceHeaders(); + + expect(headers['sentry-trace']).toEqual(`${span.traceId}-${span.spanId}-1`); + expect(headers.tracestate).toEqual(transaction.metadata?.tracestate?.sentry); }); }); From a758e1672f6560fde22aee9dccfbe7bdb3ceafc0 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 17:36:51 -0700 Subject: [PATCH 14/28] add tracestate header to outgoing requests --- packages/node/src/integrations/http.ts | 10 +- packages/node/test/integrations/http.test.ts | 23 +-- packages/tracing/src/browser/request.ts | 38 +++-- packages/tracing/test/browser/request.test.ts | 133 +++++++++++++++--- 4 files changed, 161 insertions(+), 43 deletions(-) diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 77e72228ee66..7fed8ebdfeda 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,5 +1,5 @@ import { getCurrentHub } from '@sentry/core'; -import { Integration, Span } from '@sentry/types'; +import { Integration, Span, TraceHeaders } from '@sentry/types'; import { fill, logger, parseSemver } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; @@ -115,11 +115,9 @@ function _createWrappedRequestMethodFactory( op: 'request', }); - const sentryTraceHeader = span.toTraceparent(); - logger.log( - `[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `, - ); - requestOptions.headers = { ...requestOptions.headers, 'sentry-trace': sentryTraceHeader }; + const traceHeaders = span.getTraceHeaders(); + logger.log(`[Tracing] Adding sentry-trace and tracestate headers to outgoing request to ${requestUrl}.`); + requestOptions.headers = { ...requestOptions.headers, ...(traceHeaders as TraceHeaders) }; } } diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 5f0f76799cf6..705ff6f8ebdb 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -9,7 +9,7 @@ import { NodeClient } from '../../src/client'; import { Http as HttpIntegration } from '../../src/integrations/http'; describe('tracing', () => { - function createTransactionOnScope() { + function createTransactionOnScope(): Transaction { const hub = new Hub( new NodeClient({ dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', @@ -24,7 +24,10 @@ describe('tracing', () => { jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub); - const transaction = hub.startTransaction({ name: 'dogpark' }); + // we have to cast this to a Transaction (the class) because hub.startTransaction only returns a Transaction (the + // interface, which doesn't have things like spanRecorder) (and because @sentry/hub can't depend on @sentry/tracing, + // we can't fix that) + const transaction = hub.startTransaction({ name: 'dogpark' }) as Transaction; hub.getScope()?.setSpan(transaction); return transaction; @@ -36,7 +39,7 @@ describe('tracing', () => { .reply(200); const transaction = createTransactionOnScope(); - const spans = (transaction as Span).spanRecorder?.spans as Span[]; + const spans = transaction.spanRecorder?.spans as Span[]; http.get('http://dogs.are.great/'); @@ -55,7 +58,7 @@ describe('tracing', () => { .reply(200); const transaction = createTransactionOnScope(); - const spans = (transaction as Span).spanRecorder?.spans as Span[]; + const spans = transaction.spanRecorder?.spans as Span[]; http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/'); @@ -64,7 +67,7 @@ describe('tracing', () => { expect((spans[0] as Transaction).name).toEqual('dogpark'); }); - it('attaches the sentry-trace header to outgoing non-sentry requests', async () => { + it('attaches tracing headers to outgoing non-sentry requests', async () => { nock('http://dogs.are.great') .get('/') .reply(200); @@ -72,13 +75,15 @@ describe('tracing', () => { createTransactionOnScope(); const request = http.get('http://dogs.are.great/'); - const sentryTraceHeader = request.getHeader('sentry-trace') as string; + const sentryTraceHeader = request.getHeader('sentry-trace'); + const tracestateHeader = request.getHeader('tracestate'); expect(sentryTraceHeader).toBeDefined(); - expect(SENTRY_TRACE_REGEX.test(sentryTraceHeader)).toBe(true); + expect(tracestateHeader).toBeDefined(); + expect(SENTRY_TRACE_REGEX.test(sentryTraceHeader as string)).toBe(true); }); - it("doesn't attach the sentry-trace header to outgoing sentry requests", () => { + it("doesn't attach tracing headers to outgoing sentry requests", () => { nock('http://squirrelchasers.ingest.sentry.io') .get('/api/12312012/store/') .reply(200); @@ -87,7 +92,9 @@ describe('tracing', () => { const request = http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/'); const sentryTraceHeader = request.getHeader('sentry-trace'); + const tracestateHeader = request.getHeader('tracestate'); expect(sentryTraceHeader).not.toBeDefined(); + expect(tracestateHeader).not.toBeDefined(); }); }); diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index 885d381017dc..b816568de3a7 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,6 +1,6 @@ +import { Span } from '@sentry/types'; import { addInstrumentationHandler, isInstanceOf, isMatchingPattern } from '@sentry/utils'; -import { Span } from '../span'; import { SpanStatus } from '../spanstatus'; import { getActiveTransaction, hasTracingEnabled } from '../utils'; @@ -59,6 +59,16 @@ export interface FetchData { endTimestamp?: number; } +type PolymorphicRequestHeaders = + | Record + | Array<[string, string]> + // the below is not preicsely the Header type used in Request, but it'll pass duck-typing + | { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; + append: (key: string, value: string) => void; + }; + /** Data returned from XHR request */ export interface XHRData { xhr?: { @@ -183,22 +193,26 @@ export function fetchCallback( const request = (handlerData.args[0] = handlerData.args[0] as string | Request); // eslint-disable-next-line @typescript-eslint/no-explicit-any const options = (handlerData.args[1] = (handlerData.args[1] as { [key: string]: any }) || {}); - let headers = options.headers; + let headers: PolymorphicRequestHeaders = options.headers; if (isInstanceOf(request, Request)) { headers = (request as Request).headers; } + const traceHeaders = span.getTraceHeaders() as Record; if (headers) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (typeof headers.append === 'function') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - headers.append('sentry-trace', span.toTraceparent()); + if ('append' in headers && typeof headers.append === 'function') { + headers.append('sentry-trace', traceHeaders['sentry-trace']); + if (traceHeaders.tracestate) { + headers.append('tracestate', traceHeaders.tracestate); + } } else if (Array.isArray(headers)) { - headers = [...headers, ['sentry-trace', span.toTraceparent()]]; + // TODO use the nicer version below once we stop supporting Node 6 + // headers = [...headers, ...Object.entries(traceHeaders)]; + headers = [...headers, ['sentry-trace', traceHeaders['sentry-trace']], ['tracestate', traceHeaders.tracestate]]; } else { - headers = { ...headers, 'sentry-trace': span.toTraceparent() }; + headers = { ...headers, ...traceHeaders }; } } else { - headers = { 'sentry-trace': span.toTraceparent() }; + headers = traceHeaders; } options.headers = headers; } @@ -254,7 +268,11 @@ export function xhrCallback( if (handlerData.xhr.setRequestHeader) { try { - handlerData.xhr.setRequestHeader('sentry-trace', span.toTraceparent()); + const sentryHeaders = span.getTraceHeaders(); + handlerData.xhr.setRequestHeader('sentry-trace', sentryHeaders['sentry-trace']); + if (sentryHeaders.tracestate) { + handlerData.xhr.setRequestHeader('tracestate', sentryHeaders.tracestate); + } } catch (_) { // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. } diff --git a/packages/tracing/test/browser/request.test.ts b/packages/tracing/test/browser/request.test.ts index f721e2b70939..50f9c475b09e 100644 --- a/packages/tracing/test/browser/request.test.ts +++ b/packages/tracing/test/browser/request.test.ts @@ -6,12 +6,41 @@ import { Span, SpanStatus, Transaction } from '../../src'; import { fetchCallback, FetchData, instrumentOutgoingRequests, xhrCallback, XHRData } from '../../src/browser/request'; import { addExtensionMethods } from '../../src/hubextensions'; import * as tracingUtils from '../../src/utils'; +import { objectFromEntries } from '../testutils'; + +// This is a normal base64 regex, modified to reflect that fact that we strip the trailing = or == off +const stripped_base64 = '([a-zA-Z0-9+/]{4})*([a-zA-Z0-9+/]{2,3})?'; + +const TRACESTATE_HEADER_REGEX = new RegExp( + `sentry=(${stripped_base64})` + // our part of the header - should be the only part or at least the first part + `(,\\w+=\\w+)*`, // any number of copies of a comma followed by `name=value` +); beforeAll(() => { addExtensionMethods(); - // @ts-ignore need to override global Request because it's not in the jest environment (even with an - // `@jest-environment jsdom` directive, for some reason) - global.Request = {}; + + // Add Request to the global scope (necessary because for some reason Request isn't in the jest environment, even with + // an `@jest-environment jsdom` directive) + + type MockHeaders = { + [key: string]: any; + append: (key: string, value: string) => void; + }; + + class Request { + public headers: MockHeaders; + constructor() { + // We need our headers to act like an object for key-lookup purposes, but also have an append method that adds + // items as its siblings. This hack precludes a key named `append`, of course, but for our purposes it's enough. + const headers = {} as MockHeaders; + headers.append = (key: string, value: any): void => { + headers[key] = value; + }; + this.headers = headers; + } + } + + (global as any).Request = Request; }); const hasTracingEnabled = jest.spyOn(tracingUtils, 'hasTracingEnabled'); @@ -49,7 +78,7 @@ describe('instrumentOutgoingRequests', () => { }); }); -describe('callbacks', () => { +describe('fetch and xhr callbacks', () => { let hub: Hub; let transaction: Transaction; const alwaysCreateSpan = () => true; @@ -57,7 +86,7 @@ describe('callbacks', () => { const fetchHandlerData: FetchData = { args: ['http://dogs.are.great/', {}], fetchData: { url: 'http://dogs.are.great/', method: 'GET' }, - startTimestamp: 1356996072000, + startTimestamp: 2012112120121231, }; const xhrHandlerData: XHRData = { xhr: { @@ -72,18 +101,27 @@ describe('callbacks', () => { // setRequestHeader: XMLHttpRequest.prototype.setRequestHeader, setRequestHeader, }, - startTimestamp: 1353501072000, + startTimestamp: 2012112120121231, }; - const endTimestamp = 1356996072000; + const endTimestamp = 2013041520130908; beforeAll(() => { - hub = new Hub(new BrowserClient({ tracesSampleRate: 1 })); + hub = new Hub( + new BrowserClient({ + dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', + environment: 'dogpark', + release: 'off.leash.park', + + tracesSampleRate: 1, + }), + ); makeMain(hub); }); beforeEach(() => { - transaction = hub.startTransaction({ name: 'organizations/users/:userid', op: 'pageload' }) as Transaction; + transaction = hub.startTransaction({ name: 'meetNewDogFriend', op: 'wag.tail' }) as Transaction; hub.configureScope(scope => scope.setSpan(transaction)); + jest.clearAllMocks(); }); describe('fetchCallback()', () => { @@ -111,20 +149,17 @@ describe('callbacks', () => { expect(spans).toEqual({}); }); - it('does not add fetch request headers if tracing is disabled', () => { + it('does not add tracing headers if tracing is disabled', () => { hasTracingEnabled.mockReturnValueOnce(false); // make a local copy so the global one doesn't get mutated - const handlerData: FetchData = { - args: ['http://dogs.are.great/', {}], - fetchData: { url: 'http://dogs.are.great/', method: 'GET' }, - startTimestamp: 1353501072000, - }; + const handlerData = { ...fetchHandlerData }; fetchCallback(handlerData, alwaysCreateSpan, {}); const headers = (handlerData.args[1].headers as Record) || {}; expect(headers['sentry-trace']).not.toBeDefined(); + expect(headers['tracestate']).not.toBeDefined(); }); it('creates and finishes fetch span on active transaction', () => { @@ -179,8 +214,67 @@ describe('callbacks', () => { expect(newSpan!.status).toBe(SpanStatus.fromHttpCode(404)); }); - it('adds sentry-trace header to fetch requests', () => { - // TODO + describe('adding tracing headers to fetch requests', () => { + it('can handle headers added with an `append` method', () => { + const handlerData: FetchData = { ...fetchHandlerData, args: [new Request('http://dogs.are.great'), {}] }; + + fetchCallback(handlerData, alwaysCreateSpan, {}); + + const headers = handlerData.args[1].headers; + expect(headers['sentry-trace']).toBeDefined(); + expect(headers['tracestate']).toBeDefined(); + }); + + it('can handle existing headers in array form', () => { + const handlerData = { + ...fetchHandlerData, + args: [ + 'http://dogs.are.great/', + { + headers: [ + ['GREETING_PROTOCOL', 'mutual butt sniffing'], + ['TAIL_ACTION', 'wagging'], + ], + }, + ], + }; + + fetchCallback(handlerData, alwaysCreateSpan, {}); + + const headers = objectFromEntries((handlerData.args[1] as any).headers); + expect(headers['sentry-trace']).toBeDefined(); + expect(headers['tracestate']).toBeDefined(); + }); + + it('can handle existing headers in object form', () => { + const handlerData = { + ...fetchHandlerData, + args: [ + 'http://dogs.are.great/', + { + headers: { GREETING_PROTOCOL: 'mutual butt sniffing', TAIL_ACTION: 'wagging' }, + }, + ], + }; + + fetchCallback(handlerData, alwaysCreateSpan, {}); + + const headers = (handlerData.args[1] as any).headers; + expect(headers['sentry-trace']).toBeDefined(); + expect(headers['tracestate']).toBeDefined(); + }); + + it('can handle there being no existing headers', () => { + // override the value of `args`, even though we're overriding it with the same data, as a means of deep copying + // the one part which gets mutated + const handlerData = { ...fetchHandlerData, args: ['http://dogs.are.great/', {}] }; + + fetchCallback(handlerData, alwaysCreateSpan, {}); + + const headers = (handlerData.args[1] as any).headers; + expect(headers['sentry-trace']).toBeDefined(); + expect(headers['tracestate']).toBeDefined(); + }); }); }); @@ -201,7 +295,7 @@ describe('callbacks', () => { expect(spans).toEqual({}); }); - it('does not add xhr request headers if tracing is disabled', () => { + it('does not add tracing headers if tracing is disabled', () => { hasTracingEnabled.mockReturnValueOnce(false); xhrCallback(xhrHandlerData, alwaysCreateSpan, {}); @@ -209,13 +303,14 @@ describe('callbacks', () => { expect(setRequestHeader).not.toHaveBeenCalled(); }); - it('adds sentry-trace header to XHR requests', () => { + it('adds tracing headers to XHR requests', () => { xhrCallback(xhrHandlerData, alwaysCreateSpan, {}); expect(setRequestHeader).toHaveBeenCalledWith( 'sentry-trace', expect.stringMatching(tracingUtils.SENTRY_TRACE_REGEX), ); + expect(setRequestHeader).toHaveBeenCalledWith('tracestate', expect.stringMatching(TRACESTATE_HEADER_REGEX)); }); it('creates and finishes XHR span on active transaction', () => { From a007a833b97efb601acc77ccc4c0fc0bb29e9a84 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 17:41:48 -0700 Subject: [PATCH 15/28] deprecate toTraceparent --- packages/tracing/src/span.ts | 10 ++++------ packages/types/src/span.ts | 8 ++++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts index 1b248f66aa32..46e316a20386 100644 --- a/packages/tracing/src/span.ts +++ b/packages/tracing/src/span.ts @@ -126,7 +126,7 @@ export class Span implements SpanInterface { if (spanContext.parentSpanId) { this.parentSpanId = spanContext.parentSpanId; } - // We want to include booleans as well here + // check this way instead of the normal way to make sure we don't miss cases where sampled = false if ('sampled' in spanContext) { this.sampled = spanContext.sampled; } @@ -241,11 +241,9 @@ export class Span implements SpanInterface { * @inheritDoc */ public toTraceparent(): string { - let sampledString = ''; - if (this.sampled !== undefined) { - sampledString = this.sampled ? '-1' : '-0'; - } - return `${this.traceId}-${this.spanId}${sampledString}`; + logger.warn('Direct use of `span.toTraceparent` is deprecated. Use `span.getTraceHeaders` instead.'); + + return this._toSentrytrace(); } /** diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 9ac73425639a..fbf5111f43be 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -149,8 +149,12 @@ export interface Span extends SpanContext { */ isSuccess(): boolean; - /** Return a traceparent compatible header string */ - toTraceparent(): string; + /** + * Return a traceparent-compatible header string + * + * @deprecated Do not use `span.toTraceparnt` directly. Use `span.getTraceHeaders` instead. + */ + toTraceparent(): string; // TODO (kmclb) make this private /** Returns the current span properties as a `SpanContext` */ toContext(): SpanContext; From 1a4336e892436e5b95bd5c46bd49e6c80a2f256f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 17:43:14 -0700 Subject: [PATCH 16/28] add extractTracestateData() function --- packages/tracing/src/index.ts | 1 + packages/tracing/src/utils.ts | 74 +++++++++++++++++++++++++++++ packages/tracing/test/utils.test.ts | 58 +++++++++++++++++++++- 3 files changed, 132 insertions(+), 1 deletion(-) diff --git a/packages/tracing/src/index.ts b/packages/tracing/src/index.ts index a2370873d477..6becbb0c4be8 100644 --- a/packages/tracing/src/index.ts +++ b/packages/tracing/src/index.ts @@ -24,6 +24,7 @@ export { addExtensionMethods }; export { extractSentrytraceData, + extractTracestateData, getActiveTransaction, hasTracingEnabled, SENTRY_TRACE_REGEX, diff --git a/packages/tracing/src/utils.ts b/packages/tracing/src/utils.ts index 078c4cac2301..92a739069329 100644 --- a/packages/tracing/src/utils.ts +++ b/packages/tracing/src/utils.ts @@ -10,6 +10,40 @@ export const SENTRY_TRACE_REGEX = new RegExp( '[ \\t]*$', // whitespace ); +// This is a normal base64 regex, modified to reflect that fact that we strip the trailing = or == off +const BASE64_STRIPPED_REGEX = new RegExp( + // for our purposes, we want to test against the entire string, so enforce that there's nothing before the main regex + `^` + + // any of the characters in the base64 "alphabet", in multiples of 4 + '([a-zA-Z0-9+/]{4})*' + + // either nothing or 2 or 3 base64-alphabet characters (see + // https://en.wikipedia.org/wiki/Base64#Decoding_Base64_without_padding + // for why there's never only 1 extra character) + '([a-zA-Z0-9+/]{2,3})?' + + // see above re: matching entire string + `$`, +); + +// comma-delimited list of entries of the form `xxx=yyy` +const tracestateEntry = '[^=]+=[^=]+'; +const TRACESTATE_ENTRIES_REGEX = new RegExp( + // one or more xxxxx=yyyy entries + `^(${tracestateEntry})+` + + // each entry except the last must be followed by a comma + '(,|$)', +); + +// this doesn't check that the value is valid, just that there's something there of the form `sentry=xxxx` +const SENTRY_TRACESTATE_ENTRY_REGEX = new RegExp( + // either sentry is the first entry or there's stuff immediately before it, ending in a commma (this prevents matching + // something like `coolsentry=xxx`) + '(?:^|.+,)' + + // sentry's part, not including the potential comma + '(sentry=[^,]*)' + + // either there's a comma and another vendor's entry or we end + '(?:,.+|$)', +); + /** * Determines if tracing is currently enabled. * @@ -53,6 +87,46 @@ export function extractSentrytraceData(header: string): TraceparentData | undefi return undefined; } +/** + * Extract data from an incoming `tracestate` header + * + * @param header + * @returns Object containing data from the header + */ +export function extractTracestateData(header: string): { sentry?: string; thirdparty?: string } { + let sentryEntry, thirdPartyEntry, before, after; + + // find sentry's entry, if any + const sentryMatch = SENTRY_TRACESTATE_ENTRY_REGEX.exec(header); + + if (sentryMatch !== null) { + sentryEntry = sentryMatch[1]; + + // remove the commas after the split so we don't end up with `xxx=yyy,,zzz=qqq` (double commas) when we put them + // back together + [before, after] = header.split(sentryEntry).map(s => s.replace(/^,*|,*$/g, '')); + + // extract sentry's value from its entry and test to make sure it's valid; if it isn't, discard the entire entry + // so that a new one will be created by the Transaction constructor + const sentryValue = sentryEntry.replace('sentry=', ''); + if (!BASE64_STRIPPED_REGEX.test(sentryValue)) { + sentryEntry = undefined; + } + } else { + // this could just as well be `before`; we just need to get the thirdparty data into one or the other since + // there's no valid Sentry entry + after = header; + } + + // if either thirdparty part is invalid or empty, remove it before gluing them together + const validThirdpartyEntries = [before, after].filter(x => TRACESTATE_ENTRIES_REGEX.test(x || '')); + if (validThirdpartyEntries.length) { + thirdPartyEntry = validThirdpartyEntries.join(','); + } + + return { sentry: sentryEntry, thirdparty: thirdPartyEntry }; +} + /** Grabs active transaction off scope, if any */ export function getActiveTransaction(hub: Hub = getCurrentHub()): T | undefined { return hub?.getScope()?.getTransaction() as T | undefined; diff --git a/packages/tracing/test/utils.test.ts b/packages/tracing/test/utils.test.ts index 3615730641a4..57c169d9d745 100644 --- a/packages/tracing/test/utils.test.ts +++ b/packages/tracing/test/utils.test.ts @@ -1,4 +1,4 @@ -import { extractSentrytraceData } from '../src/utils'; +import { extractSentrytraceData, extractTracestateData } from '../src/utils'; describe('extractSentrytraceData', () => { test('no sample', () => { @@ -62,3 +62,59 @@ describe('extractSentrytraceData', () => { expect(extractSentrytraceData('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-x')).toBeUndefined(); }); }); + +describe('extractTracestateData', () => { + it.each([ + // sentry only + ['sentry only', 'sentry=doGsaREgReaT', 'sentry=doGsaREgReaT', undefined], + // sentry only, invalid (`!` isn't a valid base64 character) + ['sentry only, invalid', 'sentry=doGsaREgReaT!', undefined, undefined], + // stuff before + ['stuff before', 'maisey=silly,sentry=doGsaREgReaT', 'sentry=doGsaREgReaT', 'maisey=silly'], + // stuff after + ['stuff after', 'sentry=doGsaREgReaT,maisey=silly', 'sentry=doGsaREgReaT', 'maisey=silly'], + // stuff before and after + [ + 'stuff before and after', + 'charlie=goofy,sentry=doGsaREgReaT,maisey=silly', + 'sentry=doGsaREgReaT', + 'charlie=goofy,maisey=silly', + ], + // multiple before + [ + 'multiple before', + 'charlie=goofy,maisey=silly,sentry=doGsaREgReaT', + 'sentry=doGsaREgReaT', + 'charlie=goofy,maisey=silly', + ], + // multiple after + [ + 'multiple after', + 'sentry=doGsaREgReaT,charlie=goofy,maisey=silly', + 'sentry=doGsaREgReaT', + 'charlie=goofy,maisey=silly', + ], + // multiple before and after + [ + 'multiple before and after', + 'charlie=goofy,maisey=silly,sentry=doGsaREgReaT,bodhi=floppy,cory=loyal', + 'sentry=doGsaREgReaT', + 'charlie=goofy,maisey=silly,bodhi=floppy,cory=loyal', + ], + // only third-party data + ['only third-party data', 'maisey=silly', undefined, 'maisey=silly'], + // invalid third-party data, valid sentry data + [ + 'invalid third-party data, valid sentry data', + 'maisey_is_silly,sentry=doGsaREgReaT', + 'sentry=doGsaREgReaT', + undefined, + ], + // valid third party data, invalid sentry data + ['valid third-party data, invalid sentry data', 'maisey=silly,sentry=doGsaREgReaT!', undefined, 'maisey=silly'], + // nothing valid at all + ['nothing valid at all', 'maisey_is_silly,sentry=doGsaREgReaT!', undefined, undefined], + ])('%s', (_testTitle: string, header: string, sentry?: string, thirdparty?: string): void => { + expect(extractTracestateData(header)).toEqual({ sentry, thirdparty }); + }); +}); From fcb41b3569280b9e535727733688ce22b8d8ab36 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 17:55:48 -0700 Subject: [PATCH 17/28] make transactions accept metadata in their incoming transaction context --- packages/tracing/test/hub.test.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index 691b6123978a..09d31dfe1965 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -28,6 +28,29 @@ describe('Hub', () => { jest.clearAllMocks(); }); + describe('transaction creation', () => { + const hub = new Hub( + new BrowserClient({ + dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', + environment: 'dogpark', + release: 'off.leash.trail', + }), + ); + + it('uses inherited values when given in transaction context', () => { + const transactionContext = { + name: 'FETCH /ball', + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT', thirdparty: 'maisey=silly;charlie=goofy' } }, + }; + + const transaction = hub.startTransaction(transactionContext); + + expect(transaction).toEqual(expect.objectContaining(transactionContext)); + }); + }); + describe('getTransaction()', () => { it('should find a transaction which has been set on the scope if sampled = true', () => { const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 })); From f0f7ed62a493e62d83e4194406a23a90de6f106f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 17:46:14 -0700 Subject: [PATCH 18/28] propagate incoming tracestate headers --- packages/nextjs/src/utils/instrumentServer.ts | 24 +++-- packages/nextjs/src/utils/withSentry.ts | 20 +++-- packages/node/src/handlers.ts | 12 ++- packages/node/test/handlers.test.ts | 8 +- packages/serverless/src/awslambda.ts | 18 ++-- packages/serverless/src/gcpfunction/http.ts | 16 ++-- packages/serverless/test/awslambda.test.ts | 87 +++++++++++++++++++ packages/serverless/test/gcpfunction.test.ts | 29 +++++++ 8 files changed, 182 insertions(+), 32 deletions(-) diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index 91d76f3e7b0b..91e96cdc3465 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -1,6 +1,11 @@ import { captureException, deepReadDirSync, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { extractSentrytraceData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; -import { fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; +import { + extractSentrytraceData, + extractTracestateData, + getActiveTransaction, + hasTracingEnabled, +} from '@sentry/tracing'; +import { fill, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import * as domain from 'domain'; import * as http from 'http'; import { default as createNextServer } from 'next'; @@ -199,10 +204,14 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { // We only want to record page and API requests if (hasTracingEnabled() && shouldTraceRequest(req.url, publicDirFiles)) { // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) - let traceparentData; - if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractSentrytraceData(req.headers['sentry-trace'] as string); - logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); + // Extract data from trace headers + let sentrytraceData, tracestateData; + if (req.headers?.['sentry-trace']) { + sentrytraceData = extractSentrytraceData(req.headers['sentry-trace'] as string); + logger.log(`[Tracing] Continuing trace ${sentrytraceData?.traceId}.`); + } + if (req.headers?.tracestate) { + tracestateData = extractTracestateData(req.headers.tracestate as string); } // pull off query string, if any @@ -217,7 +226,8 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { name: `${namePrefix}${reqPath}`, op: 'http.server', metadata: { requestPath: reqPath }, - ...traceparentData, + ...sentrytraceData, + ...(tracestateData && { metadata: { tracestate: tracestateData } }), }, // extra context passed to the `tracesSampler` { request: req }, diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index ffb8ad260cd1..382520a5b2fa 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -1,7 +1,7 @@ import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { extractSentrytraceData, hasTracingEnabled } from '@sentry/tracing'; +import { extractSentrytraceData, extractTracestateData, hasTracingEnabled } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; -import { addExceptionMechanism, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; +import { addExceptionMechanism, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import * as domain from 'domain'; import { NextApiHandler, NextApiResponse } from 'next'; @@ -36,11 +36,14 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { currentScope.addEventProcessor(event => parseRequest(event, req)); if (hasTracingEnabled()) { - // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) - let traceparentData; - if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractSentrytraceData(req.headers['sentry-trace'] as string); - logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); + // Extract data from trace headers + let sentrytraceData, tracestateData; + if (req.headers?.['sentry-trace']) { + sentrytraceData = extractSentrytraceData(req.headers['sentry-trace'] as string); + logger.log(`[Tracing] Continuing trace ${sentrytraceData?.traceId}.`); + } + if (req.headers?.tracestate) { + tracestateData = extractTracestateData(req.headers.tracestate as string); } const url = `${req.url}`; @@ -60,7 +63,8 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { { name: `${reqMethod}${reqPath}`, op: 'http.server', - ...traceparentData, + ...sentrytraceData, + ...(tracestateData && { metadata: { tracestate: tracestateData } }), }, // extra context passed to the `tracesSampler` { request: req }, diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 61bf6ab7db5b..1b6852d7c6f3 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ /* eslint-disable @typescript-eslint/no-explicit-any */ import { captureException, getCurrentHub, startTransaction, withScope } from '@sentry/core'; -import { extractSentrytraceData, Span } from '@sentry/tracing'; +import { extractSentrytraceData, extractTracestateData, Span } from '@sentry/tracing'; import { Event, ExtractedNodeRequestData, RequestSessionStatus, Transaction } from '@sentry/types'; import { isPlainObject, isString, logger, normalize, stripUrlQueryAndFragment } from '@sentry/utils'; import * as cookie from 'cookie'; @@ -53,17 +53,21 @@ export function tracingHandler(): ( res: http.ServerResponse, next: (error?: any) => void, ): void { - // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) - let sentrytraceData; - if (req.headers && isString(req.headers['sentry-trace'])) { + // Extract data from trace headers + let sentrytraceData, tracestateData; + if (req.headers?.['sentry-trace']) { sentrytraceData = extractSentrytraceData(req.headers['sentry-trace'] as string); } + if (req.headers?.tracestate) { + tracestateData = extractTracestateData(req.headers.tracestate as string); + } const transaction = startTransaction( { name: extractExpressTransactionName(req, { path: true, method: true }), op: 'http.server', ...sentrytraceData, + ...(tracestateData && { metadata: { tracestate: tracestateData } }), }, // extra context passed to the tracesSampler { request: extractRequestData(req) }, diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 4a2b060c887b..1b788d9441bb 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -325,8 +325,11 @@ describe('tracingHandler', () => { expect(startTransaction).toHaveBeenCalled(); }); - it("pulls parent's data from tracing header on the request", () => { - req.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' }; + it('pulls data from tracing headers on the request', () => { + req.headers = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + tracestate: 'sentry=doGsaREgReaT', + }; sentryTracingMiddleware(req, res, next); @@ -336,6 +339,7 @@ describe('tracingHandler', () => { expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toEqual(false); + expect(transaction.metadata?.tracestate).toEqual({ sentry: 'sentry=doGsaREgReaT' }); }); it('extracts request data for sampling context', () => { diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index fa0a7bb9ec2f..f1879c969efd 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -9,9 +9,9 @@ import { withScope, } from '@sentry/node'; import * as Sentry from '@sentry/node'; -import { extractSentrytraceData } from '@sentry/tracing'; +import { extractSentrytraceData, extractTracestateData } from '@sentry/tracing'; import { Integration } from '@sentry/types'; -import { isString, logger } from '@sentry/utils'; +import { logger } from '@sentry/utils'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil // eslint-disable-next-line import/no-unresolved import { Context, Handler } from 'aws-lambda'; @@ -201,7 +201,7 @@ export function wrapHandler( }; let timeoutWarningTimer: NodeJS.Timeout; - // AWSLambda is like Express. It makes a distinction about handlers based on it's last argument + // AWSLambda is like Express. It makes a distinction about handlers based on its last argument // async (event) => async handler // async (event, context) => async handler // (event, context, callback) => sync handler @@ -252,16 +252,22 @@ export function wrapHandler( }, timeoutWarningDelay); } - // Applying `sentry-trace` to context - let sentrytraceData; + // Extract tracing data from headers + let sentrytraceData, tracestateData; const eventWithHeaders = event as { headers?: { [key: string]: string } }; - if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) { + + if (eventWithHeaders.headers?.['sentry-trace']) { sentrytraceData = extractSentrytraceData(eventWithHeaders.headers['sentry-trace'] as string); } + if (eventWithHeaders.headers?.tracestate) { + tracestateData = extractTracestateData(eventWithHeaders.headers.tracestate as string); + } + const transaction = startTransaction({ name: context.functionName, op: 'awslambda.handler', ...sentrytraceData, + ...(tracestateData && { metadata: { tracestate: tracestateData } }), }); const hub = getCurrentHub(); diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index b6412db1e182..0c0adb3ddc5a 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -1,6 +1,6 @@ import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { extractSentrytraceData } from '@sentry/tracing'; -import { isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; +import { extractSentrytraceData, extractTracestateData } from '@sentry/tracing'; +import { logger, stripUrlQueryAndFragment } from '@sentry/utils'; import { domainify, getActiveDomain, proxyFunction } from './../utils'; import { HttpFunction, WrapperOptions } from './general'; @@ -49,16 +49,22 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); }); + test('incoming trace headers are correctly parsed and used', async () => { + expect.assertions(1); + + fakeEvent.headers = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy', + }; + + const handler: Handler = (_event, _context, callback) => { + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { + tracestate: { + sentry: 'sentry=doGsaREgReaT', + thirdparty: 'maisey=silly,charlie=goofy', + }, + }, + }), + ); + + callback(undefined, { its: 'fine' }); + }; + const wrappedHandler = wrapHandler(handler); + await wrappedHandler(fakeEvent, fakeContext, fakeCallback); + }); + test('capture error', async () => { expect.assertions(10); @@ -278,6 +307,35 @@ describe('AWSLambda', () => { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); }); + test('incoming trace headers are correctly parsed and used', async () => { + expect.assertions(1); + + fakeEvent.headers = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy', + }; + + const handler: Handler = async (_event, _context, callback) => { + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { + tracestate: { + sentry: 'sentry=doGsaREgReaT', + thirdparty: 'maisey=silly,charlie=goofy', + }, + }, + }), + ); + + callback(undefined, { its: 'fine' }); + }; + const wrappedHandler = wrapHandler(handler); + await wrappedHandler(fakeEvent, fakeContext, fakeCallback); + }); + test('capture error', async () => { expect.assertions(10); @@ -328,6 +386,35 @@ describe('AWSLambda', () => { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); }); + test('incoming trace headers are correctly parsed and used', async () => { + expect.assertions(1); + + fakeEvent.headers = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy', + }; + + const handler: Handler = async (_event, _context, callback) => { + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { + tracestate: { + sentry: 'sentry=doGsaREgReaT', + thirdparty: 'maisey=silly,charlie=goofy', + }, + }, + }), + ); + + callback(undefined, { its: 'fine' }); + }; + const wrappedHandler = wrapHandler(handler); + await wrappedHandler(fakeEvent, fakeContext, fakeCallback); + }); + test('capture error', async () => { expect.assertions(10); diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 4b5a00199796..731eb1fcd6de 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -120,6 +120,35 @@ describe('GCPFunction', () => { expect(Sentry.flush).toBeCalledWith(2000); }); + test('incoming trace headers are correctly parsed and used', async () => { + expect.assertions(1); + + const handler: HttpFunction = (_req, res) => { + res.statusCode = 200; + res.end(); + }; + const wrappedHandler = wrapHttpFunction(handler); + const traceHeaders = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + tracestate: 'sentry=doGsaREgReaT,maisey=silly,charlie=goofy', + }; + await handleHttp(wrappedHandler, traceHeaders); + + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { + tracestate: { + sentry: 'sentry=doGsaREgReaT', + thirdparty: 'maisey=silly,charlie=goofy', + }, + }, + }), + ); + }); + test('capture error', async () => { expect.assertions(5); From 9ab5a8ff2fcdd297affb49e619d57e89992b1f9c Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 17:57:52 -0700 Subject: [PATCH 19/28] extract and propagate tracestate data from tags --- .../tracing/src/browser/browsertracing.ts | 24 ++++-- .../test/browser/browsertracing.test.ts | 75 +++++++++++-------- 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 2776702006a1..9a69462f7fce 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -5,7 +5,7 @@ import { getGlobalObject, logger } from '@sentry/utils'; import { startIdleTransaction } from '../hubextensions'; import { DEFAULT_IDLE_TIMEOUT, IdleTransaction } from '../idletransaction'; import { SpanStatus } from '../spanstatus'; -import { extractSentrytraceData, secToMs } from '../utils'; +import { extractSentrytraceData, extractTracestateData, secToMs } from '../utils'; import { registerBackgroundTabDetection } from './backgroundtab'; import { MetricsInstrumentation } from './metrics'; import { @@ -191,7 +191,7 @@ export class BrowserTracing implements Integration { // eslint-disable-next-line @typescript-eslint/unbound-method const { beforeNavigate, idleTimeout, maxTransactionDuration } = this.options; - const parentContextFromHeader = context.op === 'pageload' ? getHeaderContext() : undefined; + const parentContextFromHeader = context.op === 'pageload' ? extractTraceDataFromMetaTags() : undefined; const expandedContext = { ...context, @@ -230,14 +230,22 @@ export class BrowserTracing implements Integration { } /** - * Gets transaction context from a sentry-trace meta. + * Gets transaction context data from `sentry-trace` and `tracestate` tags. * - * @returns Transaction context data from the header or undefined if there's no header or the header is malformed + * @returns Transaction context data or undefined neither tag exists or has valid data */ -export function getHeaderContext(): Partial | undefined { - const header = getMetaContent('sentry-trace'); - if (header) { - return extractSentrytraceData(header); +export function extractTraceDataFromMetaTags(): Partial | undefined { + const sentrytraceValue = getMetaContent('sentry-trace'); + const tracestateValue = getMetaContent('tracestate'); + + const sentrytraceData = sentrytraceValue ? extractSentrytraceData(sentrytraceValue) : undefined; + const tracestateData = tracestateValue ? extractTracestateData(tracestateValue) : undefined; + + if (sentrytraceData || tracestateData?.sentry || tracestateData?.thirdparty) { + return { + ...sentrytraceData, + ...(tracestateData && { metadata: { tracestate: tracestateData } }), + }; } return undefined; diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 7c7d1d6306cc..3501a51f59fe 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -8,7 +8,7 @@ import { BrowserTracing, BrowserTracingOptions, DEFAULT_MAX_TRANSACTION_DURATION_SECONDS, - getHeaderContext, + extractTraceDataFromMetaTags, getMetaContent, } from '../../src/browser/browsertracing'; import { defaultRequestInstrumentationOptions } from '../../src/browser/request'; @@ -209,20 +209,23 @@ describe('BrowserTracing', () => { }); }); - it('sets transaction context from sentry-trace header', () => { - const name = 'sentry-trace'; - const content = '126de09502ae4e0fb26c6967190756a4-b6e54397b12a2a0f-1'; - document.head.innerHTML = ``; + it('sets transaction context from tag data', () => { + document.head.innerHTML = + ` ` + + ``; + const startIdleTransaction = jest.spyOn(hubExtensions, 'startIdleTransaction'); createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting }); expect(startIdleTransaction).toHaveBeenCalledWith( + // because `startIdleTransaction` uses positional arguments, we have to include placeholders for all of them expect.any(Object), expect.objectContaining({ - traceId: '126de09502ae4e0fb26c6967190756a4', - parentSpanId: 'b6e54397b12a2a0f', - parentSampled: true, + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT', thirdparty: 'maisey=silly,charlie=goofy' } }, }), expect.any(Number), expect.any(Boolean), @@ -354,7 +357,7 @@ describe('BrowserTracing', () => { }); }); - describe('sentry-trace element', () => { + describe('sentry-trace/tracestate tags', () => { describe('getMetaContent', () => { it('finds the specified tag and extracts the value', () => { const name = 'sentry-trace'; @@ -384,39 +387,37 @@ describe('BrowserTracing', () => { }); }); - describe('getHeaderContext', () => { - it('correctly parses a valid sentry-trace meta header', () => { - document.head.innerHTML = ``; - - const headerContext = getHeaderContext(); - - expect(headerContext).toBeDefined(); - expect(headerContext!.traceId).toEqual('12312012123120121231201212312012'); - expect(headerContext!.parentSpanId).toEqual('1121201211212012'); - expect(headerContext!.parentSampled).toEqual(false); - }); - - it('returns undefined if the header is malformed', () => { - document.head.innerHTML = ``; + describe('extractTraceDataFromMetaTags', () => { + it('correctly captures both sentry-trace and tracestate data', () => { + document.head.innerHTML = + ` ` + + ``; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); - expect(headerContext).toBeUndefined(); + expect(headerContext).toEqual({ + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT', thirdparty: 'maisey=silly,charlie=goofy' } }, + }); }); - it("returns undefined if the header isn't there", () => { - document.head.innerHTML = ``; + it('returns undefined if neither tag is there', () => { + document.head.innerHTML = ` `; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); expect(headerContext).toBeUndefined(); }); }); - describe('using the data', () => { + describe('using tag data', () => { it('uses the data for pageload transactions', () => { // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one - document.head.innerHTML = ``; + document.head.innerHTML = + ` ` + + ``; // pageload transactions are created as part of the BrowserTracing integration's initialization createBrowserTracing(true); @@ -427,11 +428,20 @@ describe('BrowserTracing', () => { expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toBe(false); + expect(transaction.metadata?.tracestate).toEqual({ + sentry: 'sentry=doGsaREgReaT', + thirdparty: 'maisey=silly,charlie=goofy', + }); }); + // navigation transactions happen on the same page as generated a pageload transaction (with tags still + // possibly present) but they start their own trace and therefore shouldn't contain the pageload transaction's + // trace data it('ignores the data for navigation transactions', () => { mockChangeHistory = () => undefined; - document.head.innerHTML = ``; + document.head.innerHTML = + ` ` + + ``; createBrowserTracing(true); @@ -442,6 +452,9 @@ describe('BrowserTracing', () => { expect(transaction.op).toBe('navigation'); expect(transaction.traceId).not.toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toBeUndefined(); + expect(transaction.sampled).toBe(true); + expect(transaction.metadata?.tracestate?.sentry).not.toEqual('sentry=doGsaREgReaT'); + expect(transaction.metadata?.tracestate?.thirdparty).toBeUndefined(); }); }); }); From 08d9ba4ad04fef233e1766532a44ceabbf93f940 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 18:32:00 -0700 Subject: [PATCH 20/28] add missing tracestate when capturing transaction, if necessary --- packages/tracing/src/transaction.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index 32aefd43dd21..523fec78ccd0 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -116,6 +116,11 @@ export class Transaction extends SpanClass implements TransactionInterface { }).endTimestamp; } + // ensure that we have a tracestate to attach to the envelope header + if (!this.metadata.tracestate?.sentry) { + this.metadata.tracestate = { ...this.metadata.tracestate, sentry: this._getNewTracestate() }; + } + const transaction: Event = { contexts: { trace: this.getTraceContext(), From 5c6070e0245402c6dc2955aeb220f372d80a0193 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 19 Aug 2021 06:36:43 -0700 Subject: [PATCH 21/28] only deal with envelope header data if we're using an envelope --- packages/core/src/request.ts | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index 54197023a2eb..acc5094e6778 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -54,14 +54,6 @@ 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; - } - const req: SentryRequest = { body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event), type: eventType, @@ -75,18 +67,23 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { // deserialization. Instead, we only implement a minimal subset of the spec to // serialize events inline here. if (useEnvelope) { + // Extract header information from event + const { transactionSampling, ...metadata } = event.debug_meta || {}; + if (Object.keys(metadata).length === 0) { + delete event.debug_meta; + } else { + event.debug_meta = metadata; + } + const envelopeHeaders = JSON.stringify({ event_id: event.event_id, sent_at: new Date().toISOString(), ...(sdkInfo && { sdk: sdkInfo }), ...(api.forceEnvelope() && { dsn: api.getDsn().toString() }), }); - 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 @@ -101,7 +98,15 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { // size and to reduce request body size. // // 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); + // The trailing newline is optional. We intentionally don't send it to avoid // sending unnecessary bytes. // From 50819b64c92db4985f7f2e6d2ecaaca39495cda9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 19 Aug 2021 08:18:30 -0700 Subject: [PATCH 22/28] add tracestate to envelope header --- packages/core/src/request.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index acc5094e6778..9e029fbfd5b7 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -1,4 +1,5 @@ import { Event, SdkInfo, SentryRequest, SentryRequestType, Session, SessionAggregates } from '@sentry/types'; +import { base64ToUnicode, logger } from '@sentry/utils'; import { API } from './api'; @@ -68,18 +69,33 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { // serialize events inline here. if (useEnvelope) { // Extract header information from event - const { transactionSampling, ...metadata } = event.debug_meta || {}; + const { transactionSampling, tracestate, ...metadata } = event.debug_meta || {}; if (Object.keys(metadata).length === 0) { delete event.debug_meta; } else { event.debug_meta = metadata; } + // 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); + } + 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 itemHeaderEntries: { [key: string]: unknown } = { From 756e8554eeeba415ebe1aa00c7791fd064d88b4b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 19 Aug 2021 08:06:20 -0700 Subject: [PATCH 23/28] clean up comments --- packages/core/src/request.ts | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index 9e029fbfd5b7..cc5760c712e4 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -61,12 +61,9 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { 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. + // 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 || {}; @@ -101,19 +98,17 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { 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. - // - // content_type: 'application/json', + // Note: as mentioned above, `content_type` and `length` were left out on purpose. // - // 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. + // `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'`.) // - // length: new TextEncoder().encode(req.body).length, + // `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') { @@ -123,10 +118,8 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { const itemHeaders = JSON.stringify(itemHeaderEntries); - // 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`; + // 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${req.body}`; req.body = envelope; } From a0b5cafc2a32b8c9968c1ee85b096ae5448234c4 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 18:32:21 -0700 Subject: [PATCH 24/28] add http header tests --- packages/tracing/test/httpheaders.test.ts | 248 ++++++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 packages/tracing/test/httpheaders.test.ts diff --git a/packages/tracing/test/httpheaders.test.ts b/packages/tracing/test/httpheaders.test.ts new file mode 100644 index 000000000000..c2b0a7774ee8 --- /dev/null +++ b/packages/tracing/test/httpheaders.test.ts @@ -0,0 +1,248 @@ +import * as sentryCore from '@sentry/core'; +import { API } from '@sentry/core'; +import { Hub } from '@sentry/hub'; +import { SentryRequest } from '@sentry/types'; +import * as utilsPackage from '@sentry/utils'; +import { base64ToUnicode } from '@sentry/utils'; + +import { Span } from '../src/span'; +import { Transaction } from '../src/transaction'; +import { computeTracestateValue } from '../src/utils'; + +// TODO gather sentry-trace and tracestate tests here + +function parseEnvelopeRequest(request: SentryRequest): any { + const [envelopeHeaderString, itemHeaderString, eventString] = request.body.split('\n'); + + return { + envelopeHeader: JSON.parse(envelopeHeaderString), + itemHeader: JSON.parse(itemHeaderString), + event: JSON.parse(eventString), + }; +} + +describe('sentry-trace', () => { + // TODO gather relevant tests here +}); + +describe('tracestate', () => { + // grab these this way rather than importing them individually to get around TS's guards against instantiating + // abstract classes (using non-abstract classes would create a circular dependency) + const { BaseClient, BaseBackend } = sentryCore as any; + + const dsn = 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012'; + const environment = 'dogpark'; + const release = 'off.leash.trail'; + const hub = new Hub( + new BaseClient(BaseBackend, { + dsn, + environment, + release, + }), + ); + + describe('sentry tracestate', () => { + describe('lazy creation', () => { + const getNewTracestate = jest + .spyOn(Span.prototype as any, '_getNewTracestate') + .mockReturnValue('sentry=doGsaREgReaT'); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + describe('when creating a transaction', () => { + it('uses sentry tracestate passed to the transaction constructor rather than creating a new one', () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT' } }, + }); + + expect(getNewTracestate).not.toHaveBeenCalled(); + expect(transaction.metadata.tracestate?.sentry).toEqual('sentry=doGsaREgReaT'); + }); + + it("doesn't create new sentry tracestate on transaction creation if none provided", () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + }); + + expect(transaction.metadata.tracestate?.sentry).toBeUndefined(); + expect(getNewTracestate).not.toHaveBeenCalled(); + }); + }); + + describe('when getting outgoing request headers', () => { + it('uses existing sentry tracestate when getting tracing headers rather than creating new one', () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT' } }, + }); + + expect(transaction.getTraceHeaders().tracestate).toEqual('sentry=doGsaREgReaT'); + expect(getNewTracestate).not.toHaveBeenCalled(); + }); + + it('creates and stores new sentry tracestate when getting tracing headers if none exists', () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + }); + + expect(transaction.metadata.tracestate?.sentry).toBeUndefined(); + + transaction.getTraceHeaders(); + + expect(getNewTracestate).toHaveBeenCalled(); + expect(transaction.metadata.tracestate?.sentry).toEqual('sentry=doGsaREgReaT'); + expect(transaction.getTraceHeaders().tracestate).toEqual('sentry=doGsaREgReaT'); + }); + }); + + describe('when getting envelope headers', () => { + // In real life, `transaction.finish()` calls `captureEvent()`, which eventually calls `eventToSentryRequest()`, + // which in turn calls `base64ToUnicode`. Here we're short circuiting that process a little, to avoid having to + // mock out more of the intermediate pieces. + jest + .spyOn(utilsPackage, 'base64ToUnicode') + .mockImplementation(base64 => + base64 === 'doGsaREgReaT' ? '{"all the":"right stuff here"}' : '{"nope nope nope":"wrong"}', + ); + jest.spyOn(hub, 'captureEvent').mockImplementation(event => { + expect(event).toEqual( + expect.objectContaining({ debug_meta: { tracestate: { sentry: 'sentry=doGsaREgReaT' } } }), + ); + + const envelope = parseEnvelopeRequest(sentryCore.eventToSentryRequest(event, new API(dsn))); + expect(envelope.envelopeHeader).toEqual( + expect.objectContaining({ trace: { 'all the': 'right stuff here' } }), + ); + + // `captureEvent` normally returns the event id + return '11212012041520131231201209082013'; // + }); + + it('uses existing sentry tracestate in envelope headers rather than creating a new one', () => { + // one here, and two inside the `captureEvent` implementation above + expect.assertions(3); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT' } }, + sampled: true, + }, + hub, + ); + + transaction.finish(); + + expect(getNewTracestate).not.toHaveBeenCalled(); + }); + + it('creates new sentry tracestate for envelope header if none exists', () => { + // two here, and two inside the `captureEvent` implementation above + expect.assertions(4); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + sampled: true, + }, + hub, + ); + + expect(transaction.metadata.tracestate?.sentry).toBeUndefined(); + + transaction.finish(); + + expect(getNewTracestate).toHaveBeenCalled(); + }); + }); + }); + + describe('mutibility', () => { + it("won't include data set after transaction is created if there's an inherited value", () => { + expect.assertions(1); + + const inheritedTracestate = `sentry=${computeTracestateValue({ + trace_id: '12312012090820131231201209082013', + environment: 'dogpark', + release: 'off.leash.trail', + public_key: 'dogsarebadatkeepingsecrets', + })}`; + + const transaction = new Transaction( + { + name: 'FETCH /ball', + metadata: { + tracestate: { + sentry: inheritedTracestate, + }, + }, + }, + hub, + ); + + hub.withScope(scope => { + scope.setUser({ id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }); + + const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', ''); + const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue)); + + expect(reinflatedTracestate.user).toBeUndefined(); + }); + }); + + it("will include data set after transaction is created if there's no inherited value and `getTraceHeaders` hasn't been called", () => { + expect.assertions(2); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + }, + hub, + ); + + hub.withScope(scope => { + scope.setUser({ id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }); + + const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', ''); + const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue)); + + expect(reinflatedTracestate.user.id).toEqual('1121'); + expect(reinflatedTracestate.user.segment).toEqual('bigs'); + }); + }); + + it("won't include data set after first call to `getTraceHeaders`", () => { + expect.assertions(1); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + }, + hub, + ); + + transaction.getTraceHeaders(); + + hub.withScope(scope => { + scope.setUser({ id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }); + + const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', ''); + const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue)); + + expect(reinflatedTracestate.user).toBeUndefined(); + }); + }); + }); + }); + + describe('third-party tracestate', () => { + // TODO gather relevant tests here + }); +}); From f1a0dbf2ebe3155357364fb985c9aa7ae2270251 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 18 Aug 2021 18:40:35 -0700 Subject: [PATCH 25/28] random cleanup --- .../instance-initializers/sentry-performance.ts | 4 ++-- packages/tracing/src/transaction.ts | 14 ++++++-------- packages/utils/src/misc.ts | 1 + 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 81437b698419..113c4a272535 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -28,14 +28,14 @@ export function initialize(appInstance: ApplicationInstance): void { } } -function getBackburner() { +function getBackburner(): typeof _backburner { if (run.backburner) { return run.backburner; } return _backburner; } -function getTransitionInformation(transition: any, router: any) { +function getTransitionInformation(transition: any, router: any): { [key: string]: any } { const fromRoute = transition?.from?.name; const toRoute = transition && transition.to ? transition.to.name : router.currentRouteName; return { diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index 523fec78ccd0..a6ac1b49db41 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -6,7 +6,7 @@ import { TransactionContext, TransactionMetadata, } from '@sentry/types'; -import { dropUndefinedKeys, isInstanceOf, logger } from '@sentry/utils'; +import { dropUndefinedKeys, logger } from '@sentry/utils'; import { Span as SpanClass, SpanRecorder } from './span'; @@ -21,7 +21,7 @@ export class Transaction extends SpanClass implements TransactionInterface { /** * The reference to the current hub. */ - private readonly _hub: Hub = (getCurrentHub() as unknown) as Hub; + private readonly _hub: Hub; private _trimEnd?: boolean; @@ -35,16 +35,14 @@ export class Transaction extends SpanClass implements TransactionInterface { public constructor(transactionContext: TransactionContext, hub?: Hub) { super(transactionContext); - if (isInstanceOf(hub, Hub)) { - this._hub = hub as Hub; - } - this.name = transactionContext.name || ''; - this.metadata = transactionContext.metadata || {}; this._trimEnd = transactionContext.trimEnd; + this._hub = hub || getCurrentHub(); - // this is because transactions are also spans, and spans have a transaction pointer + // this is because transactions are also spans, and spans have a transaction pointer (it doesn't get set in `super` + // because theoretically you can create a span without a transaction, though at the moment it doesn't do you much + // good) this.transaction = this; } diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index 766f9231af67..aa57111ea8cf 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -217,6 +217,7 @@ interface SemVer { /** * Parses input into a SemVer interface * @param input string representation of a semver version + * @returns A object containing each part of the version string as a separate entry */ export function parseSemver(input: string): SemVer { const match = input.match(SEMVER_REGEXP) || []; From 6415a5afd0789f209d1b5e9c2b30c6f08a0b1f96 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 19 Aug 2021 14:18:23 -0700 Subject: [PATCH 26/28] don't stringify event data until the end --- packages/core/src/request.ts | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index cc5760c712e4..c7f478a9d581 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -13,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. */ @@ -55,11 +56,7 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { const eventType = event.type || 'event'; const useEnvelope = eventType === 'transaction' || api.forceEnvelope(); - const req: SentryRequest = { - body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event), - type: eventType, - url: useEnvelope ? api.getEnvelopeEndpointWithUrlEncodedAuth() : api.getStoreEndpointWithUrlEncodedAuth(), - }; + enhanceEventWithSdkInfo(event, api.metadata.sdk); // 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 @@ -118,11 +115,22 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest { 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${req.body}`; - req.body = envelope; + 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(), + }; } From 15a2e85f21e247c931a1d2b9098348a787a7b56c Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 19 Aug 2021 14:20:55 -0700 Subject: [PATCH 27/28] rework and add request tests --- packages/core/test/lib/request.test.ts | 453 ++++++++++++++++--------- 1 file changed, 293 insertions(+), 160 deletions(-) diff --git a/packages/core/test/lib/request.test.ts b/packages/core/test/lib/request.test.ts index 3419874d1a97..84da112ea92c 100644 --- a/packages/core/test/lib/request.test.ts +++ b/packages/core/test/lib/request.test.ts @@ -1,20 +1,27 @@ -import { DebugMeta, Event, SentryRequest, TransactionSamplingMethod } from '@sentry/types'; +import { Event, SentryRequest, Session, TransactionSamplingMethod } from '@sentry/types'; import { API } from '../../src/api'; import { eventToSentryRequest, sessionToSentryRequest } from '../../src/request'; -const ingestDsn = 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012'; -const ingestUrl = +const timestamp = '2012-12-31T09:08:13.000Z'; +jest.spyOn(Date.prototype, 'toISOString').mockReturnValue(timestamp); + +const squirrelChasersDSN = 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012'; +const storeUrl = + 'https://squirrelchasers.ingest.sentry.io/api/12312012/store/?sentry_key=dogsarebadatkeepingsecrets&sentry_version=7'; +const envelopeUrl = 'https://squirrelchasers.ingest.sentry.io/api/12312012/envelope/?sentry_key=dogsarebadatkeepingsecrets&sentry_version=7'; -const tunnel = 'https://hello.com/world'; - -const api = new API(ingestDsn, { - sdk: { - integrations: ['AWSLambda'], - name: 'sentry.javascript.browser', - version: `12.31.12`, - packages: [{ name: 'npm:@sentry/browser', version: `12.31.12` }], - }, +const tunnelUrl = 'https://sit.stay.rollover/good/dog/'; + +const sdkInfo = { + integrations: ['AWSLambda'], + name: 'sentry.javascript.browser', + version: `12.31.12`, + packages: [{ name: 'npm:@sentry/browser', version: `12.31.12` }], +}; + +const api = new API(squirrelChasersDSN, { + sdk: sdkInfo, }); function parseEnvelopeRequest(request: SentryRequest): any { @@ -28,186 +35,312 @@ function parseEnvelopeRequest(request: SentryRequest): any { } describe('eventToSentryRequest', () => { - let event: Event; + const eventBase = { + contexts: { trace: { trace_id: '1231201211212012', span_id: '12261980', op: 'pageload' } }, + environment: 'dogpark', + event_id: '0908201304152013', + release: 'off.leash.park', + user: { id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }, + }; - beforeEach(() => { - event = { - contexts: { trace: { trace_id: '1231201211212012', span_id: '12261980', op: 'pageload' } }, - environment: 'dogpark', - event_id: '0908201304152013', - release: 'off.leash.park', - spans: [], - transaction: '/dogs/are/great/', - type: 'transaction', - user: { id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12' }, - }; - }); + describe('error/message events', () => { + let errorEvent: Event; - it('adds transaction sampling information to item header', () => { - event.debug_meta = { transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 } }; + beforeEach(() => { + errorEvent = { + ...eventBase, + exception: { values: [{ type: 'PuppyProblemsError', value: 'Charlie ate the flip-flops :-(' }] }, + }; + }); - const result = eventToSentryRequest(event, api); - const envelope = parseEnvelopeRequest(result); + it('adds correct type, url, and event data', () => { + const result = eventToSentryRequest(errorEvent, api); - expect(envelope.itemHeader).toEqual( - expect.objectContaining({ - sample_rates: [{ id: TransactionSamplingMethod.Rate, rate: 0.1121 }], - }), - ); - }); + expect(result.type).toEqual('event'); + expect(result.url).toEqual(storeUrl); + expect(result.body).toEqual(JSON.stringify(errorEvent)); + }); - it('removes transaction sampling information (and only that) from debug_meta', () => { - event.debug_meta = { - transactionSampling: { method: TransactionSamplingMethod.Sampler, rate: 0.1121 }, - dog: 'Charlie', - } as DebugMeta; + it('uses an envelope if a tunnel is configured', () => { + const result = eventToSentryRequest(errorEvent, new API(squirrelChasersDSN, {}, tunnelUrl)); - const result = eventToSentryRequest(event, api); - const envelope = parseEnvelopeRequest(result); - - expect('transactionSampling' in envelope.event.debug_meta).toBe(false); - expect('dog' in envelope.event.debug_meta).toBe(true); + expect(() => { + // this will barf if the request body isn't of the form "\n\n" + parseEnvelopeRequest(result); + }).not.toThrowError(); + }); }); - it('removes debug_meta entirely if it ends up empty', () => { - event.debug_meta = { - transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 }, - } as DebugMeta; - - const result = eventToSentryRequest(event, api); - const envelope = parseEnvelopeRequest(result); - - expect('debug_meta' in envelope.event).toBe(false); + describe('transaction events', () => { + let transactionEvent: Event; + + beforeEach(() => { + transactionEvent = { + ...eventBase, + debug_meta: { + transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 }, + // This value is hardcoded in its base64 form to avoid a dependency on @sentry/tracing, where the method to + // compute the value lives. It's equivalent to + // computeTracestateValue({ + // trace_id: '1231201211212012', + // environment: 'dogpark', + // public_key: 'dogsarebadatkeepingsecrets', + // release: 'off.leash.park', + // user: { id: '1121', segment: 'bigs' }, + // }), + tracestate: { + sentry: + 'sentry=eyJ0cmFjZV9pZCI6IjEyMzEyMDEyMTEyMTIwMTIiLCJlbnZpcm9ubWVudCI6ImRvZ3BhcmsiLCJwdWJsaWNfa2V5Ijo' + + 'iZG9nc2FyZWJhZGF0a2VlcGluZ3NlY3JldHMiLCJyZWxlYXNlIjoib2ZmLmxlYXNoLnBhcmsiLCJ1c2VyIjp7ImlkIjoiMTEyM' + + 'SIsInNlZ21lbnQiOiJiaWdzIn19', + }, + }, + spans: [], + transaction: '/dogs/are/great/', + type: 'transaction', + }; + }); + + it('adds correct type, url, and event data', () => { + const result = eventToSentryRequest(transactionEvent, api); + const envelope = parseEnvelopeRequest(result); + + expect(result.type).toEqual('transaction'); + expect(result.url).toEqual(envelopeUrl); + expect(envelope.event).toEqual(transactionEvent); + }); + + describe('envelope header', () => { + it('adds correct entries to envelope header', () => { + const result = eventToSentryRequest(transactionEvent, api); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.envelopeHeader).toEqual({ + event_id: eventBase.event_id, + sent_at: timestamp, + sdk: { name: 'sentry.javascript.browser', version: '12.31.12' }, + // the value for `trace` is tested separately + trace: expect.any(Object), + }); + }); + + it('adds tracestate data to envelope header', () => { + const result = eventToSentryRequest(transactionEvent, api); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.envelopeHeader.trace).toBeDefined(); + expect(envelope.envelopeHeader.trace).toEqual({ + trace_id: '1231201211212012', + environment: 'dogpark', + public_key: 'dogsarebadatkeepingsecrets', + release: 'off.leash.park', + user: { id: '1121', segment: 'bigs' }, + }); + }); + }); + + describe('item header', () => { + it('adds correct entries to item header', () => { + const result = eventToSentryRequest(transactionEvent, api); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.itemHeader).toEqual({ + type: 'transaction', + // the value for `sample_rates` is tested separately + sample_rates: expect.any(Object), + }); + }); + + it('adds transaction sampling information to item header', () => { + const result = eventToSentryRequest(transactionEvent, api); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.itemHeader).toEqual( + expect.objectContaining({ + sample_rates: [{ id: TransactionSamplingMethod.Rate, rate: 0.1121 }], + }), + ); + }); + }); + + describe('debug_meta', () => { + it('removes transaction sampling and tracestate information from debug_meta', () => { + (transactionEvent.debug_meta as any).dog = 'Charlie'; + + const result = eventToSentryRequest(transactionEvent, api); + const envelope = parseEnvelopeRequest(result); + + expect('transactionSampling' in envelope.event.debug_meta).toBe(false); + expect('tracestate' in envelope.event.debug_meta).toBe(false); + expect('dog' in envelope.event.debug_meta).toBe(true); + }); + + it('removes debug_meta entirely if it ends up empty', () => { + const result = eventToSentryRequest(transactionEvent, api); + const envelope = parseEnvelopeRequest(result); + + expect('debug_meta' in envelope.event).toBe(false); + }); + }); }); - it('adds sdk info to envelope header', () => { - const result = eventToSentryRequest(event, api); - const envelope = parseEnvelopeRequest(result); - - expect(envelope.envelopeHeader).toEqual( - expect.objectContaining({ sdk: { name: 'sentry.javascript.browser', version: '12.31.12' } }), - ); - }); + describe('SDK metadata', () => { + it('adds sdk info to event body', () => { + const event = { ...eventBase }; + const result = eventToSentryRequest(event, api); - it('adds sdk info to event body', () => { - const result = eventToSentryRequest(event, api); - const envelope = parseEnvelopeRequest(result); + expect(JSON.parse(result.body).sdk).toEqual(sdkInfo); + }); - expect(envelope.event).toEqual( - expect.objectContaining({ + it('merges sdk info if sdk data already exists on the event body', () => { + const event = { + ...eventBase, sdk: { - integrations: ['AWSLambda'], - name: 'sentry.javascript.browser', - version: `12.31.12`, - packages: [{ name: 'npm:@sentry/browser', version: `12.31.12` }], + integrations: ['Ball Fetching'], + name: 'sentry.dog.tricks', + packages: [{ name: 'npm:@sentry/dogtricks', version: `11.21.12` }], + version: '11.21.12', }, - }), - ); + }; + + const result = eventToSentryRequest(event, api); + + expect(JSON.parse(result.body).sdk).toEqual({ + integrations: ['Ball Fetching', 'AWSLambda'], + name: 'sentry.dog.tricks', + packages: [ + { name: 'npm:@sentry/dogtricks', version: `11.21.12` }, + { name: 'npm:@sentry/browser', version: `12.31.12` }, + ], + version: '11.21.12', + }); + }); }); - it('merges existing sdk info if one is present on the event body', () => { - event.sdk = { - integrations: ['Clojure'], - name: 'foo', - packages: [{ name: 'npm:@sentry/clj', version: `12.31.12` }], - version: '1337', - }; - - const result = eventToSentryRequest(event, api); - const envelope = parseEnvelopeRequest(result); - - expect(envelope.event).toEqual( - expect.objectContaining({ - sdk: { - integrations: ['Clojure', 'AWSLambda'], - name: 'foo', - packages: [ - { name: 'npm:@sentry/clj', version: `12.31.12` }, - { name: 'npm:@sentry/browser', version: `12.31.12` }, - ], - version: '1337', - }, - }), - ); + describe('using a tunnel', () => { + let event: Event; + + beforeEach(() => { + event = { ...eventBase }; + }); + + it('uses the tunnel URL', () => { + const tunnelRequest = eventToSentryRequest(event, new API(squirrelChasersDSN, {}, tunnelUrl)); + expect(tunnelRequest.url).toEqual(tunnelUrl); + }); + + it('adds dsn to envelope header', () => { + const result = eventToSentryRequest(event, new API(squirrelChasersDSN, {}, tunnelUrl)); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.envelopeHeader).toEqual( + expect.objectContaining({ + dsn: squirrelChasersDSN, + }), + ); + }); + + it('defaults `type` to "event" in item header if `type` missing from event', () => { + const result = eventToSentryRequest(event, new API(squirrelChasersDSN, {}, tunnelUrl)); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.itemHeader).toEqual( + expect.objectContaining({ + type: 'event', + }), + ); + }); + + it('uses `type` value from event in item header if present', () => { + // this is obviously not a valid event type, but the only valid one is "transaction", and that requires a whole + // bunch of metadata which is beside the point here + (event as any).type = 'dogEvent'; + + const result = eventToSentryRequest(event, new API(squirrelChasersDSN, {}, tunnelUrl)); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.itemHeader).toEqual( + expect.objectContaining({ + type: 'dogEvent', + }), + ); + }); }); +}); - it('uses tunnel as the url if it is configured', () => { - const tunnelRequest = eventToSentryRequest(event, new API(ingestDsn, {}, tunnel)); - expect(tunnelRequest.url).toEqual(tunnel); +describe('sessionToSentryRequest', () => { + // { "sent_at": "2021-08-19T20:47:04.474Z", "sdk": { "name": "sentry.javascript.browser", "version": "6.11.0" } } + // {"type":"session"} + const sessionEvent = ({ + sid: '0908201304152013', + init: true, + started: timestamp, + timestamp, + status: 'ok', + errors: 0, + attrs: { + release: 'off.leash.park', + environment: 'dogpark', + }, + } as unknown) as Session; - const defaultRequest = eventToSentryRequest(event, new API(ingestDsn, {})); - expect(defaultRequest.url).toEqual(ingestUrl); - }); + const sessionAggregatesEvent = { + attrs: { release: 'off.leash.park', environment: 'dogpark' }, + aggregates: [{ started: timestamp, exited: 2 }], + }; - it('adds dsn to envelope header if tunnel is configured', () => { - const result = eventToSentryRequest(event, new API(ingestDsn, {}, tunnel)); + it('adds correct type, url, and event data to session events', () => { + const result = sessionToSentryRequest(sessionEvent, api); const envelope = parseEnvelopeRequest(result); - expect(envelope.envelopeHeader).toEqual( - expect.objectContaining({ - dsn: ingestDsn, - }), - ); + expect(result.type).toEqual('session'); + expect(result.url).toEqual(envelopeUrl); + expect(envelope.event).toEqual(sessionEvent); }); - it('adds default "event" item type to item header if tunnel is configured', () => { - delete event.type; - - const result = eventToSentryRequest(event, new API(ingestDsn, {}, tunnel)); + it('adds correct type, url, and event data to session aggregates events', () => { + const result = sessionToSentryRequest(sessionAggregatesEvent, api); const envelope = parseEnvelopeRequest(result); - expect(envelope.itemHeader).toEqual( - expect.objectContaining({ - type: 'event', - }), - ); + expect(result.type).toEqual('sessions'); + expect(result.url).toEqual(envelopeUrl); + expect(envelope.event).toEqual(sessionAggregatesEvent); }); -}); -describe('sessionToSentryRequest', () => { - it('test envelope creation for aggregateSessions', () => { - const aggregatedSession = { - attrs: { release: '1.0.x', environment: 'prod' }, - aggregates: [{ started: '2021-04-08T12:18:00.000Z', exited: 2 }], - }; - const result = sessionToSentryRequest(aggregatedSession, api); - - const [envelopeHeaderString, itemHeaderString, sessionString] = result.body.split('\n'); - - expect(JSON.parse(envelopeHeaderString)).toEqual( - expect.objectContaining({ - sdk: { name: 'sentry.javascript.browser', version: '12.31.12' }, - }), - ); - expect(JSON.parse(itemHeaderString)).toEqual( - expect.objectContaining({ - type: 'sessions', - }), - ); - expect(JSON.parse(sessionString)).toEqual( - expect.objectContaining({ - attrs: { release: '1.0.x', environment: 'prod' }, - aggregates: [{ started: '2021-04-08T12:18:00.000Z', exited: 2 }], - }), - ); - }); - - it('uses tunnel as the url if it is configured', () => { - const tunnelRequest = sessionToSentryRequest({ aggregates: [] }, new API(ingestDsn, {}, tunnel)); - expect(tunnelRequest.url).toEqual(tunnel); + it('adds correct entries to envelope header', () => { + const result = sessionToSentryRequest(sessionEvent, api); + const envelope = parseEnvelopeRequest(result); - const defaultRequest = sessionToSentryRequest({ aggregates: [] }, new API(ingestDsn, {})); - expect(defaultRequest.url).toEqual(ingestUrl); + expect(envelope.envelopeHeader).toEqual({ + sent_at: timestamp, + sdk: { name: 'sentry.javascript.browser', version: '12.31.12' }, + }); }); - it('adds dsn to envelope header if tunnel is configured', () => { - const result = sessionToSentryRequest({ aggregates: [] }, new API(ingestDsn, {}, tunnel)); + it('adds correct entries to item header', () => { + const result = sessionToSentryRequest(sessionEvent, api); const envelope = parseEnvelopeRequest(result); - expect(envelope.envelopeHeader).toEqual( - expect.objectContaining({ - dsn: ingestDsn, - }), - ); + expect(envelope.itemHeader).toEqual({ + type: 'session', + }); + }); + + describe('using a tunnel', () => { + it('uses the tunnel URL', () => { + const tunnelRequest = sessionToSentryRequest(sessionEvent, new API(squirrelChasersDSN, {}, tunnelUrl)); + expect(tunnelRequest.url).toEqual(tunnelUrl); + }); + + it('adds dsn to envelope header', () => { + const result = sessionToSentryRequest(sessionEvent, new API(squirrelChasersDSN, {}, tunnelUrl)); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.envelopeHeader).toEqual( + expect.objectContaining({ + dsn: squirrelChasersDSN, + }), + ); + }); }); }); From dd768401dcd82947b91cd30286cbd997e22ab5d7 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 20 Aug 2021 09:57:47 -0700 Subject: [PATCH 28/28] remove redundant string test, add browser test for non-base64 string --- packages/browser/test/unit/string.test.ts | 15 +++++---------- packages/utils/test/string.test.ts | 7 +------ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/packages/browser/test/unit/string.test.ts b/packages/browser/test/unit/string.test.ts index 56119c19a906..601a5ab18cca 100644 --- a/packages/browser/test/unit/string.test.ts +++ b/packages/browser/test/unit/string.test.ts @@ -17,16 +17,11 @@ describe('base64ToUnicode/unicodeToBase64', () => { expect(BASE64_REGEX.test(unicodeToBase64(unicodeString))).to.be.true; }); - it('works as expected', () => { + it('works as expected (and conversion functions are inverses)', () => { expect(unicodeToBase64(unicodeString)).to.equal(base64String); expect(base64ToUnicode(base64String)).to.equal(unicodeString); }); - it('conversion functions are inverses', () => { - expect(base64ToUnicode(unicodeToBase64(unicodeString))).to.equal(unicodeString); - expect(unicodeToBase64(base64ToUnicode(base64String))).to.equal(base64String); - }); - 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(() => { @@ -56,9 +51,9 @@ describe('base64ToUnicode/unicodeToBase64', () => { expect(() => { base64ToUnicode({} as any); }).to.throw('Unable to convert from base64'); - - // Note that by design, in node base64 encoding and decoding will accept any string, whether or not it's valid - // base64, by ignoring all invalid characters, including whitespace. Therefore, no wacky strings have been included - // here because they don't actually error. + expect(() => { + // the exclamation point makes this invalid base64 + base64ToUnicode('Dogs are great!'); + }).to.throw('Unable to convert from base64'); }); }); diff --git a/packages/utils/test/string.test.ts b/packages/utils/test/string.test.ts index fe8bfda35481..8d78f052fef7 100644 --- a/packages/utils/test/string.test.ts +++ b/packages/utils/test/string.test.ts @@ -61,16 +61,11 @@ describe('base64ToUnicode/unicodeToBase64', () => { expect(BASE64_REGEX.test(unicodeToBase64(unicodeString))).toBe(true); }); - test('works as expected', () => { + test('works as expected (and conversion functions are inverses)', () => { expect(unicodeToBase64(unicodeString)).toEqual(base64String); expect(base64ToUnicode(base64String)).toEqual(unicodeString); }); - test('conversion functions are inverses', () => { - expect(base64ToUnicode(unicodeToBase64(unicodeString))).toEqual(unicodeString); - expect(unicodeToBase64(base64ToUnicode(base64String))).toEqual(base64String); - }); - test('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(() => {