ref(transactions): Remove all of transaction metric extraction#5792
ref(transactions): Remove all of transaction metric extraction#5792
Conversation
Dav1dde
force-pushed
the
dav1d/rm-transaction-metric-extraction
branch
from
b134b65 to
52ddb1d
Compare
Dav1dde
force-pushed
the
dav1d/rm-transaction-metric-extraction
branch
from
52ddb1d to
ed909ef
Compare
Dav1dde
force-pushed
the
dav1d/rm-transaction-metric-extraction
branch
from
ed909ef to
b046034
Compare
|
@loewenheim jfyi the scope changed a bit sicne you last reviewed. I also deleted the metric namespace and had to adjust a bunch more tests. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2a7d31e. Configure here.
| Item::new(0, MetricNamespace::Transactions), | ||
| Item::new(1, MetricNamespace::Transactions), | ||
| Item::new(0, MetricNamespace::Sessions), | ||
| Item::new(1, MetricNamespace::Sessions), |
There was a problem hiding this comment.
In these tests, is there something particular about the Sessions namespace or do you just need something to replace the deleted Transactions one?
| Spans { | ||
| count: 2, | ||
| is_segment: true, | ||
| was_transaction: false, | ||
| }, |
There was a problem hiding this comment.
Naively it seems like we've lost a span here. The previous value for Transactions was 3.
| // | ||
| // Transactions are counted using a span metric, as transaction payloads should | ||
| // eventually be fully transformed into spans. But this is for now only the case for | ||
| // metrics, the payloads are lacking behind. |
There was a problem hiding this comment.
Nit:
| // metrics, the payloads are lacking behind. | |
| // metrics, the payloads are lagging behind. |
3 participants
This showcases a problem (which existed before) where we attach the transaction outcome to the span usage metric. When there are span rate limits, the transaction is also not counted. The code comment addresses a few options.