Skip to content

Async events failing with within Django redirects #4

Closed
Closed
Async events failing with within Django redirects#4

Description

@agmathew
agmathew
opened on Mar 28, 2014

I'm attempting to fire severals calls to Mixpanel from within a Django view that performs some logic and then redirects to another view. My view looks like this:

def start(request):
  if request.method == POST:
    mp.alias(email, mixpanel_id)
    mp.track(email, 'Started trial')
    mp.people_set(email, {'Some property':'Some value'})
    ...
    return HttpResponseRedirect(reverse('app.url.another_page'))

What I'm seeing is that the track event never gets called. I'm not sure if the redirect call is somehow canceling the thread. I'm seeing this both in the Django dev server and my staging nginx+uWSGI server (with "enable-threads" turned on). If I flush all the events synchronously, everything works. Here is how I'm setting up my Mixpanel object in settings.py:

from mixpanel import Mixpanel
from mixpanel_async import AsyncBufferedConsumer
mp = Mixpanel(MIXPANEL_TOKEN, consumer=AsyncBufferedConsumer())

Activity

  1. jessepollak commented on Mar 29, 2014

    @jessepollak
    jessepollak
    on Mar 29, 2014
    Owner

    So, I just tried to replicate this in Flask and I haven't been successful.

    from flask import (
    Flask,
    redirect,
    url_for
)
from mixpanel import Mixpanel
from mixpanel_async import AsyncBufferedConsumer

consumer = AsyncBufferedConsumer()
mp = Mixpanel('TOKEN', consumer=consumer)

app = Flask(__name__)

@app.route("/redirect")
def redirect_route():
    mp.track('redirect', { 'test': True })
    return redirect(url_for('.redirected'))

@app.route('/redirected')
def redirected():
    return """You just got redirected. \n
    The BufferedConsumer currently has {} events in it's buffered queue""" \
    .format(len(consumer._buffers['events']))


if __name__ == "__main__":
    app.run(debug=True)

    When I run that code and hit the /redirect route, it buffers then send the events when either the time limit is hit or the max queue size is hit.

    It'd be awesome if you could provide a small test case in Django where this error occurs so I can attempt to debug.

  2. agmathew commented on Mar 31, 2014

    @agmathew
    agmathew
    on Mar 31, 2014
    Author

    I was able to replicate this with a minimal test case. If you create a new Django project and create an application (I called mine 'async'), you can try the following steps.

    Append the following to settings.py (inserting your own Mixpanel token):

    import mixpanel

from mixpanel import Mixpanel
from mixpanel_async import AsyncBufferedConsumer

MIXPANEL_TOKEN = '<your mixpanel token>'
MIXPANEL = Mixpanel(MIXPANEL_TOKEN, consumer=AsyncBufferedConsumer())

    and this to your views.py:

    from django.shortcuts import redirect
from django.http import HttpResponse
from django.conf import settings
from django.views.decorators.csrf import csrf_exempt

mp = settings.MIXPANEL

html = '''
<html>
<body>
<form method="POST" action=".">
    <input type="text" name="mixpanel_id" id="mp" />
    <input type="submit" value="Submit" />
</form>

<script src="//ajax.googleapis.com/ajax/libs/jquery/2.1.0/jquery.min.js"></script>
<!-- start Mixpanel -->
<script type="text/javascript">(function(e,b){if(!b.__SV){var a,f,i,g;window.mixpanel=b;b._i=[];b.init=function(a,e,d){function f(b,h){var a=h.split(".");2==a.length&&(b=b[a[0]],h=a[1]);b[h]=function(){b.push([h].concat(Array.prototype.slice.call(arguments,0)))}}var c=b;"undefined"!==typeof d?c=b[d]=[]:d="mixpanel";c.people=c.people||[];c.toString=function(b){var a="mixpanel";"mixpanel"!==d&&(a+="."+d);b||(a+=" (stub)");return a};c.people.toString=function(){return c.toString(1)+".people (stub)"};i="disable track track_pageview track_links track_forms register register_once alias unregister identify name_tag set_config people.set people.set_once people.increment people.append people.track_charge people.clear_charges people.delete_user".split(" ");
    for(g=0;g<i.length;g++)f(c,i[g]);b._i.push([a,e,d])};b.__SV=1.2;a=e.createElement("script");a.type="text/javascript";a.async=!0;a.src=("https:"===e.location.protocol?"https:":"http:")+'//cdn.mxpnl.com/libs/mixpanel-2.2.min.js';f=e.getElementsByTagName("script")[0];f.parentNode.insertBefore(a,f)}})(document,window.mixpanel||[]);
    mixpanel.init("''' + settings.MIXPANEL_TOKEN + '''");
</script>
<!-- end Mixpanel -->

<script>
    $(window).load(function() {
        $("#mp").val(mixpanel.get_distinct_id());
    });
</script>
</body>
</html>
'''

@csrf_exempt
def view1(request):
    if request.method == 'POST':
        mp.alias('test_user', request.POST['mixpanel_id'])
        mp.track('test_user', 'Started trial (test)')
        mp.people_set('test_user', {'Trial (test)':True})
        return redirect('async.views.view2')
    else:
        return HttpResponse(html)

def view2(request):
    return HttpResponse('OK')




clear_html = '''
<html>
<body>
Cleared!

<script src="//ajax.googleapis.com/ajax/libs/jquery/2.1.0/jquery.min.js"></script>
<!-- start Mixpanel -->
<script type="text/javascript">(function(e,b){if(!b.__SV){var a,f,i,g;window.mixpanel=b;b._i=[];b.init=function(a,e,d){function f(b,h){var a=h.split(".");2==a.length&&(b=b[a[0]],h=a[1]);b[h]=function(){b.push([h].concat(Array.prototype.slice.call(arguments,0)))}}var c=b;"undefined"!==typeof d?c=b[d]=[]:d="mixpanel";c.people=c.people||[];c.toString=function(b){var a="mixpanel";"mixpanel"!==d&&(a+="."+d);b||(a+=" (stub)");return a};c.people.toString=function(){return c.toString(1)+".people (stub)"};i="disable track track_pageview track_links track_forms register register_once alias unregister identify name_tag set_config people.set people.set_once people.increment people.append people.track_charge people.clear_charges people.delete_user".split(" ");
    for(g=0;g<i.length;g++)f(c,i[g]);b._i.push([a,e,d])};b.__SV=1.2;a=e.createElement("script");a.type="text/javascript";a.async=!0;a.src=("https:"===e.location.protocol?"https:":"http:")+'//cdn.mxpnl.com/libs/mixpanel-2.2.min.js';f=e.getElementsByTagName("script")[0];f.parentNode.insertBefore(a,f)}})(document,window.mixpanel||[]);
    mixpanel.init("''' + settings.MIXPANEL_TOKEN + '''");
</script>
<!-- end Mixpanel -->

<script type="text/javascript">
  $(window).load(function() {
    if (mixpanel.cookie && mixpanel.cookie.clear) {
      mixpanel.cookie.clear();
    }
  });
</script>
</body>
</html>
'''

def clear(request):
    return HttpResponse(clear_html)

    and finally this to your URLs:

    from django.conf.urls import patterns, include, url

from django.contrib import admin
admin.autodiscover()

