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 Segment events to bulk actions on adminbro #152

Closed
mateodaza opened this issue Oct 5, 2021 · 13 comments
Closed

Add Segment events to bulk actions on adminbro #152

mateodaza opened this issue Oct 5, 2021 · 13 comments
Assignees

Comments

@mateodaza
Copy link
Member

No description provided.

@mohammadranjbarz
Copy link
Collaborator

@divine-comedian
it's ready for test

@divine-comedian
Copy link
Collaborator

can you provide more information on how to test this - I should list/unlist multiple projects on adminBro and check they appear on segment?

@mohammadranjbarz
Copy link
Collaborator

can you provide more information on how to test this - I should list/unlist multiple projects on adminBro and check they appear on segment?

Yeah exactly,
@CarlosQ96 Am I right?
could you add something if Mitch missed?

@divine-comedian
Copy link
Collaborator

divine-comedian commented Oct 26, 2021

I have tested all of the actions (list/unlist/verify/unverify/activate/deactivate/cancel) in bulk and they do successfully appear in segment, however their seems to be a serious issue of not each instance arriving in segment.

For examples when I bulk list 10 projects someitmes only 9 events show up in segment, other times only 8, even as few as 6 events show up.

I tried lowering the number to bulk list/cancel/verify/etc.. 5 events.. sometimes 5 show up... sometimes 4 events show up

so something weird is happening, perhaps its overloaded with having multiple requests at the exact same moment.

Would it be helpful to add a short delay between each analytcis send if we do bulk actions like this?

@CarlosQ96
Copy link
Collaborator

Hey @divine-comedian Ill double check if maybe we need to add a delay or an await so that it sends them all. Will let you know after I finish testing.

@CarlosQ96
Copy link
Collaborator

I have a question @mohammadranjbarz if analytics.track is a call to an external api, shouldn't this be a async function? we have it as a normal function in the analytics.ts

Maybe it works in most calls, because it's not an expensive call and before the request cycle ends, it answers. But in this case multiple calls do not manage to finish before the server moves on to the next nonblocking task.

segmentio/analytics-node#303 this is not exactly the same issue but he does experience something similar.

@CarlosQ96
Copy link
Collaborator

CarlosQ96 commented Oct 27, 2021

Actually after doing some testing, it seems analytics-node batches the requests in memory to send the track payload sometime in the future. (we allow batching in prod and staging).

Subsequent requests from the batches.... seem to take a up to a few seconds max to be sent, however at the same time after 10 seconds the batch is flushed, so any message not sent is lost. (this is most likely the issue)
Documentation: https://segment.com/docs/connections/sources/catalog/libraries/server/node/#batching
Gonna keep testing and let you know guys know what solution I come up with. Probably we will have to add config options to analytics constructor.

@MoeNick
Copy link
Member

MoeNick commented Oct 28, 2021

@CarlosQ96 do we need to take it back to in-progress, or make another issue?

@CarlosQ96
Copy link
Collaborator

@MoeNick Currently tweaking the configuration of Segment Analytics. Seems that if we set a small batch of events to send at the same time it works fine. Testing with Mohammed.
I think we can make another issue, it's more related to segment than the actual cancel status of the projects.

@CarlosQ96
Copy link
Collaborator

No nvm @MoeNick You are right this is the issue related to it. Ill move it back to in-progress.
Got the other issue mixed up

@mohammadranjbarz
Copy link
Collaborator

I have tested all of the actions (list/unlist/verify/unverify/activate/deactivate/cancel) in bulk and they do successfully appear in segment, however their seems to be a serious issue of not each instance arriving in segment.

For examples when I bulk list 10 projects someitmes only 9 events show up in segment, other times only 8, even as few as 6 events show up.

I tried lowering the number to bulk list/cancel/verify/etc.. 5 events.. sometimes 5 show up... sometimes 4 events show up

so something weird is happening, perhaps its overloaded with having multiple requests at the exact same moment.

Would it be helpful to add a short delay between each analytcis send if we do bulk actions like this?

Hey Mitch, could you test this please?
Noow we put 1.5s delay between events, it decreases events losses ( maybe you miss 2 events between 40), it's not best solution but as the library has bug (segmentio/analytics-node#309) we cant do it in better way.

PS: As we expect because of delay your events would showup with delay in Segment

@mohammadranjbarz
Copy link
Collaborator

@divine-comedian

This is on staging now, you can test it again

@divine-comedian
Copy link
Collaborator

testing this on the admin dashboard -

I tried out a few different functions in bulk and they successfully show up with a delay on segment and the correct amount of actions are executed. This issue looks to be resolved! Yay!

@MoeNick MoeNick closed this as completed Sep 7, 2022
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

5 participants