Add a Snowplow interface with the ability to initialize and manage multiple trackers

[OskarSigvardsson]

Hi! I work for a company that makes and sells audio plugins, and we're looking at different solutions for adding tracking to our apps.

The thing with audio plugins is that they are (usually) dynamically loaded into DAWs ("digital audio workstations", the audio plugin hosts), and it is a very common occurance to have a single audio plugin loaded multiple times in the same process (for instance: if you have an audio plugin that works as some kind of filter or audio effect, you may want that filter on several different tracks).

Snowplow seems like a very nice solution for this, except for one thing: the "global singleton" nature of the tracker. That's not ideal for us, it would be really nice to load one tracker for each plugin, if possible.

I was looking through the code an seeing if that was possible: seemingly it is: if you create multiple trackers with Tracker::init, the first one gets stored as the global singleton, but it's not an error to create multiple ones, you just have destroy it yourself (and not use Tracker::instance). I realize that the Storage class is also a global singleton, but I figure that's less of an issue, since it's locked with a mutex, and I plan on passing the same database path for every instance.

However, when implementing this solution, the compiler informed me of another problem: the Tracker destructor is private, so you literally can't manage it yourself: the only way to destroy a Tracker is through Tracker::close(), which destroys the singleton.

One solution to this is simply making the Tracker destructor (and constructor as well, while we're at it) public, and let me totally ignore the singleton functions, they serve no purpose for me anyway. However, at this point, I would like to get advice from you if this would be a good change. Is there any reason you can think of why this would be a problem?

I understand you wanting to offer a singleton as a convenience, but forcing its use like you do now is very thing for a library to do: for many kinds of applications, it's a very dangerous thing to use. There's a reason Singletons are considered an anti-pattern by most authorities.

Incidentally: if you are going to force the use of a global singleton, there are a few fixes that should probably be done:

1. Tracker::init should throw an exception if the singleton already exists (since the only way to delete the class is by deleting the singleton, it should be an error to create any other object).
2. The singleton should not have the type Tracker*, it should be a std::unique_ptr<Tracker>, to ensure that it is always uninitialized.

[paulboocock]

I don't have any real desire to force the global singleton solution, it was a design choice we made a loooong time ago, and has its origins in our web based trackers and trackers backed by sqlite but I'm not the biggest fan of singletons. It has some benefits that I can't ignore for our trackers but I'm certainly open to a more flexible solution which allows the user of the SDK to make that decision. We're using singletons in our mobile SDKs for ease of implementation within an app but I feel like our "server-side" SDKs should allow for more flexibility where possible.

I'd be open to a PR which tidied this up, or simply removed the forced singleton. A hybrid approach is interesting. One of the benefits of the singleton is to simplify the management of the "shared" components.

I'm pondering... could we keep the singleton but also make the Trackers constructor/destructor public so it can be used however you desire. Does this seem reasonable or too complicated of an interface?

We've been moving away from Singletons on our "server-side" trackers so I'm quite open to removing the need here, I think we need to be careful with the shared components though.

This also brings me around the the Storage and Emitter parts, sharing them between Tracker instances strikes race condition fears into me for multithreaded applications. Resource sharing among components increases the complexity and whilst there isn't much we can do to stop it (besides forcing singletons), it's both potentially a performance issue (all that locking), and a risk of the wrong emitters finding events from shared storage. I think I'd recommend a 1:1:1+ of tracker, storage, emitter (you could have multiple emitters for a tracker, to send to multiple collectors if you wanted).

[OskarSigvardsson]

Thanks for the quick response!

I'm pondering... could we keep the singleton but also make the Trackers constructor/destructor public so it can be used however you desire. Does this seem reasonable or too complicated of an interface?

This was exactly my plan, just move the constructor/destructor in the header to the public section and just ignore the singleton functions. At first glance I didn't see any problems with doing that, but I wanted to check with you guys and see if you could forsee any issues.

This also brings me around the the Storage and Emitter parts, sharing them between Tracker instances strikes race condition fears into me for multithreaded applications. Resource sharing among components increases the complexity and whilst there isn't much we can do to stop it (besides forcing singletons), it's both potentially a performance issue (all that locking), and a risk of the wrong emitters finding events from shared storage. I think I'd recommend a 1:1:1+ of tracker, storage, emitter (you could have multiple emitters for a tracker, to send to multiple collectors if you wanted).

I was planning on doing 1:1 emitter to tracker (having a pair for each loaded instance), but leaving the storage as a singleton. Since the storage persists between sessions it makes sense to have that in a single file, and thus with a single storage class. I didn't do a deep code review, but it seemed like Storage is protected by a mutex everywhere, so I don't think there's a huge issue there. If you wanted to do that 1:1:1, it seems to me the only way to do that would be to have in-memory SQLite databases, but that's obviously not ideal. Otherwise, would you have different files for each storage? Like, one for each instance? What if you don't load as many instances the next time you run the app?

I guess since it's SQLite, it's not going to have an issue with several database connections on the same database file (since it uses magical file based locking or whatever), but at that point, a single database connection protected by a mutex seems fine to me.

The only issue I could forsee is if you have two emitter/tracker pairs A and B, and if you add an event using A, but it ends up being B that sends it out, and for some reason that doesn't work. Would there be any issues with doing that (assuming they both have the same server)? I can't imagine that there would be, since this could happen anyway in two different sessions (like, if the program shuts down with unsaved events, the next time it starts a different emitter is going to read it and send it out, right?

[paulboocock]

Apologies for taking a few days to get back to you, had a busy old end to the week.

Many of the questions you ask are the reasons we opted for the singleton approach, it simplifies a typical set up in most applications as most applications only need a single shared tracker. I mentioned the 1:1:1 setup up, which is basically the singleton, but you're correct as soon as you have mutliple trackers, the whole situation gets a little trickier particularly with storage which ideal needs to be shared.

With multiple configruations runnings in the same instance, we've pondered coupling events that we store in sql lite to particular emitters but again the issues arise when you consider what happens on application start up when events already exist and the same emitter isn't created (or as you mention, you don't create the same number of instances).

I think the set up you propose is ideal though and will work as you expect, 1:1 tracker:emitter with shared storage. Multiple emitters are fine if they are all emitting to that same location as you're not going to get into any issues with the events being sent to locations you didn't expect, just be aware the emitter you expect might not be the one to send the event.

You could perhaps share an emitter across the trackers and "start" it when everything is ready to go however this might not work in your world. Just something to consider that might be a safer (and faster as less locking, perhap?) with shared storage.

Anyway, to sum up - your proposals sounds good to me and open up new configuration that I'm quite happy with and willing to accept a PR for.