urlpatterns = patterns('',
    # Examples:
    url(r'^$', 'async.views.view1'),
    url(r'^view2/$', 'async.views.view2'),
    url(r'^clear/$', 'async.views.clear'),
)

    If you start up the dev server and go to localhost:8000 and submit the resulting form, you should see that the "Started trial (test)" event is never posted. If you switch to using the synchronous version of Mixpanel, it should work.

  3. jessepollak commented on Mar 31, 2014

    @jessepollak
    jessepollak
    on Mar 31, 2014
    Owner

    I can confirm that this is an issue. I'll investigate today or tonight.

  4. jessepollak commented on Apr 8, 2014

    @jessepollak
    jessepollak
    on Apr 8, 2014
    Owner

    OK, did a little more digging. Observed one thing: if you reduce the it to just a single track event on the form submit, everything works as expected. Can you confirm that you see this as well?

  5. agmathew commented on Apr 8, 2014

    @agmathew
    agmathew
    on Apr 8, 2014
    Author

    Yes, a single track event works. It's just when there's more than one that
    I'm seeing issues. Specifically the alias and track events together seems
    to be failing, but I haven't tried other combinations yet.

    On Tue, Apr 8, 2014 at 1:36 AM, Jesse Pollak notifications@github.comwrote:

    OK, did a little more digging. Observed one thing: if you reduce the it to
    just a single track event on the form submit, everything works as expected.
    Can you confirm that you see this as well?

    Reply to this email directly or view it on GitHubhttps://github.com//issues/4#issuecomment-39813345
    .

  6. agmathew commented on Apr 8, 2014

    @agmathew
    agmathew
    on Apr 8, 2014
    Author

    I did some more digging and I think the issue is a race condition. After the first event is appended to the buffer, the buffer gets flushed. While this is happening, the second event is appended to the buffer. After the first event is sent, the buffer is cleared. This results in the second event getting dropped without ever being sent.

    I'm still trying to confirm that this is the case and then I can start thinking about a fix.

  7. agmathew commented on Apr 8, 2014

    @agmathew
    agmathew
    on Apr 8, 2014
    Author

    I confirmed that a race condition is the problem and I believe the cleanest way to fix it is to make sure that the mixpanel-python's BufferedConsumer only clears from its buffer what it has already sent. I'll submit a patch to them shortly.

  8. agmathew
    added a commit that references this issue on Apr 8, 2014

    Made BufferedConsumer's _flush_endpoint more thread-safe by making su…

    18ca00f

  9. joeatwork commented on Apr 8, 2014

    @joeatwork
    joeatwork
    on Apr 8, 2014

    Hey guys, I just responded to the PQ in mixpanel-python, I'm not sure the problem can be solved well there. Would you consider locking access to the buffer?

  10. jessepollak commented on Apr 8, 2014

    @jessepollak
    jessepollak
    on Apr 8, 2014
    Owner

    Locking access to the buffer seems like a bad solution to me — that would essentially mean that the main thread would block while the buffer was being flushed, right?

    We could flush the buffer in the main thread before passing it to the async thread?

  11. joeatwork commented on Apr 8, 2014

    @joeatwork
    joeatwork
    on Apr 8, 2014

    That sounds like a really good solution, since you're guaranteed only one thread on each end (or if you wanted to get fancy, you could use a Queue and have multiples on each end.) - Cool!

  12. agmathew commented on Apr 9, 2014

    @agmathew
    agmathew
    on Apr 9, 2014
    Author

    I thought there could only be a single _flush_endpoint running at one time since the thread that fires the flush grabs a lock before it starts? Maybe I'm misunderstanding.

  13. jessepollak commented on Apr 9, 2014

    @jessepollak
    jessepollak
    on Apr 9, 2014
    Owner

    I think it's probably an issue with the thread safety of the _buffers dictionary. Here's what I imagine happens:

    1. An event (e1) gets tracked
    2. Because flush_first is set, e1 instantly gets flushed
    3. While that event is flushing, another event (e2) is added to the buffer
    4. e1 gets sent and the buffer gets cleared, meaning e2 was never sent

    The solution I'm proposing would be to essentially copy the buffer, clear it in the main thread, pass the copied buffer to the new thread. With this solution, I imagine this would go something like:

    1. An event (e1) gets tracked
    2. Because flush_first is set, e1 instantly gets flushed
      • the buffer is copied and cleared
      • copied buffer is sent to thread to flush
      • buffer is now empty
    3. While that event is flushing, another event (e2) is added to the buffer
      • this event goes in the new, empty buffer
    4. whenever the buffer fills up, it is flushed as expected

    Does that make sense?

  14. agmathew commented on Apr 9, 2014

    @agmathew
    agmathew
    on Apr 9, 2014
    Author

    I do like your approach since it's more general purpose and it would be cleaner to implement in the async library. That said, I don't think the _buffers dictionary thread-safety is the issue, since we're not modifying the dictionary, but the lists contained in them. In the case you mentioned, this is how I think it plays out with my patch:

    1. An event (e1) gets tracked
    2. We note that the buffer size is 1
    3. Because flush_first is set, e1 instantly gets flushed
    4. While that event is flushing, another event (e2) is added to the buffer
    5. Since we saved the buffer size, only the first event is cleared from the buffer

  15. jessepollak commented on Apr 14, 2014

    @jessepollak
    jessepollak
    on Apr 14, 2014
    Owner

    Yeah, both of those solutions do essentially the same thing — lock in the buffer-to-be-flushed at the moment of the flush starting (which fixes the issue of things being added to the buffer mid-flush).

    I'm happy with either patch — can you open a PR here that I could take a look at?

  16. agmathew commented on Apr 21, 2014

    @agmathew
    agmathew
    on Apr 21, 2014
    Author

    My current patch works by monkey-patching the _flush_endpoint from the mixpanel-python library, so it's not very elegant. I'll have to look into implementing it using your method.

  17. jessepollak commented on Apr 29, 2014

    @jessepollak
    jessepollak
    on Apr 29, 2014
    Owner

    This issue has been fixed in 0.0.3. Sorry about the delayed turnaround time!

  18. jessepollak
    closed this as completedon Apr 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions