Skip to content

feat: Add helper for old api to guide users to startTransaction #2625

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

Closed
wants to merge 2 commits into from

Conversation

HazAT
Copy link
Member

@HazAT HazAT commented May 29, 2020

No description provided.

@HazAT HazAT requested a review from kamilogorek as a code owner May 29, 2020 07:19
@HazAT HazAT self-assigned this May 29, 2020
@getsentry-bot
Copy link
Contributor

getsentry-bot commented May 29, 2020

Warnings
⚠️ Please add a changelog entry for your changes.
Messages
📖

@sentry/browser bundle gzip'ed minified size: (ES5: 16.7676 kB) (ES6: 15.8037 kB)

📖 ✅ TSLint passed

Generated by 🚫 dangerJS against ad0a452

Comment on lines +31 to +32
* @deprecated
* We have this here as a fallback to not break users upgrading to the new API
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work in the middle of the func body? Did you mean to put if above the func signature?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's more a marker for us once we have a major bump to delete this block and document the breaking change.

`Please use \`Sentry.startTransaction({name: ${(context as any).transaction}})\` to start a Transaction.`,
);
(context as TransactionContext).name = (context as any).transaction as string;
(context as TransactionContext)._isTransaction = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I may not have fully understood this from previous PRs. What does it mean for a TransactionContext to be a Transaction (_isTransaction = true)?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have another safeguard using when people call
Sentry.startTransaction

https://github.com/getsentry/sentry-javascript/blob/master/packages/minimal/src/index.ts#L177-L181

Since everything goes through hub.startSpan internally with that _isTransaction we can have the check when people use JS (without types) if they forget to set the name property

we have this
https://github.com/getsentry/sentry-javascript/blob/master/packages/apm/src/hubextensions.ts#L30-L37

@HazAT HazAT closed this Jun 2, 2020
@HazAT HazAT deleted the hazat/am-graceful-helper branch July 23, 2020 11:08
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.

3 participants