Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sentry-core: support custom user data in TransactionContext #512

Merged
merged 2 commits into from Nov 28, 2022

Conversation

tommilligan
Copy link
Contributor

Relates to recent work in #510

Adds support for the traces_sampler inspecting:

  • first class properties of the TransactionContext
    • name
    • operation
    • sampled
  • custom transaction context data, passed by the transaction starter

This is consistent with the Python implementation for custom sampling context.

Adds support for this custom payload by:

  • making it gettable/settable in the same way sampled is; this is back compatible
  • making it None by default, to minimise performance impact
  • stored data is serde_json::Value:
    • a well established data type for arbitrary data in the Rust ecosystem
    • already a dependency in the sentry-core crate for internals of the v7 protocol
    • supports most data types useful to developers
    • mimics the existing behaviour of the python implementation (arbitary data, in a top level object with string keys)


/// Update the custom context of this Transaction.
///
/// For simply adding a key, use the `set_custom_key` method.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is set_custom_key meant to be the custom_insert method below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes. Will fix - I renamed it after I changed the signature to be in line with hashmap's insert (return previous value)

@Turbo87
Copy link
Contributor

Turbo87 commented Nov 6, 2022

@tommilligan thanks for working on this!

would it make sense to extract the public getter functions to a dedicated PR to unblock them from getting merged and released? not sure how much discussion the custom user data feature needs... :)

@tommilligan
Copy link
Contributor Author

@tommilligan thanks for working on this!

would it make sense to extract the public getter functions to a dedicated PR to unblock them from getting merged and released? not sure how much discussion the custom user data feature needs... :)

Fair point. Split the public getters into #514

@tommilligan
Copy link
Contributor Author

Rebased on master to fix conflicts.

@Swatinem Swatinem enabled auto-merge (squash) November 28, 2022 15:39
@Swatinem Swatinem merged commit e16ad3f into getsentry:master Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants