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

Sanitizing $set properties #1503

Open
alecmocatta opened this issue Oct 29, 2024 · 5 comments
Open

Sanitizing $set properties #1503

alecmocatta opened this issue Oct 29, 2024 · 5 comments

Comments

@alecmocatta
Copy link

What's the canonical way to sanitize $set properties?

I'm (ab)using sanitize_properties like below to add app_version. I see it per-event, and I see $initial_app_version in the person properties, but not current app_version?

posthog.init(..., {
    ...,
    sanitize_properties: (props, _event) => {
        if ("$current_url" in props) {
            props["app_version"] = app_version;
        }
        if ("$initial_current_url" in props) {
            props["$initial_app_version"] = app_version;
        }
    },
});

cc:

@pauldambra
Copy link
Member

Hmmm, this isn't sanitizing... it's adding data.

The correct way to do this (probably) is to register a super property. https://posthog.com/docs/libraries/js#super-properties

What is it you're trying to achieve?

@alecmocatta
Copy link
Author

alecmocatta commented Oct 29, 2024

I want to tag events with static metadata like "app version".

Having that metadata available as "current" and "initial" on person properties is a bonus.

@pauldambra
Copy link
Member

(ab)using sanitize_properties seemed a good fit? I definitely saw others doing this

yep, that lets you edit properties... and I can see why it's not working quite as you expect (the interaction between event properties, person properties, and the multiple calls to sanitize_properties is relatively complicated)

but if the question is what's the canonical way to do this...

to tag all events with a property then register is the canonical way. it uses whatever posthog persistence mode is set so no extra cookie just stores the property in the configured storage

to tag a person with properties then adding them on identify or sending a $set event with $set or $set_once properties is the canonical way https://posthog.com/docs/getting-started/person-properties


if you really want to do it here (and this seems harder to me than the methods above)

then in sanitize_properties I would check for props['$set'] when that's an object that's what's set as person properties, and props['$set_once'] those are set as initial person properties (you're side stepping any checks/validation we do by adding them manually so YMMV).

since we're calling this more than once it's not safe to add a $set or $set_once property since you might be adding them in the wrong place

@pauldambra
Copy link
Member

oh, no, yuck... this is really messy.

you can have $set and $set_once on properties, but also at the top level of the event 🤯
which is why #1462 wrapped the set_once


yep, i'll point you back at

posthog.capture(
    '$set', 
    { 
        $set: { name: 'Max Hedgehog'  },
        $set_once: { initial_name: 'Maximilian T. Hedgehog' },
    }
)

as the canonical way

using sanitize_properties is IMHO a messy enough for person properties to avoid it

@pauldambra
Copy link
Member

see #1515

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

No branches or pull requests

2 participants