Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(envelopes): Add SDK metadata to envelope header#3170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
9484d005ac7eb2417d7c4bea4af09c2992f47970aa7059b842f19b32254fc2c8eb52a3File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { getGlobalObject } from '@sentry/utils'; | ||
| import * as Sentry from '../src/index'; | ||
| const global = getGlobalObject<Window>(); | ||
| describe('global SDK metadata', () => { | ||
| it('sets correct SDK data', () => { | ||
| // the SDK data is set when we import from (and therefore run) `../src/index.ts` - it sets the angular part itself, | ||
| // and the browser part gets set when it imports from @sentry/browser - so no action is necessary here before we run | ||
| // the `expect`s | ||
| expect(global.__SENTRY__?.sdkInfo).toBeDefined(); | ||
| expect(global.__SENTRY__?.sdkInfo?.name).toEqual('sentry.javascript.angular'); | ||
| expect(global.__SENTRY__?.sdkInfo?.version).toEqual(Sentry.SDK_VERSION); | ||
| expect(global.__SENTRY__?.sdkInfo?.packages).toEqual( | ||
| expect.arrayContaining([ | ||
| { name: 'npm:@sentry/angular', version: Sentry.SDK_VERSION }, | ||
| { name: 'npm:@sentry/browser', version: Sentry.SDK_VERSION }, | ||
| ]), | ||
| ); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| { | ||
| "extends": "./tsconfig.build.json", | ||
| "include": ["src/**/*.ts"], | ||
| "include": ["src/**/*.ts", "test/**/*.ts"], | ||
| "exclude": ["dist"], | ||
| "compilerOptions": { | ||
| "rootDir": "." | ||
| "rootDir": ".", | ||
| "types": ["jest"] | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| import { Event, SentryRequest, Session } from '@sentry/types'; | ||
| import { getGlobalObject } from '@sentry/utils'; | ||
| import { API } from './api'; | ||
| /** Creates a SentryRequest from an event. */ | ||
| export function sessionToSentryRequest(session: Session, api: API): SentryRequest { | ||
| const { name, version } = getGlobalObject().__SENTRY__?.sdkInfo || {}; | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is better design to pass dependencies explicitly instead of using the global namespace. What Kamil keeps saying about pure functions and avoiding side-effects ;) Functions like | ||
| const envelopeHeaders = JSON.stringify({ | ||
| sent_at: new Date().toISOString(), | ||
| sdk: { name, version }, | ||
| }); | ||
| const itemHeaders = JSON.stringify({ | ||
| type: 'session', | ||
| @@ -39,9 +42,12 @@ 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) { | ||
| const { name, version } = getGlobalObject().__SENTRY__?.sdkInfo || {}; | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does Relay actually accept sending an | ||
| const envelopeHeaders = JSON.stringify({ | ||
| event_id: event.event_id, | ||
| sent_at: new Date().toISOString(), | ||
| sdk: { name, version }, | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For events, we could take this info off Using | ||
| }); | ||
| const itemHeaders = JSON.stringify({ | ||
| type: event.type, | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { module, test } from 'qunit'; | ||
| import * as Sentry from '@sentry/ember'; | ||
| import { getGlobalObject } from '@sentry/utils'; | ||
| const global = getGlobalObject<Window>(); | ||
| module('Unit | SDK initialization', function() { | ||
| test('adds SDK metadata globally', function(assert) { | ||
| // the SDK data is set when we import from @sentry/ember (and therefore run `addon/index.ts`) - it sets the ember | ||
| // part itself, and the browser part gets set when it imports from @sentry/browser - so no action is necessary here | ||
| // before we run the `assert`s | ||
| assert.equal(global.__SENTRY__?.sdkInfo?.name, 'sentry.javascript.ember'); | ||
| assert.equal(global.__SENTRY__?.sdkInfo?.version, Sentry.SDK_VERSION); | ||
| assert.deepEqual(global.__SENTRY__?.sdkInfo?.packages, [ | ||
| { name: 'npm:@sentry/browser', version: Sentry.SDK_VERSION }, | ||
| { name: 'npm:@sentry/ember', version: Sentry.SDK_VERSION }, | ||
| ]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { getGlobalObject } from '@sentry/utils'; | ||
| import * as Sentry from '../src/index'; | ||
| const global = getGlobalObject<Window>(); | ||
| describe('event SDK info', () => { | ||
| it('adds SDK metadata globally', () => { | ||
| // the SDK data is set when we import from (and therefore run) `../src/index.ts` - it sets the react part itself, | ||
| // and the browser part gets set when it imports from @sentry/browser - so no action is necessary here before we run | ||
| // the `expect`s | ||
| expect(global.__SENTRY__?.sdkInfo).toBeDefined(); | ||
| expect(global.__SENTRY__?.sdkInfo?.name).toEqual('sentry.javascript.react'); | ||
| expect(global.__SENTRY__?.sdkInfo?.version).toEqual(Sentry.SDK_VERSION); | ||
| expect(global.__SENTRY__?.sdkInfo?.packages).toEqual( | ||
| expect.arrayContaining([ | ||
| { name: 'npm:@sentry/react', version: Sentry.SDK_VERSION }, | ||
| { name: 'npm:@sentry/browser', version: Sentry.SDK_VERSION }, | ||
| ]), | ||
| ); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks unrelated to the subject. If that's the case, can we move it off to a separate PR for clarity?