Skip to content

Conversation

@kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Jan 15, 2021

Still need some tinkering and tests.

  • allow for passing options._metadata which is type of SdkMetadata = { sdk?: SdkInfo }
  • this metadata is then attached to options.transportOptions internally and passed to the transport
  • transport is using this metadata when instantiating new API which now has the ability to read this data
    - SDK_VERSION is fixed and always read from @sentry/core
  • SDK_VERSION is moved to @sentry/core
  • SDK_NAME is kept where it was for backwards compatibility and @sentry/browser and @sentry/node still use it as before, so it can be used as fallback for "unknown" SDKs or in a fail-safe scenario
    - event processors are still in place, as we use them to enhance packagesattribute, as well as we have to modify the event itself, which is not using envelopes yet

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.16 KB (+0.81% 🔺)
@sentry/browser - Webpack 20.97 KB (+0.66% 🔺)
@sentry/react - Webpack 20.99 KB (+0.78% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 27.31 KB (+0.59% 🔺)

@rhcarvalho
Copy link
Contributor

For posteriority, this is related to #3170, as it would simplify the implementation of that one.

Copy link
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

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

Oh I like that eventually we can have the version defined in a single place ❤️

@lobsterkatie
Copy link
Member

lobsterkatie commented Jan 16, 2021

For posteriority, this is related to #3170, as it would simplify the implementation of that one.

Indeed. @kamilogorek, even though this replaces that one, you might consider grabbing its tests (which you'd only have to modify a little) since they're already written. (I missed gatsby entirely in that PR, but fortunately the test there already exists and only needs to be updated.)

const sdkInfo = getSdkInfoFromApiMetadata(api);
const envelopeHeaders = JSON.stringify({
sent_at: new Date().toISOString(),
...(sdkInfo && { sdk: sdkInfo }),
Copy link
Member

Choose a reason for hiding this comment

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

We only want name and version here not everything.

const envelopeHeaders = JSON.stringify({
event_id: event.event_id,
sent_at: new Date().toISOString(),
...(sdkInfo && { sdk: sdkInfo }),
Copy link
Member

Choose a reason for hiding this comment

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

Same here: We only want name and version here, not everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants