Implement analytics in Nuxt #844
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openverse-bot
added
🟧 priority: high
|
Full-stack documentation: Ready https://WordPress.github.io/openverse/_preview/844 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
Co-authored-by: Zack Krida <zackkrida@pm.me>
7 tasks
There was a problem hiding this comment.
Some initial feedback. Forgive me if I am missing something that invalidates my recommendation. This is exciting work and I don't want to delay it. I believe my recommendations (if valid) make the PR get in faster because I think they would massively reduce the amount of code required to implement the baseline for event tracking.
Gotcha, sorry about that. I misunderstood what we discussed in the Plausible RFC issue and thought you'd wanted me to share feedback here, folding in what I'd pointed out in the RFC. Apologies for moving too quickly. The summarised changes you shared here look great, I'll review the code today, but I'm sure it's great. One potential improvement is that we can use Nginx for the redirect, and it could be better performance wise to not tie up the event loop with those redirects, even if they're fast. Just something to note if we notice any degradation in performance, considering these requests happen for every new client page load. |
|
@sarayourfriend I am really thankful and appreciative that you reviewed it as quickly and as thoroughly as you did. |
dhruvkb
added
🧱 stack: frontend
Plausible does not prevent us from sending events to it even if the site does not exist. So we could technically let the staging server keep sending info to Plausible that would be discarded by them. When we do need to track events, we can register the staging site on our Plausible cloud account, do the tracking we need to do, and then delete the site (and associated data) thus stopping tracking again. Also the feature flag for analytics would be switched off in staging during which no events will be sent to Plausible. $ curl --location 'https://plausible.io/api/event' \
--header 'Content-Type: application/json' \
--data '{
"n": "pageview",
"u": "https://staging.openverse.org/",
"d": "staging.openverse.org",
"r": "https://staging.openverse.org/",
"w": 999,
"h": 2
}'
ok
$ curl --location 'https://plausible.io/api/event' \
--header 'Content-Type: application/json' \
--data '{
"n": "CUSTOM_EVENT",
"u": "https://staging.openverse.org/",
"d": "staging.openverse.org",
"r": "https://staging.openverse.org/",
"p": "{\"key\":\"value\"}",
"w": 999,
"h": 2
}'
ok |
sarayourfriend
approved these changes
Mar 14, 2023
There was a problem hiding this comment.
I've tested this locally, and it looks good to me. Everything works, following the testing instructions. Plausible's UI is simple but easy to use.
I noticed two three things as I was playing around with Plausible and the custom events. None of these are blocking issues and should be addressed in follow-up issues or clarified in documentation whether they're expected behaviour.
- The "entry" pages appears to be tied to a specific user/session. If a person reloads a page, it doesn't think of it as a new "entry" and so does not fully capture every server rendered page.
- The width and height props on custom events are going to be interesting to use and I wonder if there are other ways of encoding the data, perhaps as an additional prop (breakpoint, for width, like we do with the UI state cookie?). It currently gives a separate bar graph for every single instance of every single width, so at a glance, at least from the Plausible UI, it's basically useless. We can transform this ourselves once we're analysing the data in our own pipelines, but if we're doing anything "at a glance" with these, we'll want to consider encoding ranges as a separate prop to make it easier for a human to understand.
- To test things on localhost, you must visit with "localhost" as the domain. You cannot do what I tried to do (for far too long before I realised what was happening) and visit the site on another device on your local network at the LAN IP. We'd need to set up some kind of expected hosts file configuration for being able to test on multiple devices like
openverse.localso that other devices can have the correct referrer on the request to Plausible. This is worth clarifying in the documentation (along with any other limitations we might be aware of in testing) and opening an issue to explore ways of fixing it that won't intrude on the average use case.
Anyway, like I said, none of these are blockers or should be addressed in this PR. This PR is good to go from my perspective and I'm exciting to start getting to use this!
|
Size Change: +3.64 kB (0%) Total Size: 882 kB
ℹ️ View Unchanged
|
obulat
approved these changes
Mar 17, 2023
|
Good point, @obulat! I'll add tests in a separate PR, which will focus on the usage of this utility to record all the custom events. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #825 by @dhruvkb
Description
This PR implements a framework in Nuxt for sending analytic events to Plausible. It also adds an event as an example.
Testing Instructions
just upjust frontend/run devlocalhost:50288.CLICK_HOME_GALLERY_IMAGEin the Plausible UI.Checklist
Update index.md).main) ora parent feature branch.
errors.
Developer Certificate of Origin
Developer Certificate of Origin