)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5056e27bb970d7709247e949d1204609ebcb350f","unresolved":true,"context_lines":[{"line_number":46,"context_line":"  the resource type is not known, but the empty string is used in"},{"line_number":47,"context_line":"  metric *label values* for consistency with the value of other"},{"line_number":48,"context_line":"  unknown labels such as \u0027account\u0027."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Change-Id: Ife41d80c4674ddd9ab3cefbd6e0dfdcce8eb79ad"},{"line_number":51,"context_line":"Signed-off-by: Alistair Coles \u003calistairncoles@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"31317aac_548aca82","line":49,"updated":"2026-07-10 16:40:43.000000000","message":"I should add an UpgradeImpact flag w.r.t. the change to the \u0027UNKNOWN\u0027 resource label value","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e8cf62121eb258a1e4fc61f7670e9d3634193b75","unresolved":false,"context_lines":[{"line_number":46,"context_line":"  the resource type is not known, but the empty string is used in"},{"line_number":47,"context_line":"  metric *label values* for consistency with the value of other"},{"line_number":48,"context_line":"  unknown labels such as \u0027account\u0027."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Change-Id: Ife41d80c4674ddd9ab3cefbd6e0dfdcce8eb79ad"},{"line_number":51,"context_line":"Signed-off-by: Alistair Coles \u003calistairncoles@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"522473d1_783c6199","line":49,"in_reply_to":"2bee1909_4fb8dfac","updated":"2026-07-29 13:37:06.000000000","message":"Done","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7b38afbbea5286d79332c992d07b3ddac5e1a36a","unresolved":true,"context_lines":[{"line_number":46,"context_line":"  the resource type is not known, but the empty string is used in"},{"line_number":47,"context_line":"  metric *label values* for consistency with the value of other"},{"line_number":48,"context_line":"  unknown labels such as \u0027account\u0027."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Change-Id: Ife41d80c4674ddd9ab3cefbd6e0dfdcce8eb79ad"},{"line_number":51,"context_line":"Signed-off-by: Alistair Coles \u003calistairncoles@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"2bee1909_4fb8dfac","line":49,"in_reply_to":"31317aac_548aca82","updated":"2026-07-28 12:24:12.000000000","message":"done in previous patch","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"eff8fc3c2d90c5134d1383a4594ea96bbdc32391","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dd03b2de_b15483dd","updated":"2026-07-09 13:55:37.000000000","message":"Maybe I misunderstood - is this expected?\n\n```\n\u003e\u003e\u003e from swift.common.utils import LabelsMap\n\u003e\u003e\u003e m \u003d LabelsMap()\n\u003e\u003e\u003e m.setdefault(\u0027a\u0027, None)\n\u003e\u003e\u003e m.setdefault(\u0027a\u0027, \u0027AUTH_x\u0027)\n\u003e\u003e\u003e m[\u0027a\u0027]\n\u0027\u0027\n```","commit_id":"b2c8feb218efce69f22c3223def600cb69b65e8f"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"405dcbadfb8f83e9fa6ccb858f4faa7f60c1a0ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"08480b3a_b2fe4c0e","updated":"2026-07-14 00:09:51.000000000","message":"Asked Claude to look at this patch, it said an empty label value is equivalent to the label being absent with statsd_exporter and Prometheus, that means it won\u0027t be the case that \"metrics will now always have the label keys\"?\n\n```\n1. Operator impact: empty label values on the wire. This is my main question. I built actual lines for each label mode with the new labels:\n\ngraphite   swift_proxy_server_request_timing;account\u003d;api\u003dswift;method\u003dGET;resource\u003d;status\u003d200:42|ms\ninfluxdb   swift_proxy_server_request_timing,account\u003d,api\u003dswift,method\u003dGET,resource\u003d,status\u003d200:42|ms\ndogstatsd  swift_proxy_server_request_timing:42|ms|#account:,api:swift,method:GET,resource:,status:200\nlibrato    swift_proxy_server_request_timing#account\u003d,api\u003dswift,method\u003dGET,resource\u003d,status\u003d200:42|ms\n\nGraphite\u0027s tagged-metric spec requires tag values of length ≥ 1, and InfluxDB line protocol disallows empty tag values — a native carbon or influx consumer may reject these lines entirely, whereas previously the key was simply omitted. And for the statsd_exporter→Prometheus path, an empty label value is equivalent to the label being absent, so \"metrics will now always have the label keys\" won\u0027t be observable there anyway.\n```","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"7d927fe369527a2c5c3539b71379f1e88c628506","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"710b1674_143d56b4","updated":"2026-07-10 21:25:41.000000000","message":"LGTM! one question is that for some labels we are using non-string type value, such as status label. If some code uses value of None as default for such labels, the result metric would have label of \u0027\u0027, which seems a bit string for a label expecting int value","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b6d3ed9c9d779f89627b8bb3e96c52c8bfb8286d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"089440a8_2906d529","updated":"2026-07-14 12:20:17.000000000","message":"Seems like we need to consider the use of empty label values some more, given what Jianjian has pointed out.\n\nI think the commit message is correct: Swift will emit labels even with empty values, regardless of whether collectors drop them/ignore them. However, we don\u0027t want to break collections by sending invalid metric strings.","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"dcf44e5ea84e9c4e0f9114e0d65921b3de93845d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"376cdb37_71e48ae4","in_reply_to":"089440a8_2906d529","updated":"2026-07-17 19:36:20.000000000","message":"it seems that for statsd_exporter or otel receivers, empty string label value actually works. Although Jianjian is right we also need to consider other receivers","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b6d3ed9c9d779f89627b8bb3e96c52c8bfb8286d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f45540b6_6edb1078","in_reply_to":"710b1674_143d56b4","updated":"2026-07-14 12:20:17.000000000","message":"all values are stringified when they are serialized to the wire format by the client\n\nPerhaps the LabelsMap should also stringigy values??","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"78822b35d80a21c5c09c94550ffa95db8ca660c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ec780fa8_73b6cf3d","updated":"2026-07-27 14:38:59.000000000","message":"Maybe Jianjian wants to have a look to - LGTM.","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"cabb6b48721df748e2a8d2bfb3671bf5592a9d8e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"8f2a6c19_8b964246","updated":"2026-07-31 15:07:07.000000000","message":"LGTM, let\u0027s merge it!","commit_id":"df74741e00dfab87955295e5e0a1dd661daba253"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"341e37ff787ca688153ab1227f42fad243106400","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"5e388d33_f25dd55a","updated":"2026-07-31 16:25:37.000000000","message":"rebased, fixed a merge conflict in a test comment, approving based on Christian\u0027s earlier approval","commit_id":"79326c2525a6cef3c138f34c1933de2ed1422020"}],"swift/common/middleware/proxy_logging.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"78822b35d80a21c5c09c94550ffa95db8ca660c7","unresolved":true,"context_lines":[{"line_number":584,"context_line":"                if base_labels[\u0027resource\u0027] is None:"},{"line_number":585,"context_line":"                    # allow a later middleware to update the resource label"},{"line_number":586,"context_line":"                    # once the full swift path is known."},{"line_number":587,"context_line":"                    base_labels.pop(\u0027resource\u0027)"},{"line_number":588,"context_line":"                base_labels[\u0027api\u0027] \u003d \u0027S3\u0027"},{"line_number":589,"context_line":"            else:"},{"line_number":590,"context_line":"                base_labels[\u0027api\u0027] \u003d \u0027swift\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"a053a324_7f20d223","side":"PARENT","line":587,"updated":"2026-07-27 14:38:59.000000000","message":"The removal of the .pop() hides the underlying issue before this patch: only S3 requests were removing the `resource: None`, which could then be properly set afterwards. That was not the case for non-S3 requests - this is only possible with this patch.\n\nSo at the end this patch also reduces code complexity - nice!","commit_id":"edbe8bdf86e83c02a4e238b238d119767ebe7ab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7b38afbbea5286d79332c992d07b3ddac5e1a36a","unresolved":false,"context_lines":[{"line_number":584,"context_line":"                if base_labels[\u0027resource\u0027] is None:"},{"line_number":585,"context_line":"                    # allow a later middleware to update the resource label"},{"line_number":586,"context_line":"                    # once the full swift path is known."},{"line_number":587,"context_line":"                    base_labels.pop(\u0027resource\u0027)"},{"line_number":588,"context_line":"                base_labels[\u0027api\u0027] \u003d \u0027S3\u0027"},{"line_number":589,"context_line":"            else:"},{"line_number":590,"context_line":"                base_labels[\u0027api\u0027] \u003d \u0027swift\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"53905fed_32718c36","side":"PARENT","line":587,"in_reply_to":"a053a324_7f20d223","updated":"2026-07-28 12:24:12.000000000","message":"Acknowledged","commit_id":"edbe8bdf86e83c02a4e238b238d119767ebe7ab8"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"36266054998ad485a8193b6e4cd9a2231a672237","unresolved":true,"context_lines":[{"line_number":532,"context_line":"        \"\"\""},{"line_number":533,"context_line":"        resource \u003d self.get_resource_type_from_aco(req, acc, cont, obj)"},{"line_number":534,"context_line":"        metric_method \u003d self.statsd_metric_method(self.method_from_req(req))"},{"line_number":535,"context_line":"        labels \u003d LabelsMap(account\u003dacc or None,  # acc might be \u0027\u0027"},{"line_number":536,"context_line":"                           method\u003dmetric_method,"},{"line_number":537,"context_line":"                           resource\u003dresource,"},{"line_number":538,"context_line":"                           api\u003dNone)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ac07c37_0bdebdcb","line":535,"updated":"2026-07-27 21:16:44.000000000","message":"could acc be \u0027\u0027 here from get_aco_from_path?","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7b38afbbea5286d79332c992d07b3ddac5e1a36a","unresolved":true,"context_lines":[{"line_number":532,"context_line":"        \"\"\""},{"line_number":533,"context_line":"        resource \u003d self.get_resource_type_from_aco(req, acc, cont, obj)"},{"line_number":534,"context_line":"        metric_method \u003d self.statsd_metric_method(self.method_from_req(req))"},{"line_number":535,"context_line":"        labels \u003d LabelsMap(account\u003dacc or None,  # acc might be \u0027\u0027"},{"line_number":536,"context_line":"                           method\u003dmetric_method,"},{"line_number":537,"context_line":"                           resource\u003dresource,"},{"line_number":538,"context_line":"                           api\u003dNone)"}],"source_content_type":"text/x-python","patch_set":6,"id":"999276d1_cf632104","line":535,"in_reply_to":"1ac07c37_0bdebdcb","updated":"2026-07-28 12:24:12.000000000","message":"yes:\n```\nfrom swift.common.middleware.proxy_logging import ProxyLoggingMiddleware\nmw \u003d ProxyLoggingMiddleware(None, {})\nmw.get_aco_from_path(\u0027/v1/\u0027)\n(\u0027\u0027, None, None)\n```","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"59ebe810c3aa77ddd440a44a186fb5707011bcd6","unresolved":false,"context_lines":[{"line_number":532,"context_line":"        \"\"\""},{"line_number":533,"context_line":"        resource \u003d self.get_resource_type_from_aco(req, acc, cont, obj)"},{"line_number":534,"context_line":"        metric_method \u003d self.statsd_metric_method(self.method_from_req(req))"},{"line_number":535,"context_line":"        labels \u003d LabelsMap(account\u003dacc or None,  # acc might be \u0027\u0027"},{"line_number":536,"context_line":"                           method\u003dmetric_method,"},{"line_number":537,"context_line":"                           resource\u003dresource,"},{"line_number":538,"context_line":"                           api\u003dNone)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a3165d8b_f1679b81","line":535,"in_reply_to":"999276d1_cf632104","updated":"2026-07-28 16:36:39.000000000","message":"Acknowledged","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"78822b35d80a21c5c09c94550ffa95db8ca660c7","unresolved":true,"context_lines":[{"line_number":540,"context_line":"            labels[\u0027container\u0027] \u003d cont"},{"line_number":541,"context_line":"        return labels"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"    def get_current_labels(self, req, acc, cont, obj):"},{"line_number":544,"context_line":"        \"\"\""},{"line_number":545,"context_line":"        Returns a LabelsMap of labels associated with the request, including"},{"line_number":546,"context_line":"        the following keys: \u0027account\u0027, \u0027resource\u0027, \u0027method\u0027, \u0027api\u0027. Keys whose"}],"source_content_type":"text/x-python","patch_set":6,"id":"924e3f90_2a27bd79","line":543,"updated":"2026-07-27 14:38:59.000000000","message":"First I was a bit hesitant because of the overlap with get_base_labels. After looking more closely at this, it seems to me that having two methods is the cleaner (more readable) version.","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"ce27a208537298ce2a85fba2e98b012a329dc472","unresolved":true,"context_lines":[{"line_number":540,"context_line":"            labels[\u0027container\u0027] \u003d cont"},{"line_number":541,"context_line":"        return labels"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"    def get_current_labels(self, req, acc, cont, obj):"},{"line_number":544,"context_line":"        \"\"\""},{"line_number":545,"context_line":"        Returns a LabelsMap of labels associated with the request, including"},{"line_number":546,"context_line":"        the following keys: \u0027account\u0027, \u0027resource\u0027, \u0027method\u0027, \u0027api\u0027. Keys whose"}],"source_content_type":"text/x-python","patch_set":6,"id":"d7e43d9c_85ba3afa","line":543,"in_reply_to":"924e3f90_2a27bd79","updated":"2026-07-27 14:53:45.000000000","message":"Well, and there is removing the duplicate code in 998790: proxy_logging: introduce ChainLabelsMap | https://review.opendev.org/c/openstack/swift/+/998790 - nice!","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7b38afbbea5286d79332c992d07b3ddac5e1a36a","unresolved":false,"context_lines":[{"line_number":540,"context_line":"            labels[\u0027container\u0027] \u003d cont"},{"line_number":541,"context_line":"        return labels"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"    def get_current_labels(self, req, acc, cont, obj):"},{"line_number":544,"context_line":"        \"\"\""},{"line_number":545,"context_line":"        Returns a LabelsMap of labels associated with the request, including"},{"line_number":546,"context_line":"        the following keys: \u0027account\u0027, \u0027resource\u0027, \u0027method\u0027, \u0027api\u0027. Keys whose"}],"source_content_type":"text/x-python","patch_set":6,"id":"84eeae38_76e9f47a","line":543,"in_reply_to":"d7e43d9c_85ba3afa","updated":"2026-07-28 12:24:12.000000000","message":"I considered a single parameterised method but opted for simplicity (plus the follow-up condenses the code)","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"78822b35d80a21c5c09c94550ffa95db8ca660c7","unresolved":true,"context_lines":[{"line_number":543,"context_line":"    def get_current_labels(self, req, acc, cont, obj):"},{"line_number":544,"context_line":"        \"\"\""},{"line_number":545,"context_line":"        Returns a LabelsMap of labels associated with the request, including"},{"line_number":546,"context_line":"        the following keys: \u0027account\u0027, \u0027resource\u0027, \u0027method\u0027, \u0027api\u0027. Keys whose"},{"line_number":547,"context_line":"        value is not yet known are not set."},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"        :param req: a swob.Request"}],"source_content_type":"text/x-python","patch_set":6,"id":"6c76cca1_a6340fa3","line":546,"updated":"2026-07-27 14:38:59.000000000","message":"nit: \u0027api\u0027 is never included?","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7b38afbbea5286d79332c992d07b3ddac5e1a36a","unresolved":false,"context_lines":[{"line_number":543,"context_line":"    def get_current_labels(self, req, acc, cont, obj):"},{"line_number":544,"context_line":"        \"\"\""},{"line_number":545,"context_line":"        Returns a LabelsMap of labels associated with the request, including"},{"line_number":546,"context_line":"        the following keys: \u0027account\u0027, \u0027resource\u0027, \u0027method\u0027, \u0027api\u0027. Keys whose"},{"line_number":547,"context_line":"        value is not yet known are not set."},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"        :param req: a swob.Request"}],"source_content_type":"text/x-python","patch_set":6,"id":"e0279cb2_643429bf","line":546,"in_reply_to":"6c76cca1_a6340fa3","updated":"2026-07-28 12:24:12.000000000","message":"Done","commit_id":"28212aff68c2de53446990f48674678887bbe0a0"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"de3eb92e058ed7ca021f2d7ab7657fab374b668c","unresolved":true,"context_lines":[{"line_number":612,"context_line":"        else:"},{"line_number":613,"context_line":"            # expected in the right-most proxy_logging instance"},{"line_number":614,"context_line":"            current_labels \u003d self.get_current_labels(req, acc, cont, obj)"},{"line_number":615,"context_line":"            # if these base_labels are not already known then this is the best"},{"line_number":616,"context_line":"            # idea we have; set them in base_labels so that they are visible to"},{"line_number":617,"context_line":"            # the left-most proxy_logging instance"},{"line_number":618,"context_line":"            base_labels.setdefault(\u0027account\u0027, current_labels.get(\u0027account\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"563bc705_f886fd6d","line":615,"updated":"2026-07-28 16:31:57.000000000","message":"if changing get_current_labels() to get_base_labels() here, there is one unit test error in test_proxy_logging.py test_leftmost_and_rightmost_stats_and_logs_modified_subrequest, which would be because the api\u003dNone in overlay LabelsMap. So it seems these functions are quite similar and a bit confusing. IIUC the \"if acc\" etc. in get_current_labels() is for overlay purpose. wonder if it is possible to unify these functions?","commit_id":"704300278496f079abeee5c95245cf1549bcc527"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e8cf62121eb258a1e4fc61f7670e9d3634193b75","unresolved":true,"context_lines":[{"line_number":612,"context_line":"        else:"},{"line_number":613,"context_line":"            # expected in the right-most proxy_logging instance"},{"line_number":614,"context_line":"            current_labels \u003d self.get_current_labels(req, acc, cont, obj)"},{"line_number":615,"context_line":"            # if these base_labels are not already known then this is the best"},{"line_number":616,"context_line":"            # idea we have; set them in base_labels so that they are visible to"},{"line_number":617,"context_line":"            # the left-most proxy_logging instance"},{"line_number":618,"context_line":"            base_labels.setdefault(\u0027account\u0027, current_labels.get(\u0027account\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"79a82bb7_6b6726a0","line":615,"in_reply_to":"563bc705_f886fd6d","updated":"2026-07-29 13:37:06.000000000","message":"the next patch enables the two methods to be unified. I kept the next patch separate in case anyone felt it was too much \"custom data structure\".\n\nFor now, I like the methods being separate because it highlights that we are doing two different things in the left vs right proxy-logging.","commit_id":"704300278496f079abeee5c95245cf1549bcc527"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"da90ff73b9954624eb7e694351d06b83b90e2282","unresolved":false,"context_lines":[{"line_number":612,"context_line":"        else:"},{"line_number":613,"context_line":"            # expected in the right-most proxy_logging instance"},{"line_number":614,"context_line":"            current_labels \u003d self.get_current_labels(req, acc, cont, obj)"},{"line_number":615,"context_line":"            # if these base_labels are not already known then this is the best"},{"line_number":616,"context_line":"            # idea we have; set them in base_labels so that they are visible to"},{"line_number":617,"context_line":"            # the left-most proxy_logging instance"},{"line_number":618,"context_line":"            base_labels.setdefault(\u0027account\u0027, current_labels.get(\u0027account\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"fd8b4801_62b0059e","line":615,"in_reply_to":"79a82bb7_6b6726a0","updated":"2026-07-29 15:16:12.000000000","message":"noticed the related changes on the next patch, nice!","commit_id":"704300278496f079abeee5c95245cf1549bcc527"}],"swift/common/statsd_client.py":[{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"de3eb92e058ed7ca021f2d7ab7657fab374b668c","unresolved":true,"context_lines":[{"line_number":653,"context_line":"                                     sample_rate\u003dsample_rate)"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"class LabelsMap(dict):"},{"line_number":657,"context_line":"    \"\"\""},{"line_number":658,"context_line":"    Implements a custom map whose ``setdefault()`` method allows None values to"},{"line_number":659,"context_line":"    be replaced. This enables a label key to be set with a default unknown"}],"source_content_type":"text/x-python","patch_set":7,"id":"8a29537f_358aaf13","line":656,"updated":"2026-07-28 16:31:57.000000000","message":"nit: would it be better to add __slots__ here to preserve the dict behavior on not allowing attributes, which would add another instance of dict for attributes unnecessarily? although there are not many instances of labels map, so not an issue","commit_id":"704300278496f079abeee5c95245cf1549bcc527"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e8cf62121eb258a1e4fc61f7670e9d3634193b75","unresolved":true,"context_lines":[{"line_number":653,"context_line":"                                     sample_rate\u003dsample_rate)"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"class LabelsMap(dict):"},{"line_number":657,"context_line":"    \"\"\""},{"line_number":658,"context_line":"    Implements a custom map whose ``setdefault()`` method allows None values to"},{"line_number":659,"context_line":"    be replaced. This enables a label key to be set with a default unknown"}],"source_content_type":"text/x-python","patch_set":7,"id":"f08f090c_87763380","line":656,"in_reply_to":"8a29537f_358aaf13","updated":"2026-07-29 13:37:06.000000000","message":"good call, done","commit_id":"704300278496f079abeee5c95245cf1549bcc527"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"da90ff73b9954624eb7e694351d06b83b90e2282","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                                     sample_rate\u003dsample_rate)"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"class LabelsMap(dict):"},{"line_number":657,"context_line":"    \"\"\""},{"line_number":658,"context_line":"    Implements a custom map whose ``setdefault()`` method allows None values to"},{"line_number":659,"context_line":"    be replaced. This enables a label key to be set with a default unknown"}],"source_content_type":"text/x-python","patch_set":7,"id":"72847001_0da56501","line":656,"in_reply_to":"f08f090c_87763380","updated":"2026-07-29 15:16:12.000000000","message":"Acknowledged","commit_id":"704300278496f079abeee5c95245cf1549bcc527"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e33e028abe7129d8960e9988d3e1072931ed598","unresolved":true,"context_lines":[{"line_number":5514,"context_line":"        return self._dict.__repr__()"},{"line_number":5515,"context_line":""},{"line_number":5516,"context_line":"    @staticmethod"},{"line_number":5517,"context_line":"    def _default(k, v):"},{"line_number":5518,"context_line":"        return \u0027\u0027 if v is None else v"},{"line_number":5519,"context_line":""},{"line_number":5520,"context_line":"    def __getitem__(self, key):"}],"source_content_type":"text/x-python","patch_set":1,"id":"e6855f40_1725122b","line":5517,"range":{"start_line":5517,"start_character":17,"end_line":5517,"end_character":18},"updated":"2026-07-09 14:50:58.000000000","message":"``k`` is unused","commit_id":"b2c8feb218efce69f22c3223def600cb69b65e8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5056e27bb970d7709247e949d1204609ebcb350f","unresolved":false,"context_lines":[{"line_number":5514,"context_line":"        return self._dict.__repr__()"},{"line_number":5515,"context_line":""},{"line_number":5516,"context_line":"    @staticmethod"},{"line_number":5517,"context_line":"    def _default(k, v):"},{"line_number":5518,"context_line":"        return \u0027\u0027 if v is None else v"},{"line_number":5519,"context_line":""},{"line_number":5520,"context_line":"    def __getitem__(self, key):"}],"source_content_type":"text/x-python","patch_set":1,"id":"2f3a695d_d0a035c5","line":5517,"range":{"start_line":5517,"start_character":17,"end_line":5517,"end_character":18},"in_reply_to":"e6855f40_1725122b","updated":"2026-07-10 16:40:43.000000000","message":"Done","commit_id":"b2c8feb218efce69f22c3223def600cb69b65e8f"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"eff8fc3c2d90c5134d1383a4594ea96bbdc32391","unresolved":true,"context_lines":[{"line_number":5532,"context_line":"    def __len__(self):"},{"line_number":5533,"context_line":"        return len(self._dict)"},{"line_number":5534,"context_line":""},{"line_number":5535,"context_line":"    def setdefault(self, key, value\u003dNone):"},{"line_number":5536,"context_line":"        result \u003d self._dict.setdefault(key, value)"},{"line_number":5537,"context_line":"        dflt \u003d self._default(key, None)"},{"line_number":5538,"context_line":"        if result \u003d\u003d dflt and value !\u003d dflt:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd7da07e_0646d81c","line":5535,"updated":"2026-07-09 13:55:37.000000000","message":"WDYT about adding this:\n\n    value \u003d self._default(key, value)","commit_id":"b2c8feb218efce69f22c3223def600cb69b65e8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5056e27bb970d7709247e949d1204609ebcb350f","unresolved":false,"context_lines":[{"line_number":5532,"context_line":"    def __len__(self):"},{"line_number":5533,"context_line":"        return len(self._dict)"},{"line_number":5534,"context_line":""},{"line_number":5535,"context_line":"    def setdefault(self, key, value\u003dNone):"},{"line_number":5536,"context_line":"        result \u003d self._dict.setdefault(key, value)"},{"line_number":5537,"context_line":"        dflt \u003d self._default(key, None)"},{"line_number":5538,"context_line":"        if result \u003d\u003d dflt and value !\u003d dflt:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8e921e5d_404be175","line":5535,"in_reply_to":"5cbaa096_c4467f09","updated":"2026-07-10 16:40:43.000000000","message":"Done","commit_id":"b2c8feb218efce69f22c3223def600cb69b65e8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e33e028abe7129d8960e9988d3e1072931ed598","unresolved":true,"context_lines":[{"line_number":5532,"context_line":"    def __len__(self):"},{"line_number":5533,"context_line":"        return len(self._dict)"},{"line_number":5534,"context_line":""},{"line_number":5535,"context_line":"    def setdefault(self, key, value\u003dNone):"},{"line_number":5536,"context_line":"        result \u003d self._dict.setdefault(key, value)"},{"line_number":5537,"context_line":"        dflt \u003d self._default(key, None)"},{"line_number":5538,"context_line":"        if result \u003d\u003d dflt and value !\u003d dflt:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5cbaa096_c4467f09","line":5535,"in_reply_to":"cd7da07e_0646d81c","updated":"2026-07-09 14:50:58.000000000","message":"yes, that\u0027s a bug, thanks!","commit_id":"b2c8feb218efce69f22c3223def600cb69b65e8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2c254db6115cbe89303909123bef190878f56fc5","unresolved":true,"context_lines":[{"line_number":5501,"context_line":"        return super(CooperativeIterator, self)._get_next_item()"},{"line_number":5502,"context_line":""},{"line_number":5503,"context_line":""},{"line_number":5504,"context_line":"class LabelsMap(MutableMapping):"},{"line_number":5505,"context_line":"    \"\"\""},{"line_number":5506,"context_line":"    Implements a custom map with the following properties:"},{"line_number":5507,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2485b611_adb5a420","line":5504,"updated":"2026-07-09 14:54:42.000000000","message":"I\u0027m wondering if this belongs in statsd_client.py, not sure.","commit_id":"8c2e274d6f0652c99e08583a979d4cab19381c7b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2c254db6115cbe89303909123bef190878f56fc5","unresolved":true,"context_lines":[{"line_number":5505,"context_line":"    \"\"\""},{"line_number":5506,"context_line":"    Implements a custom map with the following properties:"},{"line_number":5507,"context_line":""},{"line_number":5508,"context_line":"    - None values are translated to the empty string when set."},{"line_number":5509,"context_line":""},{"line_number":5510,"context_line":"    - The setdefault method will allow existing empty string values to be"},{"line_number":5511,"context_line":"      overwritten by a new value. This means that a label key can be set"}],"source_content_type":"text/x-python","patch_set":2,"id":"6b10c99e_7699b995","line":5508,"updated":"2026-07-09 14:54:42.000000000","message":"This is to avoid ever passing ``labels \u003d {\u0027key\u0027: None}`` to a stasd client method, which would result in a labeled metric with label such as ``key\u003dNone`` which cannot be distinguished from ``labels \u003d {\u0027key\u0027: \"None\" }`` (note quotes)\n\nAn alternative would be to have the statsd client translate all None label values to the empty string.","commit_id":"8c2e274d6f0652c99e08583a979d4cab19381c7b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b6d3ed9c9d779f89627b8bb3e96c52c8bfb8286d","unresolved":true,"context_lines":[{"line_number":5505,"context_line":"    \"\"\""},{"line_number":5506,"context_line":"    Implements a custom map with the following properties:"},{"line_number":5507,"context_line":""},{"line_number":5508,"context_line":"    - None values are translated to the empty string when set."},{"line_number":5509,"context_line":""},{"line_number":5510,"context_line":"    - The setdefault method will allow existing empty string values to be"},{"line_number":5511,"context_line":"      overwritten by a new value. This means that a label key can be set"}],"source_content_type":"text/x-python","patch_set":2,"id":"23a2cf6b_5def0807","line":5508,"in_reply_to":"0a7d0751_eca0c362","updated":"2026-07-14 12:20:17.000000000","message":"I think Claude has maybe ignored it\u0027s own advice: the ``update()`` method bypasses ``__setitem__``\n\nSo it\u0027s not as simple as it seems.\n\nIIRC this is what drove me towards a MutableMap:\n\n\u003e \"a real CPython hazard: dict.__init__ and dict.update in CPython\u0027s C\n\u003e layer bypass the subclass\u0027s __setitem__ when the class itself is \n\u003e dict\u0027s C type path, so initial values silently won\u0027t go through the \n\u003e None→\u0027\u0027 normalization unless you are careful. MutableMapping is safe \n\u003e because every operation is routed through the five abstract methods.\"\n\nBut I had another go at dict subclassing, see next patchset.","commit_id":"8c2e274d6f0652c99e08583a979d4cab19381c7b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"405dcbadfb8f83e9fa6ccb858f4faa7f60c1a0ed","unresolved":true,"context_lines":[{"line_number":5505,"context_line":"    \"\"\""},{"line_number":5506,"context_line":"    Implements a custom map with the following properties:"},{"line_number":5507,"context_line":""},{"line_number":5508,"context_line":"    - None values are translated to the empty string when set."},{"line_number":5509,"context_line":""},{"line_number":5510,"context_line":"    - The setdefault method will allow existing empty string values to be"},{"line_number":5511,"context_line":"      overwritten by a new value. This means that a label key can be set"}],"source_content_type":"text/x-python","patch_set":2,"id":"0a7d0751_eca0c362","line":5508,"in_reply_to":"6b10c99e_7699b995","updated":"2026-07-14 00:09:51.000000000","message":"I remember @yanxiao@nvidia.com asked the question related to ``MutableMapping``, sorry I forgot totally what did @alistairncoles@gmail.com reply. Now I am also thinking about it as well. Claude thinks there is a simpler version: a plain dict subclass with only ``__setitem__`` and ``setdefault`` overridden. It seems it could work?\n\n```\nThe MutableMapping base is doing a lot of work here (it provides update, get, keys, values, \nitems, __contains__, __eq__, pop, popitem) but the reason it exists rather than subclassing\ndict is a real CPython hazard: dict.__init__ and dict.update in CPython\u0027s C layer bypass the\nsubclass\u0027s __setitem__ when the class itself is dict\u0027s C type path, so initial values silently\nwon\u0027t go through the None→\u0027\u0027 normalization unless you are careful. MutableMapping is safe \nbecause every operation is routed through the five abstract methods.\n\nThat said, a dict subclass works fine if __init__ drives everything through self.update()\nrather than super().__init__(data):\n\nclass LabelsMap(dict):\n    def __init__(self, labels\u003dNone, **kwargs):\n        super().__init__()        # empty — no data bypasses __setitem__\n        if labels:\n            self.update(labels)   # goes through overridden __setitem__\n        if kwargs:\n            self.update(kwargs)\n\n    def __setitem__(self, key, value):\n        super().__setitem__(key, \u0027\u0027 if value is None else value)\n\n    def setdefault(self, key, value\u003dNone):\n        value \u003d \u0027\u0027 if value is None else value\n        if self.get(key, \u0027\u0027) \u003d\u003d \u0027\u0027:\n            self[key] \u003d value\n        return self[key]\n        \nThis is simpler (no __slots__, no private _dict, no __iter__/__len__/__delitem__ boilerplate,\ndict is faster than MutableMapping\u0027s pure-Python fallbacks). In CPython dict.update() does call\nan overridden __setitem__ — only dict.__init__ passed data is the hazard.\n```","commit_id":"8c2e274d6f0652c99e08583a979d4cab19381c7b"}],"test/unit/common/middleware/test_proxy_logging.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"405dcbadfb8f83e9fa6ccb858f4faa7f60c1a0ed","unresolved":true,"context_lines":[{"line_number":696,"context_line":"        app.statsd \u003d self.statsd"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"        def do_test(bad_path):"},{"line_number":699,"context_line":"            exp_labels \u003d {\u0027account\u0027: \u0027\u0027,"},{"line_number":700,"context_line":"                          \u0027resource\u0027: \u0027\u0027,"},{"line_number":701,"context_line":"                          \u0027method\u0027: \u0027GET\u0027,"},{"line_number":702,"context_line":"                          \u0027api\u0027: \u0027swift\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"a7da409b_8e733444","line":699,"updated":"2026-07-14 00:09:51.000000000","message":"another review comment from Claude which makes sense to me.\n\n```\nThe motivating scenario lacks direct metrics-level coverage. The commit message\nmotivates with \"an S3 request that failed to reach the rightmost proxy logging\".\nThe nearest tests are test_swift_base_labels_end_to_end_account_s3 (asserts the\nleftmost snapshot has account: \u0027\u0027/resource: \u0027\u0027 — but that request does reach\nthe rightmost) and test_log_request_stat_type_bad_GET (leftmost-only, but\nnative Swift API). No test emits labeled metrics for an S3 request rejected\nbefore the rightmost instance (e.g. an s3api auth failure in the\n_make_logged_pipeline fixture) asserting the emitted labels include\naccount: \u0027\u0027, resource: \u0027\u0027, api: \u0027S3\u0027. Can one be added to pin the exact bug\nscenario?\n```","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b6d3ed9c9d779f89627b8bb3e96c52c8bfb8286d","unresolved":false,"context_lines":[{"line_number":696,"context_line":"        app.statsd \u003d self.statsd"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"        def do_test(bad_path):"},{"line_number":699,"context_line":"            exp_labels \u003d {\u0027account\u0027: \u0027\u0027,"},{"line_number":700,"context_line":"                          \u0027resource\u0027: \u0027\u0027,"},{"line_number":701,"context_line":"                          \u0027method\u0027: \u0027GET\u0027,"},{"line_number":702,"context_line":"                          \u0027api\u0027: \u0027swift\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"b0f7ea5f_9079ee29","line":699,"in_reply_to":"a7da409b_8e733444","updated":"2026-07-14 12:20:17.000000000","message":"Done","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"405dcbadfb8f83e9fa6ccb858f4faa7f60c1a0ed","unresolved":true,"context_lines":[{"line_number":1387,"context_line":"        ], app)"},{"line_number":1388,"context_line":"        self.assertLabeledUpdateStats(["},{"line_number":1389,"context_line":"            (\u0027swift_proxy_server_request_body_bytes\u0027, 0, {"},{"line_number":1390,"context_line":"                \u0027account\u0027: \u0027\u0027,"},{"line_number":1391,"context_line":"                \u0027resource\u0027: \u0027SOS\u0027,"},{"line_number":1392,"context_line":"                \u0027api\u0027: \u0027swift\u0027,"},{"line_number":1393,"context_line":"                \u0027method\u0027: \u0027GET\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ca540e45_7f5b20fe","line":1390,"updated":"2026-07-14 00:09:51.000000000","message":"11 tests in this test file failed if I revert changes in ``swift/common/middleware/proxy_logging.py`` only.","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b6d3ed9c9d779f89627b8bb3e96c52c8bfb8286d","unresolved":false,"context_lines":[{"line_number":1387,"context_line":"        ], app)"},{"line_number":1388,"context_line":"        self.assertLabeledUpdateStats(["},{"line_number":1389,"context_line":"            (\u0027swift_proxy_server_request_body_bytes\u0027, 0, {"},{"line_number":1390,"context_line":"                \u0027account\u0027: \u0027\u0027,"},{"line_number":1391,"context_line":"                \u0027resource\u0027: \u0027SOS\u0027,"},{"line_number":1392,"context_line":"                \u0027api\u0027: \u0027swift\u0027,"},{"line_number":1393,"context_line":"                \u0027method\u0027: \u0027GET\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"b2109ffb_2329ceb9","line":1390,"in_reply_to":"ca540e45_7f5b20fe","updated":"2026-07-14 12:20:17.000000000","message":"Acknowledged","commit_id":"839b9875cce647e7904ed7b7dc71991efbf71c48"}]}