)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"103b42bd74708bc943c71f1c15f25ca4fa934d6d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2ad101bf_ddcdd7ea","updated":"2023-09-22 14:32:58.000000000","message":"I like the more granular naming","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5469fac56018432156cb84aeae0899071ca8e757","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"52b7aa39_87306936","updated":"2023-09-22 21:13:24.000000000","message":"The more granular naming is surely worth pursuing -- maybe I should wait until I can just do it with proper labels, though.","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"103b42bd74708bc943c71f1c15f25ca4fa934d6d","unresolved":true,"context_lines":[{"line_number":1735,"context_line":"            node \u003d self.primary_nodes.pop(0)"},{"line_number":1736,"context_line":"            if self.app.error_limited(node):"},{"line_number":1737,"context_line":"                self.logger.increment(\u0027%s.error_limited.primary\u0027 %"},{"line_number":1738,"context_line":"                                      self.server_type.lower())"},{"line_number":1739,"context_line":"            else:"},{"line_number":1740,"context_line":"                yield node"},{"line_number":1741,"context_line":"                if not self.app.error_limited(node):"}],"source_content_type":"text/x-python","patch_set":2,"id":"370a211f_edd45137","line":1738,"updated":"2023-09-22 14:32:58.000000000","message":"do you think the metric emitted by app.error_limited() is still useful? - these new metrics seem to be emitted at the same time but with more detailed naming.","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5469fac56018432156cb84aeae0899071ca8e757","unresolved":true,"context_lines":[{"line_number":1735,"context_line":"            node \u003d self.primary_nodes.pop(0)"},{"line_number":1736,"context_line":"            if self.app.error_limited(node):"},{"line_number":1737,"context_line":"                self.logger.increment(\u0027%s.error_limited.primary\u0027 %"},{"line_number":1738,"context_line":"                                      self.server_type.lower())"},{"line_number":1739,"context_line":"            else:"},{"line_number":1740,"context_line":"                yield node"},{"line_number":1741,"context_line":"                if not self.app.error_limited(node):"}],"source_content_type":"text/x-python","patch_set":2,"id":"e6cc08e8_b2fce991","line":1738,"in_reply_to":"370a211f_edd45137","updated":"2023-09-22 21:13:24.000000000","message":"Bah -- somehow I forgot about what those new metrics from https://review.opendev.org/c/openstack/swift/+/863446 actually meant. So maybe we **already** have graphs to tell us whether https://review.opendev.org/c/openstack/swift/+/890527 is doing what we expect?\n\nMaybe it would be better to pass some labels to `self.app.error_limited()` and just update the metrics we\u0027re already emitting?","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5863c830658c5fb541f164d90e5535ac31ca5164","unresolved":true,"context_lines":[{"line_number":1735,"context_line":"            node \u003d self.primary_nodes.pop(0)"},{"line_number":1736,"context_line":"            if self.app.error_limited(node):"},{"line_number":1737,"context_line":"                self.logger.increment(\u0027%s.error_limited.primary\u0027 %"},{"line_number":1738,"context_line":"                                      self.server_type.lower())"},{"line_number":1739,"context_line":"            else:"},{"line_number":1740,"context_line":"                yield node"},{"line_number":1741,"context_line":"                if not self.app.error_limited(node):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f9e0425_d398f965","line":1738,"in_reply_to":"e6cc08e8_b2fce991","updated":"2023-09-25 09:59:46.000000000","message":"I\u0027m not sure how much I like that error_limited() emits a metric - I mean, it\u0027s just a getter helper method, why should it be inferring such significance in it being called. OK, so currently it is only called when we want to consume a node, so it\u0027s a moot point, but in principle I feel it is the caller that should determine the significance of the node being error limited (\"oh, this is bad, let\u0027s emit a metric\" vs \"NM, I was just curious\").\n\nThe fact that we\u0027d have to pass the context of the caller to the error_limited() method makes me feel that the caller has the context, let the caller emit the metric (i.e *this* patch)?\n\nBut, there\u0027s always more than one way to craft it...","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"103b42bd74708bc943c71f1c15f25ca4fa934d6d","unresolved":true,"context_lines":[{"line_number":1754,"context_line":"                if not self.app.error_limited(node):"},{"line_number":1755,"context_line":"                    self.nodes_left -\u003d 1"},{"line_number":1756,"context_line":"                    if self.nodes_left \u003c\u003d 0:"},{"line_number":1757,"context_line":"                        return"},{"line_number":1758,"context_line":""},{"line_number":1759,"context_line":"    def _annotate_node(self, node):"},{"line_number":1760,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"b303fb3d_268730d8","line":1757,"updated":"2023-09-22 14:32:58.000000000","message":"how about counting how many primary/handoff nodes are error_limited and if non-zero then increment:\n\n```\n\u003cserver_type\u003e.error_limited.[primary|handoff|total].\u003ccount\u003e\n```\n\nthat might be interesting when there\u0027s a hotspot event","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5863c830658c5fb541f164d90e5535ac31ca5164","unresolved":true,"context_lines":[{"line_number":1754,"context_line":"                if not self.app.error_limited(node):"},{"line_number":1755,"context_line":"                    self.nodes_left -\u003d 1"},{"line_number":1756,"context_line":"                    if self.nodes_left \u003c\u003d 0:"},{"line_number":1757,"context_line":"                        return"},{"line_number":1758,"context_line":""},{"line_number":1759,"context_line":"    def _annotate_node(self, node):"},{"line_number":1760,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa2cde4e_5f10bf2e","line":1757,"in_reply_to":"92233646_c68dd347","updated":"2023-09-25 09:59:46.000000000","message":"yeah, policy would be good.\n\nIDK about every request or just requests where \u003e0 nodes were found to be error limited.","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5469fac56018432156cb84aeae0899071ca8e757","unresolved":true,"context_lines":[{"line_number":1754,"context_line":"                if not self.app.error_limited(node):"},{"line_number":1755,"context_line":"                    self.nodes_left -\u003d 1"},{"line_number":1756,"context_line":"                    if self.nodes_left \u003c\u003d 0:"},{"line_number":1757,"context_line":"                        return"},{"line_number":1758,"context_line":""},{"line_number":1759,"context_line":"    def _annotate_node(self, node):"},{"line_number":1760,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"92233646_c68dd347","line":1757,"in_reply_to":"b303fb3d_268730d8","updated":"2023-09-22 21:13:24.000000000","message":"Oh, good idea -- are you thinking one of these per proxy-server request?\n\nWe\u0027d surely want to get some policy info in there, too, though -- seeing a `object.error_limited.primary.2` should elicit very different reactions from ops depending on whether it\u0027s for a 3x replicated policy or a 8+4 EC policy.","commit_id":"c0830dd3e0b22bc077e1fb4caf74f4fada613e94"}]}