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

Adds serverside collection support for v2 #111

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mackenly
Copy link

Adds support for collecting events sent serverside. This enables users to send analytics events without running client-side scripts on their user's browsers. Furthermore, it will also enhance #17 allowing the managed component to use serverside tracking rather than the client side script.

Leaving this in draft for now. Still needs:

  • Better test coverage
  • Documentation of the API endpoint

This is a recreation of #94 updated to use v2. I'll make the requested changes made to that PR here.

@mackenly
Copy link
Author

@benvinegar Looking to get this wrapped up. Made changes to use the existing variable names.

Previously, there was some mention of responding based on content type. By that, do you want everything to go through GET request and branch based on the content type?

Right now, it's restful and when the collect endpoint receives a POST request, it accepts fields from the request body using the same field names and the parameter names (with a few additions) used for GET requests (with the pixel).

interface PostRequestBody {
    sid?: string; // site id
    h?: string; // host
    ua?: string; // user agent
    p?: string; // path
    c?: string; // country
    r?: string; // referrer
    bn?: string; // browser name
    dm?: string; // device model
    ims?: string; // if modified since
}

If ims is not present, it tries the header. Same thing for ua.

@benvinegar
Copy link
Owner

Thanks for coming back. This seems in the right ballpark.

By that, do you want everything to go through GET request and branch based on the content type?

I think what you have makes sense.

Give me some more time to review but at a glance it seems to make sense for me. I'm not sure I'd expect the POST client to bother with if-modified-since shenanigans though.

@mackenly
Copy link
Author

Sounds good. I still need to write tests as well.

I'm not sure I'd expect the POST client to bother with if-modified-since shenanigans though.

My use case is saving the timestamp persistently in the user's client and passing that along with serverside requests. So, if being used in that type of setup, I do think there's value in keeping it available.

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

Successfully merging this pull request may close these issues.

2 participants