feat(measurements): Add experimental set_measurement api on transaction#1359
feat(measurements): Add experimental set_measurement api on transaction#1359sl0thentr0py merged 11 commits intomasterfrom
Conversation
antonpirker
left a comment
There was a problem hiding this comment.
I like this better then the plural version. Very clean interface. This way one can set and update values.
|
|
||
| events = capture_events() | ||
|
|
||
| transaction = start_transaction(name="measuring stuff") |
There was a problem hiding this comment.
if I do:
child = transaction.start_child(x)
can I do child.set_measurement(x, y) too?
Or start_child would return a different interface that does not provide set_measurement? since measurements are only available at the transaction level.
There was a problem hiding this comment.
Currently they are just on transaction and not span, start_child returns a span so you can't add measurements there. From what I understood of the requirements, we only want to expose this api on the transaction but this is a design decision and we can also change it.
Open decision is also if we want to expose this on the hub/top-level api as well where it'll just fetch the current hub/scope/transaction and set it there.
There was a problem hiding this comment.
I agree that we should not leak this API to the Child level as well, but a few SDKs return the very same interface for start_transaction and start_child, so unless we change that, the public API will leak.
cc @bruno-garcia
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
3036ecb to
28662c0
Compare
62a0ac6 to
822ad0f
Compare
|
tested end-end with relay/ingestion |
|
tested graceful error handling (unit name too long)
|
I went with
set_measurement(name, value, unit)(singular) instead ofset_measurementsas in JS since I think this is more convenient for the end user. Otherwise they need to build the name -> unit/value dict themselves.This should be merged once develop is synced and the changes in relay have been made to support units for
measurements.