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 CR id to plausible events #6035

Merged
merged 5 commits into from
Jan 30, 2024
Merged

feat: add CR id to plausible events #6035

merged 5 commits into from
Jan 30, 2024

Conversation

sjaanus
Copy link
Contributor

@sjaanus sjaanus commented Jan 25, 2024

Added conflict count to CR metrics and CR id.

Something to think about:
There was idea that we can aggregate this data based on CR id, but CR id is just a number from 0 to x. So it will not be unique across instances.

Copy link

vercel bot commented Jan 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 8:23am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 8:23am

@thomasheartman
Copy link
Contributor

Regarding the uniqueness of CR ids across instances: this can be handled by using the instance's baseUriPath or its instanceId. I've made that change in dc0a198

@thomasheartman
Copy link
Contributor

thomasheartman commented Jan 26, 2024

Regarding using base Uri path vs instance ID, I've discussed this in #6022, so I'll copy it in here. Happy to hear your thoughts:

I've set the Unleash baseUriPath as the preferred identifier. Because Plausible is only used on our hosted instance, it should always map to one of our customers (and the fallback should only be used if that's not available).

I'm a little hesitant about putting that data in the event because we don't really need that data. However, we could potentially use it to see if different clients have different usage patterns and encounter higher rates of conflicts.

I'd be happy to discuss this and move to preferring instanceId instead.

That said, I believe we can already do the correlation in plausible anyway due to the source of the events.

@sjaanus sjaanus changed the title feat: add conflict count to cr metrics feat: add CR id to plausible events Jan 30, 2024
@sjaanus sjaanus merged commit 2643ac1 into main Jan 30, 2024
9 of 12 checks passed
@sjaanus sjaanus deleted the conflict-cr branch January 30, 2024 08:38
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