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

Add group support on Mixpanel #4

Closed
wants to merge 1 commit into from
Closed

Add group support on Mixpanel #4

wants to merge 1 commit into from

Conversation

cpinto
Copy link

@cpinto cpinto commented Dec 21, 2014

This patch brings group() support to Mixpanel, by adding users with a property called isGroup set to true and forcing the prefix 'group.' on $distinct_id so that it doesn't conflict with normal users.

It additionally appends a property called "groups" to user properties, that is a unique set of group IDs, making it possible to go from a user to the groups.

This is extremely useful for SaaS businesses with pricing per organisation, enabling analytics such as how often an organisation is active, revenue per organisation, messaging to users of an organisation through Mixpanel, etc.

The biggest problem with this patch, however, is that I have not found a way to test by running it. I am not familiar with Segment's architecture, and less so with node.js, however I'm keen to accept reviews and make amendments where needed to make this patch work.

@ndhoule
Copy link
Contributor

ndhoule commented Jan 8, 2015

Hey @cpinto, thanks for your PR. Unfortunately, this is functionality that Mixpanel will need to add support for before we can add

Some of the issues here include that this assumes that users don't otherwise use the isGroup property (more likely, and an unsafe assumption), or already have an ID scheme that includes group. (less of a concern, but still a real one).

A bigger issue is that this would begin feeding group calls into Mixpanel, contaminating any existing reporting and requiring all users to set Mixpanel up with consideration given to separating groups from normal users, which is a pretty big onus to put on users and not a great user experience.

I'd love to see a grouping feature added to Mixpanel, but without support on their end it makes too many assumptions and would be too intrusive.

@ndhoule ndhoule closed this Jan 8, 2015
@cpinto
Copy link
Author

cpinto commented Jan 9, 2015

That sucks, but thanks for taking the time to explain. However, it's not consistent with customer.io, on that integration, segment sets "group" properties making the same assumptions (however if a user belongs to two groups only one shows). Would it be possible to at least have this?

@ndhoule
Copy link
Contributor

ndhoule commented Jan 10, 2015

You're right—it's not entirely consistent, and I can explain why that is. We evaluate special additions on a case-by-case basis, and the main criteria we use for evaluating them are these: We only add them in when a) we've heard the same feature request multiple times, b) we're positive it's a net gain as far as user usability goes, c) it fits with the ethos of the tool, d) the tool is unlikely to add the feature going forward, or if it does, adapting our API would be simple and non-destructive, and e) doesn't break workflows. One additional constraint is how widely the tool is adopted amongst Segment users. This feature (and the alternative) fails most of these criteria, and in light of the final note about adoption, makes me pretty reluctant to adopt changes we're not convinced is a long-term net benefit for all our customers.

The customer.io case is one I'm admittedly not a huge fan of personally, but a special one where the added functionality doesn't break user workflow, and helps in many use cases. This PR is different in that it breaks all existing users' workflow and adds an additional, cumbersome step to the Mixpanel-Segment setup.

We would, of course, be open to seeing how the alternate approach would work. If you'd like us to evaluate that case, feel free to put in another PR.

Thanks again!

@cpinto
Copy link
Author

cpinto commented Jan 10, 2015

Thanks for thoroughly listing the objections Nathan. Given the lack of encouragement, I can't be bothered to build a solution so I'll leave it at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants