feat: Create IdleTransaction class #2720
Conversation
| private _finishCallback?: (transactionSpan: IdleTransaction) => void; | ||
|
|
||
| public constructor( | ||
| transactionContext: TransactionContext, |
There was a problem hiding this comment.
Same here for some of parameters
| const popActivity = (id: string) => { | ||
| this._popActivity(id); | ||
| }; | ||
| this.spanRecorder = new IdleTransactionSpanRecorder(pushActivity, popActivity, this.spanId, maxlen); |
There was a problem hiding this comment.
Passing these methods is a bit overkill for my taste, but they help to test stuff, so it's fine
There was a problem hiding this comment.
Yes I couldn't find a better way around this, I'm not the biggest fan of it either.
|
I didn't review the implementations you moved from previous class, but rather the structure itself. Looks good! |
| export class IdleTransaction extends Transaction { | ||
| // Activities store a list of active spans | ||
| // TODO: Can we use `Set()` here? | ||
| public activities: Record<string, boolean> = {}; |
There was a problem hiding this comment.
@kamilogorek can we use es6 Set() in this codebase? - or do you think we should stay away from this and just use regular JS objects.
There was a problem hiding this comment.
Let's use object, Set is only support IE 11
| const popActivity = (id: string) => { | ||
| this._popActivity(id); | ||
| }; | ||
| this.spanRecorder = new IdleTransactionSpanRecorder(pushActivity, popActivity, this.spanId, maxlen); |
There was a problem hiding this comment.
Yes I couldn't find a better way around this, I'm not the biggest fan of it either.
* feat: Adjust hub for idle transaction * feat: Add IdleTransaction class * test: IdleTransaction * ref: Some uneeded code * ref: Declare class variables in constructor * chore: Cleanup set() comments
* feat: Create IdleTransaction class (#2720) * feat: Add BrowserTracing integration (#2723) * feat: Add span creators to @sentry/tracing package (#2736) * ref: Convert React and Vue Tracing to use active transaction (#2741) * build: generate tracing bundles * fix: Remove circular dependency between span and transaction * ref: Add side effects true to tracing * build: Only include @sentry/browser for bundle * fix: Make sure vue and react are backwards compatible with @sentry/apm
Dependent on: #2719
As part of the
Tracingintegration refactor, this PR adds a newIdleTransactionclass. By separating the concept of activities/idle transactions from the browser integration, we get some cool benefits.IdleTransactiondon't have to worry about "idle". They can just create and add spans as usual.This is pretty much the same implementation as the one from #2705, just with some added tests and some small quality of life improvements. See the usage there to get an idea what this looks like.
Next steps:
BrowserTracingintegration that allowsRoutingInstrumention(what we have with pageload/navigation), and a bunch of span registers, which are functions that will create spans.BrowserTracingintegration.