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

Add pubsub connection #15389

Merged
merged 31 commits into from
Jan 29, 2020
Merged

Add pubsub connection #15389

merged 31 commits into from
Jan 29, 2020

Conversation

esloho
Copy link
Contributor

@esloho esloho commented Jan 15, 2020

Closes https://github.com/CartoDB/global_metrics/issues/31

As done in https://github.com/CartoDB/cartodb-central/pull/2598, this PR introduces the same PubSubTracker for sending events to PubSub from cartodb. Since in this case we must cover different authentication scenarios (like a future on-premise case), tracker initialization in this implementation includes additional cases.

The idea is to later remove the Segments events so only PubSub ones are sent, thus the duplication of the event models.

Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Nice! 👏 I've added a few minor comments.

We must remember to carefully test Instagram, Dropbox and Google integrations.

Gemfile Show resolved Hide resolved
config/initializers/at_exit.rb Outdated Show resolved Hide resolved
lib/carto/tracking/formats/pubsub.rb Outdated Show resolved Hide resolved
lib/carto/tracking/formats/pubsub.rb Outdated Show resolved Hide resolved
lib/carto/tracking/formats/pubsub.rb Outdated Show resolved Hide resolved
lib/carto/tracking/services/pubsub_tracker.rb Outdated Show resolved Hide resolved
lib/carto/tracking/services/pubsub_tracker.rb Outdated Show resolved Hide resolved
lib/carto/tracking/services/pubsub_tracker.rb Outdated Show resolved Hide resolved
@esloho esloho force-pushed the add-pubsub-connection branch 2 times, most recently from c53d069 to f477554 Compare January 21, 2020 12:47
@esloho
Copy link
Contributor Author

esloho commented Jan 21, 2020

After experiencing some problems with the async publishing of pubsub events in central we decided to try a different approach and replace it by sync publishing through resque jobs. Once we check that approach is working I'll perform the same change in this PR :)

Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

👌

@gonzaloriestra
Copy link
Contributor

✔️ Acceptance OK

Tested with several events that they are logged and also received in Pub/Sub.

@esloho
Copy link
Contributor Author

esloho commented Jan 23, 2020

After reviewing the full list of events and do some cleaning, these are the ones that should be published to pubsub at the moment:

  • map_created
  • map_deleted
  • map_exported
  • map_published
  • map_exported
  • dataset_created
  • dataset_deleted
  • analysis_created
  • anañysis_updated
  • analysis_deleted
  • oauth_app_created
  • oauth_app_deleted
  • oauth_app_user_created
  • oauth_app_user_deleted
  • quota_exceeded

Acceptance: These should appear in Rollbar as info messages and in the corresponding project in Google Cloud every time the action is performed by the user.

/cc @gonzaloriestra

@esloho
Copy link
Contributor Author

esloho commented Jan 24, 2020

Blocked until the necessary credentials for pubsub are created in production.

@esloho esloho removed the blocked label Jan 29, 2020
@esloho esloho merged commit 47c0170 into master Jan 29, 2020
@esloho esloho deleted the add-pubsub-connection branch January 29, 2020 15:16
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.

None yet

2 participants