From c0d7e38e191d6dad9812d5f8f9291578449c7528 Mon Sep 17 00:00:00 2001 From: Deimos Date: Wed, 16 Oct 2019 18:57:05 -0600 Subject: [PATCH] Use SQLAlchemy object lifecycle event for metrics The creation of the Theme Previews page showed that the way I was doing metrics for database model objects being created wasn't very good. Whenever someone loaded the Theme Previews page, the "total topics" metric would increase by 2, "total comments" by 4, and "exemplary labels" by 1, because the page is creating that many fake objects and the metrics were being sent in each class's __init__ method. This changes to take advantage of SQLAlchemy's object lifecycle event for "pending to persistent", which only triggers when an object is actually persisted to the database. When this event happens, the object's _update_creation_metric method is called, and all metric updates have been moved into that method now. --- tildes/tildes/database.py | 11 ++++++++++- tildes/tildes/models/comment/comment.py | 1 + tildes/tildes/models/comment/comment_label.py | 3 ++- tildes/tildes/models/comment/comment_vote.py | 1 + tildes/tildes/models/database_model.py | 12 ++++++++++++ tildes/tildes/models/group/group_subscription.py | 1 + tildes/tildes/models/message/message.py | 2 ++ tildes/tildes/models/topic/topic.py | 7 +++---- tildes/tildes/models/topic/topic_vote.py | 1 + 9 files changed, 33 insertions(+), 6 deletions(-) diff --git a/tildes/tildes/database.py b/tildes/tildes/database.py index 0ddf888..c1cf4b4 100644 --- a/tildes/tildes/database.py +++ b/tildes/tildes/database.py @@ -7,7 +7,7 @@ from typing import Callable, Type from pyramid.config import Configurator from pyramid.request import Request -from sqlalchemy import engine_from_config +from sqlalchemy import engine_from_config, event from sqlalchemy.orm import sessionmaker from sqlalchemy.orm.session import Session from sqlalchemy.pool import NullPool @@ -92,6 +92,15 @@ def includeme(config: Configurator) -> None: reify=True, ) + # add a listener to the session to update database model creation metrics when + # any object goes through the "pending to persistent" state change + event.listen( + session_factory, + "pending_to_persistent", + # pylint: disable=protected-access + lambda session, instance: instance._update_creation_metric(), + ) + config.add_request_method(query_factory, "query") config.add_request_method(obtain_lock, "obtain_lock") diff --git a/tildes/tildes/models/comment/comment.py b/tildes/tildes/models/comment/comment.py index 185c69b..c6ed266 100644 --- a/tildes/tildes/models/comment/comment.py +++ b/tildes/tildes/models/comment/comment.py @@ -141,6 +141,7 @@ class Comment(DatabaseModel): self.markdown = markdown self.parent_comment = parent_comment + def _update_creation_metric(self) -> None: incr_counter("comments") def __acl__(self) -> Sequence[Tuple[str, Any, str]]: diff --git a/tildes/tildes/models/comment/comment_label.py b/tildes/tildes/models/comment/comment_label.py index 9170cc3..cb3357d 100644 --- a/tildes/tildes/models/comment/comment_label.py +++ b/tildes/tildes/models/comment/comment_label.py @@ -64,7 +64,8 @@ class CommentLabel(DatabaseModel): self.weight = weight self.reason = reason - incr_counter("comment_labels", label=label.name) + def _update_creation_metric(self) -> None: + incr_counter("comment_labels", label=self.label.name) @property def name(self) -> str: diff --git a/tildes/tildes/models/comment/comment_vote.py b/tildes/tildes/models/comment/comment_vote.py index cae5913..fa9e03f 100644 --- a/tildes/tildes/models/comment/comment_vote.py +++ b/tildes/tildes/models/comment/comment_vote.py @@ -48,4 +48,5 @@ class CommentVote(DatabaseModel): self.user = user self.comment = comment + def _update_creation_metric(self) -> None: incr_counter("votes", target_type="comment") diff --git a/tildes/tildes/models/database_model.py b/tildes/tildes/models/database_model.py index e6144c1..f413a45 100644 --- a/tildes/tildes/models/database_model.py +++ b/tildes/tildes/models/database_model.py @@ -97,6 +97,18 @@ class DatabaseModelBase: return utc_now() - self.created_time # type: ignore + def _update_creation_metric(self) -> None: + """Update the metric tracking creations of this model type. + + This function will be attached to the SQLAlchemy Object Lifecycle event for the + "pending to persistent" transition, which occurs when an object is persisted to + the database. This ensures that the metric is only updated when an object is + truly created in the database, not just whenever the model class is initialized. + + Model classes that have a creation metric should override this method. + """ + pass + def _validate_new_value(self, attribute: str, value: Any) -> Any: """Validate the new value for a column. diff --git a/tildes/tildes/models/group/group_subscription.py b/tildes/tildes/models/group/group_subscription.py index 6638c39..67ddfe6 100644 --- a/tildes/tildes/models/group/group_subscription.py +++ b/tildes/tildes/models/group/group_subscription.py @@ -48,4 +48,5 @@ class GroupSubscription(DatabaseModel): self.user = user self.group = group + def _update_creation_metric(self) -> None: incr_counter("subscriptions") diff --git a/tildes/tildes/models/message/message.py b/tildes/tildes/models/message/message.py index 50f4da6..5df1109 100644 --- a/tildes/tildes/models/message/message.py +++ b/tildes/tildes/models/message/message.py @@ -118,6 +118,7 @@ class MessageConversation(DatabaseModel): self.markdown = markdown self.rendered_html = convert_markdown_to_safe_html(markdown) + def _update_creation_metric(self) -> None: incr_counter("messages", type="conversation") def __acl__(self) -> Sequence[Tuple[str, Any, str]]: @@ -243,6 +244,7 @@ class MessageReply(DatabaseModel): self.markdown = markdown self.rendered_html = convert_markdown_to_safe_html(markdown) + def _update_creation_metric(self) -> None: incr_counter("messages", type="reply") @property diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index 45d5864..fd06e46 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -217,8 +217,6 @@ class Topic(DatabaseModel): new_topic.topic_type = TopicType.TEXT new_topic.markdown = markdown - incr_counter("topics", type="text") - return new_topic @classmethod @@ -230,10 +228,11 @@ class Topic(DatabaseModel): new_topic.topic_type = TopicType.LINK new_topic.link = link - incr_counter("topics", type="link") - return new_topic + def _update_creation_metric(self) -> None: + incr_counter("topics", type=self.topic_type.name.lower()) + def __acl__(self) -> Sequence[Tuple[str, Any, str]]: """Pyramid security ACL.""" acl = [] diff --git a/tildes/tildes/models/topic/topic_vote.py b/tildes/tildes/models/topic/topic_vote.py index 2bdef07..4b6a37f 100644 --- a/tildes/tildes/models/topic/topic_vote.py +++ b/tildes/tildes/models/topic/topic_vote.py @@ -48,4 +48,5 @@ class TopicVote(DatabaseModel): self.user = user self.topic = topic + def _update_creation_metric(self) -> None: incr_counter("votes", target_type="topic")