)]}'
{"/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"}],"/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"}],"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"}]}