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.

Sign up for GitHub

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

Jump to bottom

[server] Add trackEvent to support dashboard analytics #4753

Merged
merged 1 commit into from
Jul 9, 2021
Merged

[server] Add trackEvent to support dashboard analytics #4753

merged 1 commit into from
Jul 9, 2021

Conversation

csweichel
Copy link
Contributor

@csweichel csweichel commented Jul 8, 2021 •
edited
Loading

This PR adds a trackEvent call to the Gitpod server to enable analytics event propagation from the dashboard.

fixes #4728

@csweichel csweichel requested review from svenefftinge, jakobhero, a team and JanKoehnlein and removed request for a team and JanKoehnlein July 8, 2021 14:41
@svenefftinge
Copy link
Member

Awesome! Would it make sense to use a reduced shape for the message that doesn't require adding the userid and timestamp as we can add that on the server side?

svenefftinge
svenefftinge reviewed Jul 8, 2021
View reviewed changes
components/gitpod-protocol/src/analytics.ts
| { userId?: string | number; anonymousId: string | number };

interface Message {
messageId?: string;
Copy link
Member

@svenefftinge svenefftinge Jul 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What is the messageId and its purpose? Can it be omitted on the client-side and generally added when forwarded to segment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

messageId is not required per se, but can be used to de-duplicate messages. Segment will de-dup messages with the same ID that came in in a 24h window.

@csweichel csweichel force-pushed the csweichel/server-add-analytics-4728 branch from 72024ba to fbbce19 Compare July 8, 2021 17:49
@csweichel
Copy link
Contributor Author

I've updated the shape to better reflect which fields we actually use.
The timestamp is optional and set by the analytics writer (e.g. segment) when needed.

@csweichel
Copy link
Contributor Author

csweichel commented Jul 8, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run no-preview

👍 started the job as gitpod-build-csweichel-server-add-analytics-4728.4

@csweichel csweichel merged commit 53bc9e9 into main Jul 9, 2021
@csweichel csweichel deleted the csweichel/server-add-analytics-4728 branch July 9, 2021 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@svenefftinge svenefftinge svenefftinge approved these changes

@jakobhero jakobhero Awaiting requested review from jakobhero

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[server] Add analytics API
2 participants
@csweichel @svenefftinge