Skip to content

bug: daily active users modal in Deployment tab is incorrect #7275

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

Closed
ericpaulsen opened this issue Apr 24, 2023 · 6 comments · Fixed by #7647 or #7736
Closed

bug: daily active users modal in Deployment tab is incorrect #7275

ericpaulsen opened this issue Apr 24, 2023 · 6 comments · Fixed by #7647 or #7736
Assignees
Labels
s2 Broken use cases or features (with a workaround). Only humans may set this. site Area: frontend dashboard
Milestone

Comments

@ericpaulsen
Copy link
Member

customer daily active user count showed 16 on Sunday, which was incorrect, since no users had logged in. the figure was likely off by a day, since it showed 0 for the previous day, Saturday.

@ericpaulsen ericpaulsen added bug site Area: frontend dashboard labels Apr 24, 2023
@bpmct bpmct added the s2 Broken use cases or features (with a workaround). Only humans may set this. label Apr 27, 2023
@Emyrk Emyrk self-assigned this Apr 28, 2023
@Emyrk
Copy link
Member

Emyrk commented Apr 28, 2023

This might be a UTC vs local timezone thing. I will look into it.

@Emyrk Emyrk removed their assignment May 9, 2023
@Emyrk
Copy link
Member

Emyrk commented May 9, 2023

I hope to come back to this. If anyone else wants to, then can fix this.

Pretty sure it is a UTC vs local timezone issue. We use UTC on the backend to generate these buckets.

@ammario ammario added this to the 🧹 Sprint 0 milestone May 22, 2023
@ammario ammario removed the sprint-0 label May 22, 2023
@Emyrk
Copy link
Member

Emyrk commented May 23, 2023

The issue is that the SQL query truncates the dates in UTC. So only UTC date buckets work.

(created_at at TIME ZONE 'UTC')::date as date,

The counting calculation is cached.

See the counting here:

respMap := make(map[time.Time][]uuid.UUID)
for _, row := range rows {
respMap[row.Date] = append(respMap[row.Date], row.UserID)
}

A possible solution is to expose the raw timestamps as utc values, and have the golang make the correct date buckets. We can then do the calculation 24 times, one for each major timezone. To accommodate weird timezones, we just do timezone offsets, and let the caller choose the closest offset to them.

Thoughts?

@Emyrk
Copy link
Member

Emyrk commented May 23, 2023

Just realized the postgres does a group by on the UTC date to remove duplicates. So we cannot return the raw date from the sql, or else we could be returning a large amount of rows.

Hmm 🤔

@Emyrk
Copy link
Member

Emyrk commented May 23, 2023

My PR describes what I implemented. This makes the response able to change in reference to the caller's local timezone.

@Kira-Pilot Kira-Pilot assigned Emyrk and unassigned Kira-Pilot May 23, 2023
@bpmct
Copy link
Member

bpmct commented May 30, 2023

I'm going to reopen this until we change the frontend to show the proper values based on the user's timezone. There is a backend fix, but it's not really obvious/accessible for the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s2 Broken use cases or features (with a workaround). Only humans may set this. site Area: frontend dashboard
Projects
None yet
7 participants