ref(tracing): Make metadata property on Transaction class public
#3557
Conversation
size-limit report
|
| /** JSDoc */ | ||
| export class Transaction extends SpanClass implements TransactionInterface { | ||
| public name: string; | ||
|
|
||
| private _metadata: TransactionMetadata = {}; | ||
| public metadata: TransactionMetadata; |
There was a problem hiding this comment.
Related to the previous comment. If we already have a setter, do we want to make it public? IMO it seems unnecessary and misleading. I'd either go with the getter/setter or the public attribute.
There was a problem hiding this comment.
The setter above as you mentioned "merges", so it's helpful method. But I do agree that we could skip it if we'd go with get/set only, and not merge.
There was a problem hiding this comment.
It feels way simpler to me to just have it public. Honestly, IDK why I made it private to begin with.
One thing we might think about (in the long list of things we're thinking about for v7) is having a more coherent strategy for what's public and what's private with setters and getters. I don't feel like I have a great intuitive sense for which is better in cases like this.
lobsterkatie
force-pushed
the
kmclb-make-transaction-metadata-public
branch
from
06054b0 to
d7fb1fd
Compare
lobsterkatie
force-pushed
the
kmclb-make-transaction-metadata-public
branch
from
d7fb1fd to
bf75a3d
Compare
Pulling this in from https://github.com/getsentry/sentry-javascript/tree/kmclb-dynamic-sampling-data-in-envelopes. It makes the
metadataproperty on theTransactionclass public, and allows it to be passed in at transaction creation time, which at this moment would be helpful for nextjs tracing work.