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 #94

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mackenly
Copy link

@mackenly mackenly commented Sep 4, 2024

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

@benvinegar
Copy link
Owner

benvinegar commented Sep 5, 2024

Hey @mackenly – thanks for submitting a PR!

I'm down with adding a POST endpoint; makes total sense. I'm really hesitant to bifurcate the API though, where the GET handler supports different parameter names than the POST parameters.

I know the GET parameters are pretty opaque (sid, h, p, etc.), but I'd prefer we use the same names in both handlers. Similarly, what's the harm in also passing if-modified-since as an HTTP header in the POST request (vs passing it in the body)? It should be possible to do so from the server-side.

Basically, I'd like to avoid complexity as much as possible, and this feels too much like having two very different handlers.

In some ways, it feels like the thing that this really needs is to accept and respond to a Content-type (e.g. return the gif or return a parseable JSON response). Could we do that instead?

@mackenly
Copy link
Author

mackenly commented Sep 5, 2024

@benvinegar Good points! Would it make sense to have it at a separate path to clarify the difference?

@benvinegar
Copy link
Owner

benvinegar commented Sep 7, 2024

Would it make sense to have it at a separate path to clarify the difference?

I think I'm trying to say: I'd prefer it didn't have a separate path, and instead the same original handler instead responds appropriately to Content-type and returns a JSON response.

That means having to use the same hard-to-decipher parameters, and the same HTTP headers. That way we only have to test a single endpoint, document a single endpoint, etc.

Perhaps in the future I'd rename the parameters to be more human-readable, but we'd do it for every endpoint, not just POST vs GET.

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