Skip to content

feat: add super properties#138

Merged
daibhin merged 4 commits intomasterfrom
dn-feat/add-super-properties
Oct 3, 2024
Merged

feat: add super properties#138
daibhin merged 4 commits intomasterfrom
dn-feat/add-super-properties

Conversation

@daibhin
Copy link
Contributor

@daibhin daibhin commented Oct 3, 2024

Problem

Customers of error tracking want to know what repository a particular exception came from by filtering in the UI. Rather than adding some specific property (e.g. exception_source_identifier) that would then need to be added to the JS SDK and any other language we support in future, I thought it would be easier to reuse the super properties concept found in our other SDKs

I'm not tied to this approach but "PRs > Slack" and all that... Open to feedback here.

Changes

  • Went with a very basic implementation that allows you to set super_properties when initialising the client
    • Could have gone with the register method seen elsewhere but didn't want to mislead customers into thinking that properties were somehow stored elsewhere (like they are in local storage). Better to make sure that they're always included when the client is initialised

@daibhin daibhin requested a review from neilkakkar October 3, 2024 09:51
Copy link
Contributor

@neilkakkar neilkakkar left a comment

Choose a reason for hiding this comment

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

One blocking comment, otherwise LGTM! Pragmatic approach to adding it on sdk init, I like it

@neilkakkar
Copy link
Contributor

(also remember to bump version and changelog before merging)

@daibhin daibhin requested a review from neilkakkar October 3, 2024 12:08
Copy link
Contributor

@neilkakkar neilkakkar left a comment

Choose a reason for hiding this comment

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

noice

@daibhin daibhin merged commit ee03059 into master Oct 3, 2024
@daibhin daibhin deleted the dn-feat/add-super-properties branch October 3, 2024 16:07
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.

2 participants