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

feat: add super properties #138

Merged
merged 4 commits into from
Oct 3, 2024
Merged

feat: add super properties #138

merged 4 commits into from
Oct 3, 2024

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

Copy link
Collaborator

@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

posthog/client.py Outdated Show resolved Hide resolved
posthog/test/test_client.py Show resolved Hide resolved
@neilkakkar
Copy link
Collaborator

(also remember to bump version and changelog before merging)

Copy link
Collaborator

@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
2 checks passed
@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