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

Analytics in Postgres #7594

Merged
merged 10 commits into from
Feb 6, 2024
Merged

Analytics in Postgres #7594

merged 10 commits into from
Feb 6, 2024

Conversation

normanrz
Copy link
Member

@normanrz normanrz commented Jan 29, 2024

  • Adds tracking of analytics events in our own postgres database
  • Adds the POST /api/analytics/ingest route to accept events from other wk instances, just like the current event-relay does

Steps to test:

  • fetch("/api/analytics/ingest", {method:"POST", headers:{"content-type": "application/json"}, body: JSON.stringify({"events":[{"event_type":"open_annotation","user_id":"123456789012345678901234","time":"1706510609682","user_properties":{"organization_id":"123456789012345678901234","is_organization_admin":false,"is_superuser":false,"webknossos_uri":"https://webknossos.org"},"event_properties":{"annotation_id":"123456789012345678901234","annotation_owner_multiuser_id":"123456789012345678901234","annotation_dataset_id":"123456789012345678901234"},"session_id":1706510609681}], "api_key": "dingdong"})}) (also try with other webknossos_uri and api_key)
  • Click around in wk and check the postgres database

TODOs:

  • Add migration

(Please delete unneeded items, merge only when none are left open)

@normanrz normanrz requested a review from fm3 January 29, 2024 16:13
@normanrz normanrz self-assigned this Jan 29, 2024
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I’ll test some once the CI is green

Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested some locally and saving the events of the own instance works.

Did you also test the ingesting? Do you have steps to test for that? Ah, I see them now. Will do some more tests now!

I did see some Unsuccessful WS request to https://events-relay.webknossos.org/events (ID: 0).Status: 500. in the logging. Did something change there?

I also added some comments on the code.

Also, reminder for the migration guide

app/models/analytics/AnalyticsService.scala Outdated Show resolved Hide resolved
app/models/analytics/AnalyticsService.scala Outdated Show resolved Hide resolved
app/controllers/Application.scala Outdated Show resolved Hide resolved
app/models/analytics/AnalyticsService.scala Outdated Show resolved Hide resolved
conf/application.conf Outdated Show resolved Hide resolved
conf/application.conf Outdated Show resolved Hide resolved
app/models/analytics/AnalyticsService.scala Outdated Show resolved Hide resolved
@fm3
Copy link
Member

fm3 commented Feb 1, 2024

Testing the ingest worked with the fetch you provided. It also worked with wrong api keys and wrong uris, though. I couldn’t immediately spot the error in the code. Can you reproduce this? If so, maybe you can debug why it is not strictly checked.

@normanrz
Copy link
Member Author

normanrz commented Feb 1, 2024

It also worked with wrong api keys and wrong uris

URIs that are not in the list are not protected. For these, you can send events with any key.

@bulldozer-boy bulldozer-boy bot merged commit d9cd527 into master Feb 6, 2024
2 checks passed
@bulldozer-boy bulldozer-boy bot deleted the analytics-postgres branch February 6, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants