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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions packages/apm/src/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,27 @@ function traceHeaders(this: Hub): { [key: string]: string } {
}

/**
* This functions starts a span. If there is already a `Span` on the Scope,
* the created Span with the SpanContext will have a reference to it and become it's child.
* Otherwise it'll crete a new `Span`.
*
* @param context Properties with which the span should be created
* {@see Hub.startSpan}
*/
function startSpan(this: Hub, context: SpanContext | TransactionContext): Transaction | Span {
/**
* @deprecated
* We have this here as a fallback to not break users upgrading to the new API
Comment on lines +27 to +28
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.

*/
if ((context as any).transaction !== undefined) {
logger.warn(`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

}

// This is our safeguard so people always get a Transaction in return.
// We set `_isTransaction: true` in {@link Sentry.startTransaction} to have a runtime check
// if the user really wanted to create a Transaction.
if ((context as TransactionContext)._isTransaction && !(context as TransactionContext).name) {
logger.warn('You are trying to start a Transaction but forgot to provide a `name` property.');
logger.warn('Will fall back to <unlabeled transaction>, use `transaction.setName()` to change it.');
(context as TransactionContext).name = '<unlabeled transaction>';
const name = '<unlabeled transaction>';
logger.warn(`Will fall back to \`${name}\``);
(context as TransactionContext).name = name;
}

if ((context as TransactionContext).name) {
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ export interface Hub {
traceHeaders(): { [key: string]: string };

/**
* This functions starts either a Span or a Transaction (depending on the argument passed).
* If there is a Span on the Scope we use the `trace_id` for all other created Transactions / Spans as a reference.
* This functions starts either a Span or a Transaction (depending on the context).
* If there is a Span on the Scope, the created Span will be a child.
*
* @param context Properties with which the Transaction/Span should be created
*/
Expand Down