)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4e3256b8e7c4b673136a710d1b297c47a10cbc38","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2023-07-24 16:53:15 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"proxy: Get rid of MetricsPrefixLoggerAdapter"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Related-Change: I0522b1953722ca96021a0002cf93432b973ce626"},{"line_number":10,"context_line":"Change-Id: Ieebafb19c3fa60334aff2914ab1ae70b8f140342"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7138e409_299ba3ff","line":8,"updated":"2023-07-25 14:28:58.000000000","message":"having some statement of \"why?\" would be useful - if only to help weary reviewers along ;-)","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5240b380d6e79bdb3c0d93fe52132f051f05d883","unresolved":true,"context_lines":[{"line_number":7,"context_line":"proxy: Get rid of MetricsPrefixLoggerAdapter"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It adds another layer of indirection and state for the sake of labeling;"},{"line_number":10,"context_line":"longer term it\u0027ll be easier to be explicit at the point of emission."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Related-Change: I0522b1953722ca96021a0002cf93432b973ce626"},{"line_number":13,"context_line":"Change-Id: Ieebafb19c3fa60334aff2914ab1ae70b8f140342"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"04ff9920_5c5980cb","line":10,"updated":"2023-09-05 17:04:36.000000000","message":"I like explicit!  That makes sense to me!","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4e3256b8e7c4b673136a710d1b297c47a10cbc38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3638902a_97c8a8c4","updated":"2023-07-25 14:28:58.000000000","message":"I\u0027m trying to understand the need for this change...is it because the following patches changes the metric names, with the existing names becoming \"legacy format\", and so we need a different strategy for differentiating the server types?","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5240b380d6e79bdb3c0d93fe52132f051f05d883","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"a51086a4_0b1e508b","updated":"2023-09-05 17:04:36.000000000","message":"I think this seems reasonable, I\u0027d like to understand the metrics tests and prefix normalization better.","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"c71b1c58fef8e83f3eb7f904a121579929d8865a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"aabec80e_02057f42","updated":"2023-09-11 03:56:07.000000000","message":"This makes the stats story way less confusing, for which I for one am happy about. Being a little more explicit can feel like a bit of a step back, but I agree we have a stats future direction in which this will help, so let\u0027s go!","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4e3256b8e7c4b673136a710d1b297c47a10cbc38","unresolved":true,"context_lines":[{"line_number":745,"context_line":""},{"line_number":746,"context_line":""},{"line_number":747,"context_line":"def record_cache_op_metrics("},{"line_number":748,"context_line":"        logger, layer, op_type, cache_state, resp\u003dNone):"},{"line_number":749,"context_line":"    \"\"\""},{"line_number":750,"context_line":"    Record a single cache operation into its corresponding metrics."},{"line_number":751,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"887e4853_35189fbb","line":748,"updated":"2023-07-25 14:28:58.000000000","message":"There\u0027s a lot of precedent for the concept that \"layer\" refers to being named \"server_type\", and that tends to be the name of the variable that callers pass in, so I\u0027d advocate being consistent with that","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4e3256b8e7c4b673136a710d1b297c47a10cbc38","unresolved":true,"context_lines":[{"line_number":749,"context_line":"    \"\"\""},{"line_number":750,"context_line":"    Record a single cache operation into its corresponding metrics."},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"    :param  logger: the metrics logger"},{"line_number":753,"context_line":"    :param  op_type: the name of the operation type, includes \u0027shard_listing\u0027,"},{"line_number":754,"context_line":"              \u0027shard_updating\u0027, and etc."},{"line_number":755,"context_line":"    :param  cache_state: the state of this cache operation. When it\u0027s"}],"source_content_type":"text/x-python","patch_set":4,"id":"882c83e9_7a4d7561","line":752,"updated":"2023-07-25 14:28:58.000000000","message":"missing param","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fee6181f26ebdf33b34e1079f67df82090819a3","unresolved":false,"context_lines":[{"line_number":749,"context_line":"    \"\"\""},{"line_number":750,"context_line":"    Record a single cache operation into its corresponding metrics."},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"    :param  logger: the metrics logger"},{"line_number":753,"context_line":"    :param  op_type: the name of the operation type, includes \u0027shard_listing\u0027,"},{"line_number":754,"context_line":"              \u0027shard_updating\u0027, and etc."},{"line_number":755,"context_line":"    :param  cache_state: the state of this cache operation. When it\u0027s"}],"source_content_type":"text/x-python","patch_set":4,"id":"14517c75_3af3d427","line":752,"in_reply_to":"882c83e9_7a4d7561","updated":"2023-09-05 17:25:24.000000000","message":"Done","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4e3256b8e7c4b673136a710d1b297c47a10cbc38","unresolved":true,"context_lines":[{"line_number":1641,"context_line":"    \"\"\""},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"    def __init__(self, server_type, app, ring, partition, logger, request,"},{"line_number":1644,"context_line":"                 node_iter\u003dNone, policy\u003dNone):"},{"line_number":1645,"context_line":"        self.server_type \u003d server_type"},{"line_number":1646,"context_line":"        self.app \u003d app"},{"line_number":1647,"context_line":"        self.ring \u003d ring"}],"source_content_type":"text/x-python","patch_set":4,"id":"cec3d83f_cfb04c2c","line":1644,"updated":"2023-07-25 14:28:58.000000000","message":"I\u0027d feel more comfortable adding a kwarg to a class constructor","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9520cbe8b709d7e74e7c4ee60298fbdfccd7648b","unresolved":true,"context_lines":[{"line_number":1641,"context_line":"    \"\"\""},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"    def __init__(self, server_type, app, ring, partition, logger, request,"},{"line_number":1644,"context_line":"                 node_iter\u003dNone, policy\u003dNone):"},{"line_number":1645,"context_line":"        self.server_type \u003d server_type"},{"line_number":1646,"context_line":"        self.app \u003d app"},{"line_number":1647,"context_line":"        self.ring \u003d ring"}],"source_content_type":"text/x-python","patch_set":4,"id":"e947b3dd_bf70b9d4","line":1644,"in_reply_to":"cec3d83f_cfb04c2c","updated":"2023-08-21 21:38:04.000000000","message":"It\u0027s a little tricky -- it\u0027s new, required, and we don\u0027t have enough context to provide a good default :-(","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fee6181f26ebdf33b34e1079f67df82090819a3","unresolved":false,"context_lines":[{"line_number":1641,"context_line":"    \"\"\""},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"    def __init__(self, server_type, app, ring, partition, logger, request,"},{"line_number":1644,"context_line":"                 node_iter\u003dNone, policy\u003dNone):"},{"line_number":1645,"context_line":"        self.server_type \u003d server_type"},{"line_number":1646,"context_line":"        self.app \u003d app"},{"line_number":1647,"context_line":"        self.ring \u003d ring"}],"source_content_type":"text/x-python","patch_set":4,"id":"17461364_0bde6ccc","line":1644,"in_reply_to":"e947b3dd_bf70b9d4","updated":"2023-09-05 17:25:24.000000000","message":"Done","commit_id":"c9110e2857b1090a59d03ef89dfaf1a807489d00"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"c71b1c58fef8e83f3eb7f904a121579929d8865a","unresolved":true,"context_lines":[{"line_number":1661,"context_line":"        None for an account or container ring."},{"line_number":1662,"context_line":"    \"\"\""},{"line_number":1663,"context_line":""},{"line_number":1664,"context_line":"    def __init__(self, server_type, app, ring, partition, logger, request,"},{"line_number":1665,"context_line":"                 node_iter\u003dNone, policy\u003dNone):"},{"line_number":1666,"context_line":"        self.server_type \u003d server_type"},{"line_number":1667,"context_line":"        self.app \u003d app"}],"source_content_type":"text/x-python","patch_set":6,"id":"e20c5c21_8f05621a","line":1664,"range":{"start_line":1664,"start_character":23,"end_line":1664,"end_character":34},"updated":"2023-09-11 03:56:07.000000000","message":"Changing the first required parameter of the NodeIter seems like a big move. It does make sense, but I do worry about fallout from this.\n\nEDIT: Looking in the codebase I don\u0027t actaully see that many references to them outside of the controllers.. which in hindsight makes alot of sense :P","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5240b380d6e79bdb3c0d93fe52132f051f05d883","unresolved":true,"context_lines":[{"line_number":1711,"context_line":"        extra_handoffs \u003d handoffs - self.expected_handoffs"},{"line_number":1712,"context_line":"        if extra_handoffs \u003e 0:"},{"line_number":1713,"context_line":"            self.logger.increment(\u0027%s.handoff_count\u0027 %"},{"line_number":1714,"context_line":"                                  self.server_type.lower())"},{"line_number":1715,"context_line":"            self.logger.warning("},{"line_number":1716,"context_line":"                \u0027Handoff requested (%d)\u0027 % handoffs)"},{"line_number":1717,"context_line":"            if (extra_handoffs \u003d\u003d self.num_primary_nodes):"}],"source_content_type":"text/x-python","patch_set":6,"id":"89c1322b_77abda76","line":1714,"updated":"2023-09-05 17:04:36.000000000","message":"maybe we should normalize in `__init__`?","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"c71b1c58fef8e83f3eb7f904a121579929d8865a","unresolved":true,"context_lines":[{"line_number":1711,"context_line":"        extra_handoffs \u003d handoffs - self.expected_handoffs"},{"line_number":1712,"context_line":"        if extra_handoffs \u003e 0:"},{"line_number":1713,"context_line":"            self.logger.increment(\u0027%s.handoff_count\u0027 %"},{"line_number":1714,"context_line":"                                  self.server_type.lower())"},{"line_number":1715,"context_line":"            self.logger.warning("},{"line_number":1716,"context_line":"                \u0027Handoff requested (%d)\u0027 % handoffs)"},{"line_number":1717,"context_line":"            if (extra_handoffs \u003d\u003d self.num_primary_nodes):"}],"source_content_type":"text/x-python","patch_set":6,"id":"5309ec0b_92d39464","line":1714,"in_reply_to":"89c1322b_77abda76","updated":"2023-09-11 03:56:07.000000000","message":"+1 to this. Seems weird to have to worry about this being in a different case.\n\nIt is rather annoying that all the server_types in the proxy controllers are all titled, I guess we\u0027re worried that could be passed here. Why are they even titled? what purpose does that serve? Almost everywhere we want to then use self.server_type is defined for .lower()\u0027ed. Not to mention in other swift services, like container replicator or auditor they\u0027re defined lowercase.","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fee6181f26ebdf33b34e1079f67df82090819a3","unresolved":true,"context_lines":[{"line_number":1051,"context_line":"            if ml and bytes_transferred \u003c ml:"},{"line_number":1052,"context_line":"                self.logger.warning("},{"line_number":1053,"context_line":"                    \u0027Client disconnected without sending enough data\u0027)"},{"line_number":1054,"context_line":"                self.logger.increment(\u0027object.client_disconnects\u0027)"},{"line_number":1055,"context_line":"                raise HTTPClientDisconnect(request\u003dreq)"},{"line_number":1056,"context_line":""},{"line_number":1057,"context_line":"            trail_md \u003d self._get_footers(req)"}],"source_content_type":"text/x-python","patch_set":6,"id":"0bc3435f_4f78f0b5","line":1054,"updated":"2023-09-05 17:25:24.000000000","message":"yep, I\u0027m warming to these metric names being more explicit in the code","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"}],"test/unit/proxy/controllers/test_base.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5240b380d6e79bdb3c0d93fe52132f051f05d883","unresolved":true,"context_lines":[{"line_number":673,"context_line":"            self.logger, \u0027container\u0027, \u0027shard_listing\u0027, \u0027infocache_hit\u0027)"},{"line_number":674,"context_line":"        self.assertEqual("},{"line_number":675,"context_line":"            self.logger.statsd_client.get_increment_counts().get("},{"line_number":676,"context_line":"                \u0027container.shard_listing.infocache.hit\u0027),"},{"line_number":677,"context_line":"            1)"},{"line_number":678,"context_line":"        record_cache_op_metrics("},{"line_number":679,"context_line":"            self.logger, \u0027container\u0027, \u0027shard_listing\u0027, \u0027hit\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da5ab8b3_886c8bdd","line":676,"updated":"2023-09-05 17:04:36.000000000","message":"should I assume this isn\u0027t actually changing the metrics that get emitted, but the metrics prefix adapter obfuscated the actual metrics.","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fee6181f26ebdf33b34e1079f67df82090819a3","unresolved":true,"context_lines":[{"line_number":673,"context_line":"            self.logger, \u0027container\u0027, \u0027shard_listing\u0027, \u0027infocache_hit\u0027)"},{"line_number":674,"context_line":"        self.assertEqual("},{"line_number":675,"context_line":"            self.logger.statsd_client.get_increment_counts().get("},{"line_number":676,"context_line":"                \u0027container.shard_listing.infocache.hit\u0027),"},{"line_number":677,"context_line":"            1)"},{"line_number":678,"context_line":"        record_cache_op_metrics("},{"line_number":679,"context_line":"            self.logger, \u0027container\u0027, \u0027shard_listing\u0027, \u0027hit\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"864fb56d_0f059a10","line":676,"in_reply_to":"da5ab8b3_886c8bdd","updated":"2023-09-05 17:25:24.000000000","message":"I think the change here is because self.logger is a plain old DebugLogger without a MetricsPrefixAdapter so on master the server type wasn\u0027t automagically pre-pended. But now the server type is explicitly set in the call to record_cache_op_metrics, so we see it captured in DebugLogger.","commit_id":"9f385c07f3ea3a55c8922c65124d14202e168351"}]}