fix(tracing) Omit the top level status tag

[markstory]

In the product side we don't really want people to search by this tag as it is far more expensive than the transaction.status property which is indexed separately. By not emitting this tag and only including it in the trace context we won't end up with poor performing tags for users to click on.

[untitaker]

@kamilogorek @HazAT can you make sure this happens for JS

[HazAT]

We never did any of this in JS 🤔
Was this just a precaution or am I missing something?
@untitaker

Edit: Removed out of place "string" word xD (was coding beside writing this)

[untitaker]

What do you mean with precaution string? We used to send this:

{
    "event_id": ...,
    "tags": {"status": "ok"},
    "contexts": {"trace": {"status": "ok"}}
}

with this change we remove the event tag

[HazAT]

@untitaker OK, updated my comment.
And we never did this in JS

[markstory]

I had to add a status attribute to the Span object. Without that transactions collected inside celery would not have statuses set because get_trace_context() is called multiple times and after the first the status tag is gone.

[untitaker]

@HazAT I am looking at the UI and I do see JS transactions having a status tag. Could you please figure out what logic to apply here? Do we want to get rid of tags altogether, etc

[untitaker]

we found out it wroks exactly the same in JS: https://github.com/getsentry/sentry-javascript/blob/712b6e7de4a5af6449e651915ce73984751105b7/packages/apm/src/span.ts#L308

However, we do not set the status explicitly most of the time, therefore there rarely is a top-level tag in JS