electerious / ackee-tracker

Transfer data to Ackee.
MIT License
196 stars 22 forks source link

Too many visitors with Gatsby / React app #24

Closed gempain closed 4 years ago

gempain commented 4 years ago

I'm trying to add Ackee tracker to my Gatsby project, without using the gatsby plugin as it seems out of date.

In my understanding, you should

I've implemented this pattern, but unfortunately my "Active visitors" count is increased on each page change, even though I can confirm that there only a single instance of Ackee tracker created.

Here is my gatsby-browser.js:

import React from "react"
import { Ackee } from './src/hooks/use-analytics';

export const wrapRootElement = ({ element }) => (
  <Ackee>{element}</Ackee>
)

and use-analytics.js:

import React, { createContext, useContext, useEffect, useState } from 'react';
import * as ackeeTracker from 'ackee-tracker';

const AckeeContext = createContext(undefined);

export const useAckee = () => useContext(AckeeContext);

export function Ackee(props) {
  const [ackee] = useState(
    process.env.GATSBY_ACKEE_SERVER && process.env.GATSBY_ACKEE_DOMAIN_ID ? ackeeTracker.create({
      server: process.env.GATSBY_ACKEE_SERVER,
      domainId: process.env.GATSBY_ACKEE_DOMAIN_ID,
    }, {
      ignoreLocalhost: process.env.GATSBY_ACKEE_IGNORE_LOCALHOST !== 'false',
      detailed: process.env.GATSBY_ACKEE_DETAILED !== 'false',
    }) : undefined,
  );

  const [record, setRecord] = useState();

  const navigate = (path) => {
    if (record) {
      record.stop();
    }
    if (ackee) {
      setRecord(ackee.record({
        siteLocation: `${window.location.origin}${path}`,
      }));
    }
  };

  useEffect(() => {
    // first record
    if (ackee) {
      setRecord(ackee.record());
    }
    return () => {
      if (record) {
        record.stop();
      }
    }
  }, [ackee]);

  return (
    <AckeeContext.Provider value={navigate} {...props}/>
  )
}

In my app, in my side nav component, I do this:

...
const pathChange = useAckee();

const onClick = () => pathChange(path);
...

This works very well:

But, my Active visitor counter is not correct.

Any idea ?

electerious commented 4 years ago

It's not you, it's Ackee. The current counter considers all records active that have been updated in the last 30 seconds. We should improve the aggregation to only show non-anonymized data (the most recent visit of a user): https://github.com/electerious/Ackee/blob/master/src/aggregations/aggregateActiveVisitors.js

There's also a use-ackee module which might be what you're looking for :)

electerious commented 4 years ago

I've created a new issue. Help is welcome. Should be easy to fix :)

gempain commented 4 years ago

@electerious I tried using useAckee and had the same result. I initially though that the problem was coming from creating a new tracker each time the component was mounted, but after all that hard work creating a more complex solution, I ended up being a bit disappointed. What I would have wanted to see is a method changePath(path: string) on the record instance.

But why is the stop() method not ending the page visit directly ? I would expect stop() to tell Ackee "this visitor is not active on this page anymore", and since I'm directly after calling .record() on the same tracker instance, it should know that the visitor is now active on the new page (and I'm assuming it should be aware that this is the same user, since it's the same tracker instance).

electerious commented 4 years ago

But why is the stop() method not ending the page visit directly ?

Entries become stale when you no longer update them. It's because not every record will be stopped via .stop. Most of the time users simply leave your site and will therefore stop updating their record. I think that's fine, but I agree that there's room for improvement when it comes to the API of ackee-tracker. It would be nice to see other implementations so users can choose the one that they like most. In the end it's just a script that pushes data to a server.

e.g. we could eliminate instances at all, so you would create a record directly and can do something with it. You can still use variables for server and domain if you want to have it in a separate place or wrap it inside a function to create an instance like API.

const record = ackeeTracker.record({
    server: 'https://example.com',
    domainId: 'hd11f820-68a1-11e6-8047-79c0c2d9bce0'
})
record.update({
    siteLocation: window.location.href
})

I however realise that this would be wrong because you aren't updating a record, you're creating a new one. It's still interesting to think about how we could improve the API.

gempain commented 4 years ago

Thanks for the answer. I honestly think the API makes sense as is, it's great ! The only thing that bugged me was that in my understanding, when I say "stop", I expect the tracking for that user to be ended and hence it to be removed from counters. Other than that, what you have done is prefect IMO. I've integrated Ackee in docs.metroline.io and it works great 😄