)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Yan Xiao \u003cyanxiao@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2025-03-21 09:54:09 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Object-server: add labeled timing metrics for object REPLICATE request"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I155572bc5f4d955c918c951d6d2af9dbbae0136d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"847183dd_c46b358c","line":7,"range":{"start_line":7,"start_character":53,"end_line":7,"end_character":62},"updated":"2025-04-02 16:07:18.000000000","message":"REPLICATE and GET ... but now the patch has broadened to GET, why not other methods?\n\nUpdate: My understanding is that @Yan will be following up with labeled metrics for other methods.","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yan Xiao \u003cyanxiao@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2025-03-21 09:54:09 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Object-server: add labeled timing metrics for object REPLICATE request"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I155572bc5f4d955c918c951d6d2af9dbbae0136d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"1b643958_6a50354a","line":7,"range":{"start_line":7,"start_character":53,"end_line":7,"end_character":62},"in_reply_to":"847183dd_c46b358c","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c4acd41f_b1ebbb9e","updated":"2025-03-18 14:31:55.000000000","message":"This looks pretty good! I\u0027d suggest moving the ``labels[\u0027status\u0027]\u003d`` assignment into the decorator.\n\nThere should be a test for the object server: teh decorator *requires* that the controller have a statsd_client so we should at least validate that is the case.","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9aa1e256315a0455846023985cb45bc17b87b677","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c7837604_b8ee9975","updated":"2025-03-19 18:45:54.000000000","message":"-1 for my concern about the arg order and the labels arg on the decorator itself","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"391b2488_a1cfa14c","updated":"2025-03-19 17:38:02.000000000","message":"I\u0027m not sure anything has to change here to be correct and complete - but some of the new (as-of-yet-unused?) utility/helper behaviors seems weird:\n\n945019: moar tests | https://review.opendev.org/c/openstack/swift/+/945019\n\n* probably should force method to be the req.method (or in a follow-up?)\n* maybe always hand the wrapped function an empty dict and THEN add our \"required\" labels\n* less likely extract policy labeling assuming we\u0027ll want to DRY out instrumenting of the other objec-server METHODs (but probably KISS and save the DRYing for when follow-on)","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c847e74b_f16ac475","updated":"2025-04-02 16:07:18.000000000","message":"my comments are nits apart from improving the unit test coverage for the legacy stats","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"de569bd025273eb4c82bd0ffcbbbdc9e485d9023","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3909e6d8_b8333d3a","updated":"2025-04-04 17:36:54.000000000","message":"Ran this in my vsaio (remember to set statsd options in object-server conf!) and saw the expected labeled metrics using https://github.com/tipabu/udp_listener\n\n```\nvagrant@vagrant:~/swift$ head -n 12 /etc/swift/object-server/server.conf-template\n[DEFAULT]\ndisable_fallocate \u003d true\nworkers \u003d 1\nlog_facility \u003d LOG_LOCAL2\n# NOTE: servers_per_port necessary for replicator/reconstructor so must go here\nservers_per_port \u003d 0\nlog_statsd_host \u003d localhost\nlog_statsd_port \u003d 8125\nstatsd_label_mode \u003d dogstatsd\n\n[pipeline:main]\npipeline \u003d recon object-server\n\n\nvagrant@vagrant:~/udp_listener$ ./udp_listener.py\n\nswift_object_request_timing:0.752|ms|@0.1|#method:REPLICATE,policy:0,skip_rehash:False,status:200\n\n\nswift_object_request_timing:0.4079|ms|#account:AUTH_test,container:test,method:GET,policy:0,status:200\n\n```","commit_id":"81cf98c51ce5043c072873f01386039d6dfd72ff"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b47cb6128794c060ae51e171d2d61f73f8d796d1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"8f1afb27_cd9ee4ea","updated":"2025-04-07 16:23:11.000000000","message":"recheck","commit_id":"81cf98c51ce5043c072873f01386039d6dfd72ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b4910a85dd95406f2b449ac9e3b61371dfccdaef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"cc23f49e_b19ef385","updated":"2025-04-09 15:43:47.000000000","message":"I think we should solicit some feedback from SRE on the new metric name\n\nfor legacy proxy-logging timing metrics we have:\n\n    ss_proxy_server_timing\n\nfor new proxy-logging timing metrics we already collect in prod:\n\n    swift_proxy_request_timing\n\n917711: Add labeled metrics to proxy-logging | https://review.opendev.org/c/openstack/swift/+/917711\n\nfor legacy object-server timing metrics we have:\n\n    ss_object_server_timing\n\nThis change adds a new object-server timing metric as:\n\n    swift_object_request_timing\n\n... which matches the new proxy format, but I worry will cause ambiguity with the implied `swift_object_server_request_timing` as opposed to `swift_object_updater_request_timing` which can\u0027t be \"just\" `swift_updater_request_timing` because of the overlap with `swift_container_updater_request_timing`.  The proxy-server is unique in that there are not proxy-XXX services to confuse it with, but obviously someone decided the the proxy_server_timing metric name should be adopted for consistency.\n\nAlso I don\u0027t understand when servers or daemons (clients) should emit a \"request\" as opposed to \"response\" timing metric?  What about when a server *makes* a request like a `swift_object_server_container_update_timing`?\n\nToday I think it might be better to name this new metric:\n\n`swift_object_server_timing`\n\n... but maybe on a different day I would have said \"timing for *what*\"?!??  OTOH `server_timing` should be pretty obvious for a RESTful server.  A replicator might have `swift_object_server_replicate_timing`.  Since neither a req nor a resp have both a status and a method maybe given our labels `request` vs `response` is a false dichotomy.  Or we maybe just agree to always use the term \"request\" when either seems like it might do.\n\nAside from wanting to be sure about the name because we\u0027re setting a precedent I think the unrelated debug log message churn in `obj.test_server` adds sufficiently to the degree of \"makes things slightly worse\" that we should pause and consider if ANY possible alternative might exist before we merge and compromise with \"well, I guess it\u0027s un-avoidable complexity cost if we want this new feature\" ... which perhaps there is?\n\n946762: tests: add more tests for object-server labeled metrics | https://review.opendev.org/c/openstack/swift/+/946762","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b97bb04002b92f8903f2735bc8c25cdd71f67e13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"d83e9699_c8be12f7","updated":"2025-04-09 12:30:49.000000000","message":"This looks good. We can add labeled metrics to other request methods in subsequent patches.\n\nI think we can improve the test coverage https://review.opendev.org/c/openstack/swift/+/946762\n\nI don\u0027t mind if that ^^ merges separately or is squashed in. Happy if someone else wants to +2A this patch. I\u0027ve been quite involved so I\u0027ll wait for another opinion.","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":true,"context_lines":[{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"},{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"4345d22f_9d8e927a","line":1000,"range":{"start_line":1000,"start_character":12,"end_line":1000,"end_character":33},"updated":"2025-03-18 14:31:55.000000000","message":"that\u0027s interesting - are you thinking that in the future there may be nested decorators passing a labels dict to each other?","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"},{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f0f19448_336fa9c2","line":1000,"range":{"start_line":1000,"start_character":12,"end_line":1000,"end_character":33},"in_reply_to":"2aee7f95_2a6f6876","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"},{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"803bbefa_bf1e1109","line":1000,"range":{"start_line":1000,"start_character":12,"end_line":1000,"end_character":33},"in_reply_to":"4345d22f_9d8e927a","updated":"2025-03-19 15:26:36.000000000","message":"Actually was thinking that the object controller could pass labels such as account to the decorator or request function","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"},{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"43b7268a_dd2e1395","line":1000,"range":{"start_line":1000,"start_character":12,"end_line":1000,"end_character":33},"in_reply_to":"4345d22f_9d8e927a","updated":"2025-03-19 17:38:02.000000000","message":"I\u0027m not sure this method should have a labels kwarg - I don\u0027t understand it.","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9aa1e256315a0455846023985cb45bc17b87b677","unresolved":true,"context_lines":[{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"},{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"2aee7f95_2a6f6876","line":1000,"range":{"start_line":1000,"start_character":12,"end_line":1000,"end_character":33},"in_reply_to":"43b7268a_dd2e1395","updated":"2025-03-19 18:45:54.000000000","message":"I agree, I don\u0027t perceive a use case for passing labels to the decorator\u0027s wrapper.\n\nIn fact, it probably MUST not be there if this decorator is to be used with any func that has more args than ``(self, req)``\n\n```\n@labeled_timing_stats(metric\u003d\u0027foo\u0027)\ndef myfunc(req, other):\n   ...\n\nmyfunc(req, other)  # other will be treated as the labels dict in _timing_stats so we\u0027ll attempt other[\u0027method\u0027] \u003d method 😞\n```","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":true,"context_lines":[{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"            ctrl.statsd.timing_since(metric, start_time, labels\u003dlabels,"},{"line_number":1006,"context_line":"                                     **dec_kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3c458cf0_7a317086","line":1003,"updated":"2025-03-18 14:31:55.000000000","message":"we should include the resp status code in the labels - note how timing_stats varies the metric is the response is an error","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"            ctrl.statsd.timing_since(metric, start_time, labels\u003dlabels,"},{"line_number":1006,"context_line":"                                     **dec_kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"18f0fe26_bfdcb6e5","line":1003,"in_reply_to":"3c458cf0_7a317086","updated":"2025-03-19 17:38:02.000000000","message":"\u003e we should include the resp status code in the labels\nagreed, that seems done\n\n\u003e varies the metric is the response is an error\ns/varies the metric *if* the response is an error - sure but we don\u0027t need that just labels.","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, labels\u003dlabels, *args, **kwargs)"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"            ctrl.statsd.timing_since(metric, start_time, labels\u003dlabels,"},{"line_number":1006,"context_line":"                                     **dec_kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"d94abeaa_6158c10f","line":1003,"in_reply_to":"3c458cf0_7a317086","updated":"2025-03-19 15:26:36.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":993,"context_line":"    code."},{"line_number":994,"context_line":"    \"\"\""},{"line_number":995,"context_line":"    def decorating_func(func):"},{"line_number":996,"context_line":"        method \u003d func.__name__"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"6c1c6d30_5321bada","line":996,"updated":"2025-03-19 17:38:02.000000000","message":"i\u0027m not sure I buy it - why is this better than req.method?","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9aa1e256315a0455846023985cb45bc17b87b677","unresolved":true,"context_lines":[{"line_number":993,"context_line":"    code."},{"line_number":994,"context_line":"    \"\"\""},{"line_number":995,"context_line":"    def decorating_func(func):"},{"line_number":996,"context_line":"        method \u003d func.__name__"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"88106b3a_707ba40f","line":996,"in_reply_to":"6c1c6d30_5321bada","updated":"2025-03-19 18:45:54.000000000","message":"I don\u0027t disagree, but it is consistent with timing_stats @ line 1020, and we have cases where requests are passed on to other functions wrapped with timing_stats where we\u0027ve wanted the function name rather than the req method to differentiate the metric name https://github.com/openstack/swift/blob/128124cdd8ca09136d4988affd1bb8c5c1361fc1/swift/container/server.py#L513-L514\n\nHowever, arguably we wouldn\u0027t need to use labeled_timing_stats on PUT_object or PUT_shard, because we\u0027d just add an object vs shard label in the PUT, so it\u0027s moot.\n\nIf we do set ``labels[\u0027method\u0027] \u003d req.method`` *before* calling the func then we can always override it in the func if we really need to.\n\nSo I\u0027m cool with ``labels[\u0027method\u0027] \u003d req.method`` but note that we cannot make ``timing_stats`` do the same because of the container server anomaly linked above.","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":993,"context_line":"    code."},{"line_number":994,"context_line":"    \"\"\""},{"line_number":995,"context_line":"    def decorating_func(func):"},{"line_number":996,"context_line":"        method \u003d func.__name__"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"4c73a187_51447c7e","line":996,"in_reply_to":"88106b3a_707ba40f","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":996,"context_line":"        method \u003d func.__name__"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"},{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"4b9e191e_5f816af9","line":999,"updated":"2025-03-19 17:38:02.000000000","message":"but the decorator itself declares the signature as having the kwarg labels.\n\nI still don\u0027t know why the tests pass.","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":996,"context_line":"        method \u003d func.__name__"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        @functools.wraps(func)"},{"line_number":999,"context_line":"        def _timing_stats(ctrl, req, labels\u003dNone, *args, **kwargs):"},{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"304c04f6_4f9a66dc","line":999,"in_reply_to":"4b9e191e_5f816af9","updated":"2025-03-20 15:32:43.000000000","message":"Updated args per suggestions","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, timing_stats_labels\u003dlabels, *args, **kwargs)"},{"line_number":1004,"context_line":"            labels[\u0027status\u0027] \u003d resp.status_int"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"            ctrl.statsd.timing_since(metric, start_time, labels\u003dlabels,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9dc8ac2b_5095ff29","line":1003,"updated":"2025-03-19 17:38:02.000000000","message":"so the wrapped func is always called with the kwarg `timing_stats_labels`\n\nthe wrapped method must accept this signature - that\u0027s good.","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9aa1e256315a0455846023985cb45bc17b87b677","unresolved":true,"context_lines":[{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, timing_stats_labels\u003dlabels, *args, **kwargs)"},{"line_number":1004,"context_line":"            labels[\u0027status\u0027] \u003d resp.status_int"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"            ctrl.statsd.timing_since(metric, start_time, labels\u003dlabels,"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab04476_406ea666","line":1003,"in_reply_to":"9dc8ac2b_5095ff29","updated":"2025-03-19 18:45:54.000000000","message":"I\u0027m not sure about the arg order, whether it shouldn\u0027t be:\n\n``func(ctrl, req, *args, timing_stats_labels\u003dlabels, **kwargs)``\n\nfor the cases when the func already has other positional args after ``req`` - it would be nice to preserve the existing arg order and *append* the ``labels`` arg to the *end* of the existing positional args  (and it\u0027s kind of what I am used to decorators doing, admittedly based on limited experience i.e. ``@mock.patch``)\n\nFor example, https://paste.openstack.org/show/bhyMQEl3qXH28ryaszfk/\n\n(That may not be the best example of where we\u0027d apply labeled_timing_stats but illustrates how the arg order is significant in general).","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"            labels \u003d labels or {}"},{"line_number":1001,"context_line":"            labels[\u0027method\u0027] \u003d method"},{"line_number":1002,"context_line":"            start_time \u003d time.time()"},{"line_number":1003,"context_line":"            resp \u003d func(ctrl, req, timing_stats_labels\u003dlabels, *args, **kwargs)"},{"line_number":1004,"context_line":"            labels[\u0027status\u0027] \u003d resp.status_int"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"            ctrl.statsd.timing_since(metric, start_time, labels\u003dlabels,"}],"source_content_type":"text/x-python","patch_set":3,"id":"cbab76b2_95f2015c","line":1003,"in_reply_to":"dab04476_406ea666","updated":"2025-03-20 15:32:43.000000000","message":"Good point on arg order!","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":988,"context_line":""},{"line_number":989,"context_line":"def labeled_timing_stats(metric, **dec_kwargs):"},{"line_number":990,"context_line":"    \"\"\""},{"line_number":991,"context_line":"    Returns a decorator that logs labeled metrics timing events or errors"},{"line_number":992,"context_line":"    for public methods in swift\u0027s wsgi server controllers, based on response"},{"line_number":993,"context_line":"    code."},{"line_number":994,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1afadeed_e695325a","line":991,"range":{"start_line":991,"start_character":29,"end_line":991,"end_character":33},"updated":"2025-04-02 16:07:18.000000000","message":"nit: s/logs/emits/","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":988,"context_line":""},{"line_number":989,"context_line":"def labeled_timing_stats(metric, **dec_kwargs):"},{"line_number":990,"context_line":"    \"\"\""},{"line_number":991,"context_line":"    Returns a decorator that logs labeled metrics timing events or errors"},{"line_number":992,"context_line":"    for public methods in swift\u0027s wsgi server controllers, based on response"},{"line_number":993,"context_line":"    code."},{"line_number":994,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1f68d9a9_f8bc3243","line":991,"range":{"start_line":991,"start_character":29,"end_line":991,"end_character":33},"in_reply_to":"1afadeed_e695325a","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":993,"context_line":"    code."},{"line_number":994,"context_line":""},{"line_number":995,"context_line":"    The controller methods are not allowed to override the following labels:"},{"line_number":996,"context_line":"    \u0027method\u0027, \u0027status\u0027."},{"line_number":997,"context_line":"    \"\"\""},{"line_number":998,"context_line":"    def decorating_func(func):"},{"line_number":999,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"faf7d5fb_e8439462","line":996,"updated":"2025-04-02 16:07:18.000000000","message":"nit: if you want to be *really* sure of that then stash req.method before passing req to the controller, because a devious controller could change req.method ;-)","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":993,"context_line":"    code."},{"line_number":994,"context_line":""},{"line_number":995,"context_line":"    The controller methods are not allowed to override the following labels:"},{"line_number":996,"context_line":"    \u0027method\u0027, \u0027status\u0027."},{"line_number":997,"context_line":"    \"\"\""},{"line_number":998,"context_line":"    def decorating_func(func):"},{"line_number":999,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"e8eddbef_2ad9cd3c","line":996,"in_reply_to":"faf7d5fb_e8439462","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b4910a85dd95406f2b449ac9e3b61371dfccdaef","unresolved":true,"context_lines":[{"line_number":1035,"context_line":"            # Examples include 507 for an unmounted drive or 500 for an"},{"line_number":1036,"context_line":"            # unhandled exception."},{"line_number":1037,"context_line":"            if not is_server_error(resp.status_int):"},{"line_number":1038,"context_line":"                ctrl.logger.timing_since(method + \u0027.timing\u0027,"},{"line_number":1039,"context_line":"                                         start_time, **dec_kwargs)"},{"line_number":1040,"context_line":"            else:"},{"line_number":1041,"context_line":"                ctrl.logger.timing_since(method + \u0027.errors.timing\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba9236c5_c29d4d90","line":1038,"updated":"2025-04-09 15:43:47.000000000","message":"how did this metric get `object_server` in the name somewhere?","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"714ad90bc8ea1177282400ccca29d99dcc0fd9e5","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            # Examples include 507 for an unmounted drive or 500 for an"},{"line_number":1036,"context_line":"            # unhandled exception."},{"line_number":1037,"context_line":"            if not is_server_error(resp.status_int):"},{"line_number":1038,"context_line":"                ctrl.logger.timing_since(method + \u0027.timing\u0027,"},{"line_number":1039,"context_line":"                                         start_time, **dec_kwargs)"},{"line_number":1040,"context_line":"            else:"},{"line_number":1041,"context_line":"                ctrl.logger.timing_since(method + \u0027.errors.timing\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"39f93381_aa489c02","line":1038,"in_reply_to":"ba9236c5_c29d4d90","updated":"2025-04-17 19:22:05.000000000","message":"Actually from the name variable in conf","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"}],"swift/obj/server.py":[{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"9c6997a8157df29097218fdd3b27b68f3c3c1f2b","unresolved":true,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"LABELED_METRIC_NAME \u003d \u0027swift_object_request_timing\u0027"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def iter_mime_headers_and_bodies(wsgi_input, mime_boundary, read_chunk_size):"},{"line_number":72,"context_line":"    mime_documents_iter \u003d iter_multipart_mime_documents("},{"line_number":73,"context_line":"        wsgi_input, mime_boundary, read_chunk_size)"}],"source_content_type":"text/x-python","patch_set":2,"id":"d3759ba2_da4d8ff2","line":70,"updated":"2025-03-18 14:26:26.000000000","message":"extra line prolly?","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"LABELED_METRIC_NAME \u003d \u0027swift_object_request_timing\u0027"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def iter_mime_headers_and_bodies(wsgi_input, mime_boundary, read_chunk_size):"},{"line_number":72,"context_line":"    mime_documents_iter \u003d iter_multipart_mime_documents("},{"line_number":73,"context_line":"        wsgi_input, mime_boundary, read_chunk_size)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7045b6a7_b9e2edf9","line":70,"in_reply_to":"d3759ba2_da4d8ff2","updated":"2025-03-19 15:26:36.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"LABELED_METRIC_NAME \u003d \u0027swift_object_request_timing\u0027"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def iter_mime_headers_and_bodies(wsgi_input, mime_boundary, read_chunk_size):"},{"line_number":72,"context_line":"    mime_documents_iter \u003d iter_multipart_mime_documents("},{"line_number":73,"context_line":"        wsgi_input, mime_boundary, read_chunk_size)"}],"source_content_type":"text/x-python","patch_set":2,"id":"91a155d0_31e4c97d","line":70,"in_reply_to":"d3759ba2_da4d8ff2","updated":"2025-03-19 17:38:02.000000000","message":"i think two new-lines after a constant is ok, zuul seems to think it lints ok\n\nI think this is a sufficiently generic metric name for adding timing/labels to the other object_server_request\u0027s method\u0027s methods.","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":true,"context_lines":[{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"    @public"},{"line_number":1320,"context_line":"    @replication"},{"line_number":1321,"context_line":"    @labeled_timing_stats(metric\u003dLABELED_METRIC_NAME, sample_rate\u003d0.1)"},{"line_number":1322,"context_line":"    def REPLICATE(self, request, labels):"},{"line_number":1323,"context_line":"        \"\"\""},{"line_number":1324,"context_line":"        Handle REPLICATE requests for the Swift Object Server.  This is used"}],"source_content_type":"text/x-python","patch_set":2,"id":"6aca66f9_280b05d6","line":1321,"updated":"2025-03-18 14:31:55.000000000","message":"hmmm, so we lose the legacy stat. Looks like it would work to nest the decorators (i.e. retain timing_stats at line 1320)","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"    @public"},{"line_number":1320,"context_line":"    @replication"},{"line_number":1321,"context_line":"    @labeled_timing_stats(metric\u003dLABELED_METRIC_NAME, sample_rate\u003d0.1)"},{"line_number":1322,"context_line":"    def REPLICATE(self, request, labels):"},{"line_number":1323,"context_line":"        \"\"\""},{"line_number":1324,"context_line":"        Handle REPLICATE requests for the Swift Object Server.  This is used"}],"source_content_type":"text/x-python","patch_set":2,"id":"f67d575c_a29e8a16","line":1321,"in_reply_to":"6aca66f9_280b05d6","updated":"2025-03-19 15:26:36.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":false,"context_lines":[{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"    @public"},{"line_number":1320,"context_line":"    @replication"},{"line_number":1321,"context_line":"    @labeled_timing_stats(metric\u003dLABELED_METRIC_NAME, sample_rate\u003d0.1)"},{"line_number":1322,"context_line":"    def REPLICATE(self, request, labels):"},{"line_number":1323,"context_line":"        \"\"\""},{"line_number":1324,"context_line":"        Handle REPLICATE requests for the Swift Object Server.  This is used"}],"source_content_type":"text/x-python","patch_set":2,"id":"5df0b8ab_01fb510e","line":1321,"in_reply_to":"6aca66f9_280b05d6","updated":"2025-03-19 17:38:02.000000000","message":"Done","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":true,"context_lines":[{"line_number":1319,"context_line":"    @public"},{"line_number":1320,"context_line":"    @replication"},{"line_number":1321,"context_line":"    @labeled_timing_stats(metric\u003dLABELED_METRIC_NAME, sample_rate\u003d0.1)"},{"line_number":1322,"context_line":"    def REPLICATE(self, request, labels):"},{"line_number":1323,"context_line":"        \"\"\""},{"line_number":1324,"context_line":"        Handle REPLICATE requests for the Swift Object Server.  This is used"},{"line_number":1325,"context_line":"        by the object replicator to get hashes for directories."}],"source_content_type":"text/x-python","patch_set":2,"id":"9a19a3b3_fa8580c0","line":1322,"range":{"start_line":1322,"start_character":33,"end_line":1322,"end_character":39},"updated":"2025-03-18 14:31:55.000000000","message":"nit: I\u0027d suggest naming this timing_stats_labels as a hint that the arg is coming from labeled_timing_stats","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":1319,"context_line":"    @public"},{"line_number":1320,"context_line":"    @replication"},{"line_number":1321,"context_line":"    @labeled_timing_stats(metric\u003dLABELED_METRIC_NAME, sample_rate\u003d0.1)"},{"line_number":1322,"context_line":"    def REPLICATE(self, request, labels):"},{"line_number":1323,"context_line":"        \"\"\""},{"line_number":1324,"context_line":"        Handle REPLICATE requests for the Swift Object Server.  This is used"},{"line_number":1325,"context_line":"        by the object replicator to get hashes for directories."}],"source_content_type":"text/x-python","patch_set":2,"id":"ce85fea0_598b15ee","line":1322,"range":{"start_line":1322,"start_character":33,"end_line":1322,"end_character":39},"in_reply_to":"9a19a3b3_fa8580c0","updated":"2025-03-19 15:26:36.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":false,"context_lines":[{"line_number":1319,"context_line":"    @public"},{"line_number":1320,"context_line":"    @replication"},{"line_number":1321,"context_line":"    @labeled_timing_stats(metric\u003dLABELED_METRIC_NAME, sample_rate\u003d0.1)"},{"line_number":1322,"context_line":"    def REPLICATE(self, request, labels):"},{"line_number":1323,"context_line":"        \"\"\""},{"line_number":1324,"context_line":"        Handle REPLICATE requests for the Swift Object Server.  This is used"},{"line_number":1325,"context_line":"        by the object replicator to get hashes for directories."}],"source_content_type":"text/x-python","patch_set":2,"id":"3e352e04_915d5b88","line":1322,"range":{"start_line":1322,"start_character":33,"end_line":1322,"end_character":39},"in_reply_to":"9a19a3b3_fa8580c0","updated":"2025-03-19 17:38:02.000000000","message":"Done","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":true,"context_lines":[{"line_number":1334,"context_line":""},{"line_number":1335,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1336,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1337,"context_line":"        labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"        try:"},{"line_number":1340,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":2,"id":"eddf6bef_078d71e5","line":1337,"range":{"start_line":1337,"start_character":32,"end_line":1337,"end_character":35},"updated":"2025-03-18 14:31:55.000000000","message":"I forget - is the str necessary or will statsd_client stringify all the label values for us?","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":1334,"context_line":""},{"line_number":1335,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1336,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1337,"context_line":"        labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"        try:"},{"line_number":1340,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":2,"id":"eaca412e_2828ead7","line":1337,"range":{"start_line":1337,"start_character":32,"end_line":1337,"end_character":35},"in_reply_to":"4b71992b_e0531d70","updated":"2025-03-20 15:32:43.000000000","message":"Good point!","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":1334,"context_line":""},{"line_number":1335,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1336,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1337,"context_line":"        labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"        try:"},{"line_number":1340,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":2,"id":"5bba01d8_87f11c12","line":1337,"range":{"start_line":1337,"start_character":32,"end_line":1337,"end_character":35},"in_reply_to":"eddf6bef_078d71e5","updated":"2025-03-19 15:26:36.000000000","message":"Should be necessary","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":1334,"context_line":""},{"line_number":1335,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1336,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1337,"context_line":"        labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"        try:"},{"line_number":1340,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":2,"id":"4b71992b_e0531d70","line":1337,"range":{"start_line":1337,"start_character":32,"end_line":1337,"end_character":35},"in_reply_to":"eddf6bef_078d71e5","updated":"2025-03-19 17:38:02.000000000","message":"this is a good question, I think it would be a fairly brittle interface in a dynamic language if it threw a type-error on non-strings, but I suppose forcing the caller to be explicit could reduce bugs if you pass complex objects w/o thinking - but we don\u0027t do that:\n\nhttps://github.com/NVIDIA/swift/blob/ss-release-2.34.0.0.3/swift/common/statsd_client.py#L79\n\nif `\u0027%s:%s\u0027 % (k, v)` does what you want - YOU\u0027RE GOOD!","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":true,"context_lines":[{"line_number":1346,"context_line":"            # force pickle protocol for compatibility with py2 nodes"},{"line_number":1347,"context_line":"            resp \u003d Response(body\u003dpickle.dumps(hashes, protocol\u003d2))"},{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":"        labels[\u0027status\u0027] \u003d resp.status_int"},{"line_number":1350,"context_line":"        return resp"},{"line_number":1351,"context_line":""},{"line_number":1352,"context_line":"    @public"}],"source_content_type":"text/x-python","patch_set":2,"id":"b6fff31d_b577f95f","line":1349,"updated":"2025-03-18 14:31:55.000000000","message":"ok, so status is added here (re. my comment in utils)\n\nDoing it in the decorator save repeating in every method that is decorated, but does require that ``labeled_timing_stats`` must only be applied to functions that return a swob.Response. However, timing_stats already has that assumption, so it seems reasonable.","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":1346,"context_line":"            # force pickle protocol for compatibility with py2 nodes"},{"line_number":1347,"context_line":"            resp \u003d Response(body\u003dpickle.dumps(hashes, protocol\u003d2))"},{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":"        labels[\u0027status\u0027] \u003d resp.status_int"},{"line_number":1350,"context_line":"        return resp"},{"line_number":1351,"context_line":""},{"line_number":1352,"context_line":"    @public"}],"source_content_type":"text/x-python","patch_set":2,"id":"47187eb9_503f239a","line":1349,"in_reply_to":"b6fff31d_b577f95f","updated":"2025-03-19 15:26:36.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":1346,"context_line":"            # force pickle protocol for compatibility with py2 nodes"},{"line_number":1347,"context_line":"            resp \u003d Response(body\u003dpickle.dumps(hashes, protocol\u003d2))"},{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":"        labels[\u0027status\u0027] \u003d resp.status_int"},{"line_number":1350,"context_line":"        return resp"},{"line_number":1351,"context_line":""},{"line_number":1352,"context_line":"    @public"}],"source_content_type":"text/x-python","patch_set":2,"id":"e735acce_c8ac8c16","line":1349,"in_reply_to":"b6fff31d_b577f95f","updated":"2025-03-19 17:38:02.000000000","message":"policy might be another one","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":1346,"context_line":"            # force pickle protocol for compatibility with py2 nodes"},{"line_number":1347,"context_line":"            resp \u003d Response(body\u003dpickle.dumps(hashes, protocol\u003d2))"},{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":"        labels[\u0027status\u0027] \u003d resp.status_int"},{"line_number":1350,"context_line":"        return resp"},{"line_number":1351,"context_line":""},{"line_number":1352,"context_line":"    @public"}],"source_content_type":"text/x-python","patch_set":2,"id":"367c9021_ac7c2abc","line":1349,"in_reply_to":"e735acce_c8ac8c16","updated":"2025-03-20 15:32:43.000000000","message":"policy seems should be updated in the request function?","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":1331,"context_line":"        \"\"\""},{"line_number":1332,"context_line":"        device, partition, suffix_parts, policy \u003d \\"},{"line_number":1333,"context_line":"            get_name_and_placement(request, 2, 3, True)"},{"line_number":1334,"context_line":"        timing_stats_labels[\u0027policy\u0027] \u003d int(policy)"},{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"}],"source_content_type":"text/x-python","patch_set":3,"id":"02619a31_d72485e6","line":1334,"updated":"2025-03-19 17:38:02.000000000","message":"we MUST type here b/c we don\u0027t want the policy.__repr__ as the value - we want the int (even if it\u0027ll be a str like `\u00270\u0027` or `\u00271\u0027` when query it in prom)","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"        \"\"\""},{"line_number":1332,"context_line":"        device, partition, suffix_parts, policy \u003d \\"},{"line_number":1333,"context_line":"            get_name_and_placement(request, 2, 3, True)"},{"line_number":1334,"context_line":"        timing_stats_labels[\u0027policy\u0027] \u003d int(policy)"},{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9811ed54_0bd02d83","line":1334,"in_reply_to":"02619a31_d72485e6","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1338,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"        try:"},{"line_number":1341,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"e7d6aeda_8352f80c","line":1338,"updated":"2025-03-19 17:38:02.000000000","message":"so the policy label is an int, but this can\u0027t be a bool?  I think in the end statsd will have to str all label keys and values when it builds the udp packet:\n\nhttps://github.com/NVIDIA/swift/blob/ss-release-2.34.0.0.3/swift/common/statsd_client.py#L79\n\nI don\u0027t know what\u0027s most obvious from a usage stand point - should we try (and fail?) to consistently only give labels strings, or go ahead and understand that if `\u0027%s:%s\u0027 % (k, v)` gives us the correct result we don\u0027t care much about what types we stick in here and there\u0027s no real need to double convert.","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b8c36ba081ce821122c89fb3a587f72484ad737e","unresolved":true,"context_lines":[{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1338,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"        try:"},{"line_number":1341,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"dad8c6d2_826671b9","line":1338,"in_reply_to":"041ae3b9_3fda7cc2","updated":"2025-04-03 13:59:25.000000000","message":"see https://review.opendev.org/c/openstack/swift/+/946257","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1338,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"        try:"},{"line_number":1341,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"041ae3b9_3fda7cc2","line":1338,"in_reply_to":"964dc9bc_72eb73ae","updated":"2025-04-02 16:07:18.000000000","message":"I like that LabeledStatsdClient will stringify whatever value type we give it, but I\u0027m not sure we have a test that verifies the behavior - we should add that in another patch to statsd_client","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1338,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"        try:"},{"line_number":1341,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"91d3ec25_3b87c88b","line":1338,"in_reply_to":"dad8c6d2_826671b9","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1e9e5d55cb69c19f72131f1366c6c98d6ef4f01c","unresolved":false,"context_lines":[{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1338,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"        try:"},{"line_number":1341,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"0eaf7f84_854608fc","line":1338,"in_reply_to":"dad8c6d2_826671b9","updated":"2025-04-04 17:03:28.000000000","message":"Done","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1337,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1338,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d str(skip_rehash)"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"        try:"},{"line_number":1341,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":3,"id":"964dc9bc_72eb73ae","line":1338,"in_reply_to":"e7d6aeda_8352f80c","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    X_DELETE_TYPE"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"LABELED_METRIC_NAME \u003d \u0027swift_object_request_timing\u0027"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def iter_mime_headers_and_bodies(wsgi_input, mime_boundary, read_chunk_size):"}],"source_content_type":"text/x-python","patch_set":5,"id":"c63f2fed_dc26be52","line":68,"updated":"2025-04-02 16:07:18.000000000","message":"ok this is consistent with ``\u0027swift_proxy_request_timing\u0027``","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    X_DELETE_TYPE"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"LABELED_METRIC_NAME \u003d \u0027swift_object_request_timing\u0027"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def iter_mime_headers_and_bodies(wsgi_input, mime_boundary, read_chunk_size):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5c0e7639_769945ff","line":68,"in_reply_to":"c63f2fed_dc26be52","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        super(ObjectController, self).__init__(conf)"},{"line_number":147,"context_line":"        self.logger \u003d logger or get_logger(conf, log_route\u003d\u0027object-server\u0027)"},{"line_number":148,"context_line":"        self.statsd \u003d get_labeled_statsd_client("},{"line_number":149,"context_line":"            conf, self.logger.logger)"},{"line_number":150,"context_line":"        self.node_timeout \u003d float(conf.get(\u0027node_timeout\u0027, 3))"},{"line_number":151,"context_line":"        self.container_update_timeout \u003d float("},{"line_number":152,"context_line":"            conf.get(\u0027container_update_timeout\u0027, 1))"}],"source_content_type":"text/x-python","patch_set":5,"id":"e512b611_de872447","line":149,"updated":"2025-04-02 16:07:18.000000000","message":"nit: is the line-wrap necessary? (I didn\u0027t try, just based on looking at diff)","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        super(ObjectController, self).__init__(conf)"},{"line_number":147,"context_line":"        self.logger \u003d logger or get_logger(conf, log_route\u003d\u0027object-server\u0027)"},{"line_number":148,"context_line":"        self.statsd \u003d get_labeled_statsd_client("},{"line_number":149,"context_line":"            conf, self.logger.logger)"},{"line_number":150,"context_line":"        self.node_timeout \u003d float(conf.get(\u0027node_timeout\u0027, 3))"},{"line_number":151,"context_line":"        self.container_update_timeout \u003d float("},{"line_number":152,"context_line":"            conf.get(\u0027container_update_timeout\u0027, 1))"}],"source_content_type":"text/x-python","patch_set":5,"id":"fafa1c92_2bae99e9","line":149,"in_reply_to":"e512b611_de872447","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1342,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1343,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d skip_rehash"},{"line_number":1344,"context_line":""},{"line_number":1345,"context_line":"        try:"},{"line_number":1346,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":5,"id":"d4155f73_98d776c2","line":1343,"updated":"2025-04-02 16:07:18.000000000","message":"nit: might be easier to grok if all the labels are defined in one place i.e. move line 1339 to here","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"        suffixes \u003d suffix_parts.split(\u0027-\u0027) if suffix_parts else []"},{"line_number":1342,"context_line":"        skip_rehash \u003d bool(suffixes)"},{"line_number":1343,"context_line":"        timing_stats_labels[\u0027skip_rehash\u0027] \u003d skip_rehash"},{"line_number":1344,"context_line":""},{"line_number":1345,"context_line":"        try:"},{"line_number":1346,"context_line":"            hashes \u003d self._diskfile_router[policy].get_hashes("}],"source_content_type":"text/x-python","patch_set":5,"id":"98ffeb29_43ca6841","line":1343,"in_reply_to":"d4155f73_98d776c2","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b4910a85dd95406f2b449ac9e3b61371dfccdaef","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    X_DELETE_TYPE"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"LABELED_METRIC_NAME \u003d \u0027swift_object_request_timing\u0027"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def iter_mime_headers_and_bodies(wsgi_input, mime_boundary, read_chunk_size):"}],"source_content_type":"text/x-python","patch_set":9,"id":"35584e4f_ecc7ead7","line":68,"updated":"2025-04-09 15:43:47.000000000","message":"this may not be the ONLY labeled metric that the object-server EVER produces... although it\u0027s possible.\n\nWe could also go with something like:\n\n    LABELED_REQ_TIMING_METRIC\n    \nOr if we agree labeled metrics are the future even just:\n\n    TIMING_METRIC\n    \nIf we wanted to get super-fancy we could try to use a partial:\n\n    legacy_timing_stats \u003d utils.timing_stats\n    timing_stats \u003d functools.partial(utils.labeled_timing_stats,\n                                     \u0027swift_object_server_timing\u0027)\n    ...\n    @legacy_timing_stats(sample_rate\u003d0.1)\n    @timing_stats(sample_rate\u003d0.1)\n    def REPLICATE(...):\n        ...\n    @legacy_timing_stats()\n    @timing_stats()\n    def GET(...):\n        ...","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"714ad90bc8ea1177282400ccca29d99dcc0fd9e5","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    X_DELETE_TYPE"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"LABELED_METRIC_NAME \u003d \u0027swift_object_request_timing\u0027"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def iter_mime_headers_and_bodies(wsgi_input, mime_boundary, read_chunk_size):"}],"source_content_type":"text/x-python","patch_set":9,"id":"ca5b538f_317f7127","line":68,"in_reply_to":"35584e4f_ecc7ead7","updated":"2025-04-17 19:22:05.000000000","message":"Consider changing this in follow-up patch","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b4910a85dd95406f2b449ac9e3b61371dfccdaef","unresolved":true,"context_lines":[{"line_number":144,"context_line":"        /etc/swift/object-server.conf-sample."},{"line_number":145,"context_line":"        \"\"\""},{"line_number":146,"context_line":"        super(ObjectController, self).__init__(conf)"},{"line_number":147,"context_line":"        self.logger \u003d logger or get_logger(conf, log_route\u003d\u0027object-server\u0027)"},{"line_number":148,"context_line":"        self.statsd \u003d get_labeled_statsd_client(conf, self.logger.logger)"},{"line_number":149,"context_line":"        self.node_timeout \u003d float(conf.get(\u0027node_timeout\u0027, 3))"},{"line_number":150,"context_line":"        self.container_update_timeout \u003d float("}],"source_content_type":"text/x-python","patch_set":9,"id":"eca4872e_f4b91dc6","line":147,"updated":"2025-04-09 15:43:47.000000000","message":"did log_route somehow get involved in legacy metrics prefix?\n\n```\n4716 ss_object_server_timing{hostname\u003d\"host.example.net\",method\u003d\"GET\",server\u003d\"-r\",quantile\u003d\"0.5\"} 0.0738175\n```","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"714ad90bc8ea1177282400ccca29d99dcc0fd9e5","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        /etc/swift/object-server.conf-sample."},{"line_number":145,"context_line":"        \"\"\""},{"line_number":146,"context_line":"        super(ObjectController, self).__init__(conf)"},{"line_number":147,"context_line":"        self.logger \u003d logger or get_logger(conf, log_route\u003d\u0027object-server\u0027)"},{"line_number":148,"context_line":"        self.statsd \u003d get_labeled_statsd_client(conf, self.logger.logger)"},{"line_number":149,"context_line":"        self.node_timeout \u003d float(conf.get(\u0027node_timeout\u0027, 3))"},{"line_number":150,"context_line":"        self.container_update_timeout \u003d float("}],"source_content_type":"text/x-python","patch_set":9,"id":"c472ad1b_57c94136","line":147,"in_reply_to":"eca4872e_f4b91dc6","updated":"2025-04-17 19:22:05.000000000","message":"Acknowledged","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"}],"test/unit/common/test_utils.py":[{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"9c6997a8157df29097218fdd3b27b68f3c3c1f2b","unresolved":true,"context_lines":[{"line_number":8065,"context_line":"        self.assertEqual("},{"line_number":8066,"context_line":"            {\u0027timing_since\u0027: [((\u0027mock_controller_request_timing\u0027, now), {"},{"line_number":8067,"context_line":"                \u0027labels\u0027: {"},{"line_number":8068,"context_line":"                    \u0027method\u0027: \u0027METHOD\u0027,"},{"line_number":8069,"context_line":"                    \u0027status\u0027: 200"},{"line_number":8070,"context_line":"                }"},{"line_number":8071,"context_line":"            })]},"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff5a1f21_505036dd","line":8068,"range":{"start_line":8068,"start_character":31,"end_line":8068,"end_character":37},"updated":"2025-03-18 14:26:26.000000000","message":"Shouldn\u0027t the method label actually state the method instead of the word method? like \u0027method\u0027: \u0027GET\u0027? \nIf so, we can tell the labels here.\ndef METHOD(mock_self, req, labels):\n                labels[\u0027method\u0027] \u003d req.method\n                labels[\u0027status\u0027] \u003d mock_self.status\n                return Response(status\u003dmock_self.status)\nNot sure if that\u0027s the goal to have method \u003d method though.","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":8065,"context_line":"        self.assertEqual("},{"line_number":8066,"context_line":"            {\u0027timing_since\u0027: [((\u0027mock_controller_request_timing\u0027, now), {"},{"line_number":8067,"context_line":"                \u0027labels\u0027: {"},{"line_number":8068,"context_line":"                    \u0027method\u0027: \u0027METHOD\u0027,"},{"line_number":8069,"context_line":"                    \u0027status\u0027: 200"},{"line_number":8070,"context_line":"                }"},{"line_number":8071,"context_line":"            })]},"}],"source_content_type":"text/x-python","patch_set":2,"id":"597be11d_6aa56d71","line":8068,"range":{"start_line":8068,"start_character":31,"end_line":8068,"end_character":37},"in_reply_to":"671bb3c2_a0f579fa","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":8065,"context_line":"        self.assertEqual("},{"line_number":8066,"context_line":"            {\u0027timing_since\u0027: [((\u0027mock_controller_request_timing\u0027, now), {"},{"line_number":8067,"context_line":"                \u0027labels\u0027: {"},{"line_number":8068,"context_line":"                    \u0027method\u0027: \u0027METHOD\u0027,"},{"line_number":8069,"context_line":"                    \u0027status\u0027: 200"},{"line_number":8070,"context_line":"                }"},{"line_number":8071,"context_line":"            })]},"}],"source_content_type":"text/x-python","patch_set":2,"id":"12a4d0ee_8f4dc4b6","line":8068,"range":{"start_line":8068,"start_character":31,"end_line":8068,"end_character":37},"in_reply_to":"ff5a1f21_505036dd","updated":"2025-03-19 15:26:36.000000000","message":"Indeed it was confusing as the decorator uses the function name, changed the method to GET","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":8065,"context_line":"        self.assertEqual("},{"line_number":8066,"context_line":"            {\u0027timing_since\u0027: [((\u0027mock_controller_request_timing\u0027, now), {"},{"line_number":8067,"context_line":"                \u0027labels\u0027: {"},{"line_number":8068,"context_line":"                    \u0027method\u0027: \u0027METHOD\u0027,"},{"line_number":8069,"context_line":"                    \u0027status\u0027: 200"},{"line_number":8070,"context_line":"                }"},{"line_number":8071,"context_line":"            })]},"}],"source_content_type":"text/x-python","patch_set":2,"id":"671bb3c2_a0f579fa","line":8068,"range":{"start_line":8068,"start_character":31,"end_line":8068,"end_character":37},"in_reply_to":"ff5a1f21_505036dd","updated":"2025-03-19 17:38:02.000000000","message":"yes!  using `req.method` would be better than the method\u0027s name IMHO.\n\n```\n\u003e\u003e\u003e from swift.obj.server import ObjectController\n\u003e\u003e\u003e ObjectController.GET.__name__\n\u0027GET\u0027\n```","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b03070f601d4661592dc3459692eeeb21cccdb3","unresolved":true,"context_lines":[{"line_number":8094,"context_line":"                    \u0027status\u0027: 500"},{"line_number":8095,"context_line":"                }"},{"line_number":8096,"context_line":"            })]},"},{"line_number":8097,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8098,"context_line":""},{"line_number":8099,"context_line":"    def test_memcached_timing_stats(self):"},{"line_number":8100,"context_line":"        class MockMemcached(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7913783b_f528947b","line":8097,"updated":"2025-03-18 14:31:55.000000000","message":"nice\n\nWould be good to have a scenario where the METHOD adds other labels (like policy), and also updates existing labels - to verify the precedence.","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"a80e6625998b33ad18c2d3c3eb9eb21f33fba247","unresolved":false,"context_lines":[{"line_number":8094,"context_line":"                    \u0027status\u0027: 500"},{"line_number":8095,"context_line":"                }"},{"line_number":8096,"context_line":"            })]},"},{"line_number":8097,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8098,"context_line":""},{"line_number":8099,"context_line":"    def test_memcached_timing_stats(self):"},{"line_number":8100,"context_line":"        class MockMemcached(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"86894e98_d39d6bc6","line":8097,"in_reply_to":"7913783b_f528947b","updated":"2025-03-19 15:26:36.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":8094,"context_line":"                    \u0027status\u0027: 500"},{"line_number":8095,"context_line":"                }"},{"line_number":8096,"context_line":"            })]},"},{"line_number":8097,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8098,"context_line":""},{"line_number":8099,"context_line":"    def test_memcached_timing_stats(self):"},{"line_number":8100,"context_line":"        class MockMemcached(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"b04cd9a8_f8f77307","line":8097,"in_reply_to":"7913783b_f528947b","updated":"2025-03-19 17:38:02.000000000","message":"right!  never write one test when 5 will do just as well!\n\nit also helps boost your test:code line ratio 😜","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":8094,"context_line":"                    \u0027status\u0027: 500"},{"line_number":8095,"context_line":"                }"},{"line_number":8096,"context_line":"            })]},"},{"line_number":8097,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8098,"context_line":""},{"line_number":8099,"context_line":"    def test_memcached_timing_stats(self):"},{"line_number":8100,"context_line":"        class MockMemcached(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"920c5d1a_f1b3a79c","line":8097,"in_reply_to":"b04cd9a8_f8f77307","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"a050f79d8d2b5f00eb6e221f9d021e8c8826eae3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":8053,"context_line":""},{"line_number":8054,"context_line":"            @utils.labeled_timing_stats("},{"line_number":8055,"context_line":"                metric\u003d\u0027mock_controller_request_timing\u0027)"},{"line_number":8056,"context_line":"            def GET(mock_self, req, labels):"},{"line_number":8057,"context_line":"                labels[\u0027status\u0027] \u003d mock_self.status"},{"line_number":8058,"context_line":"                return Response(status\u003dmock_self.status)"},{"line_number":8059,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"500b198d_b5978066","line":8056,"updated":"2025-03-19 17:38:02.000000000","message":"I don\u0027t understand why the decorator can call this method with the timing_stats_labels kwarg.\n\nLike... it\u0027s really bothering me... maybe I\u0027m misunderstanding the zuul results.","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":8053,"context_line":""},{"line_number":8054,"context_line":"            @utils.labeled_timing_stats("},{"line_number":8055,"context_line":"                metric\u003d\u0027mock_controller_request_timing\u0027)"},{"line_number":8056,"context_line":"            def GET(mock_self, req, labels):"},{"line_number":8057,"context_line":"                labels[\u0027status\u0027] \u003d mock_self.status"},{"line_number":8058,"context_line":"                return Response(status\u003dmock_self.status)"},{"line_number":8059,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"19ba5758_e56d79b1","line":8056,"in_reply_to":"500b198d_b5978066","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":7995,"context_line":"        self.status \u003d status"},{"line_number":7996,"context_line":"        self.extra_labels \u003d extra_labels or {}"},{"line_number":7997,"context_line":""},{"line_number":7998,"context_line":"    def _update_labels(self, req, labels):"},{"line_number":7999,"context_line":"        labels.update(self.extra_labels)"},{"line_number":8000,"context_line":""},{"line_number":8001,"context_line":"    @utils.labeled_timing_stats(metric\u003d\u0027my_timing_metric\u0027)"},{"line_number":8002,"context_line":"    def handle_req(self, req, timing_stats_labels):"},{"line_number":8003,"context_line":"        self._update_labels(req, timing_stats_labels)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3bec37b5_3dfd7f24","line":8000,"range":{"start_line":7998,"start_character":4,"end_line":8000,"end_character":0},"updated":"2025-04-02 16:07:18.000000000","message":"nit: a method seems unnecessary for this rather than doing the update in handle_req. I can see it is overridden in one test but it wuld be pretty simple to override handle_req instead","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":7995,"context_line":"        self.status \u003d status"},{"line_number":7996,"context_line":"        self.extra_labels \u003d extra_labels or {}"},{"line_number":7997,"context_line":""},{"line_number":7998,"context_line":"    def _update_labels(self, req, labels):"},{"line_number":7999,"context_line":"        labels.update(self.extra_labels)"},{"line_number":8000,"context_line":""},{"line_number":8001,"context_line":"    @utils.labeled_timing_stats(metric\u003d\u0027my_timing_metric\u0027)"},{"line_number":8002,"context_line":"    def handle_req(self, req, timing_stats_labels):"},{"line_number":8003,"context_line":"        self._update_labels(req, timing_stats_labels)"}],"source_content_type":"text/x-python","patch_set":5,"id":"b065088e_73b5dba6","line":8000,"range":{"start_line":7998,"start_character":4,"end_line":8000,"end_character":0},"in_reply_to":"3bec37b5_3dfd7f24","updated":"2025-04-04 16:19:40.000000000","message":"seems could be better if overriding less code there?","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":8069,"context_line":"                    \u0027status\u0027: 404,"},{"line_number":8070,"context_line":"                }"},{"line_number":8071,"context_line":"            })]},"},{"line_number":8072,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8073,"context_line":""},{"line_number":8074,"context_line":"    def test_labeled_timing_stats_can_not_override_method(self):"},{"line_number":8075,"context_line":"        req \u003d Request.blank(\u0027/v1/AUTH_test/c/o\u0027, method\u003d\u0027POST\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"a5443168_cd09d855","line":8072,"updated":"2025-04-02 16:07:18.000000000","message":"nice","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":8069,"context_line":"                    \u0027status\u0027: 404,"},{"line_number":8070,"context_line":"                }"},{"line_number":8071,"context_line":"            })]},"},{"line_number":8072,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8073,"context_line":""},{"line_number":8074,"context_line":"    def test_labeled_timing_stats_can_not_override_method(self):"},{"line_number":8075,"context_line":"        req \u003d Request.blank(\u0027/v1/AUTH_test/c/o\u0027, method\u003d\u0027POST\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"819dfcfa_6f95f715","line":8072,"in_reply_to":"a5443168_cd09d855","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":8086,"context_line":"            })]},"},{"line_number":8087,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8088,"context_line":""},{"line_number":8089,"context_line":"    def test_labeled_timing_stats_can_remove_labels(self):"},{"line_number":8090,"context_line":""},{"line_number":8091,"context_line":"        class MutilatingController(MockLabeledTimingController):"},{"line_number":8092,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"8c3ed7e4_e9759dd1","line":8089,"range":{"start_line":8089,"start_character":34,"end_line":8089,"end_character":39},"updated":"2025-04-02 16:07:18.000000000","message":"should this be ``cannot``?","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":8086,"context_line":"            })]},"},{"line_number":8087,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8088,"context_line":""},{"line_number":8089,"context_line":"    def test_labeled_timing_stats_can_remove_labels(self):"},{"line_number":8090,"context_line":""},{"line_number":8091,"context_line":"        class MutilatingController(MockLabeledTimingController):"},{"line_number":8092,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"24816aae_f972d028","line":8089,"range":{"start_line":8089,"start_character":34,"end_line":8089,"end_character":39},"in_reply_to":"8c3ed7e4_e9759dd1","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b97bb04002b92f8903f2735bc8c25cdd71f67e13","unresolved":true,"context_lines":[{"line_number":8105,"context_line":"                    \u0027status\u0027: 42,"},{"line_number":8106,"context_line":"                }"},{"line_number":8107,"context_line":"            })]},"},{"line_number":8108,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8109,"context_line":""},{"line_number":8110,"context_line":""},{"line_number":8111,"context_line":"class TestTimingStatsDecorators(unittest.TestCase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"aba104f4_075e0b7a","line":8108,"updated":"2025-04-09 12:30:49.000000000","message":"nice tests. The one regression that I made without a test failing was for the controller mutating req.method (see sq follow up patch for a suggestion)","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"96529dae6eee5d0bcda15f52ff387d0838b4091d","unresolved":false,"context_lines":[{"line_number":8105,"context_line":"                    \u0027status\u0027: 42,"},{"line_number":8106,"context_line":"                }"},{"line_number":8107,"context_line":"            })]},"},{"line_number":8108,"context_line":"            mock_controller.statsd.calls)"},{"line_number":8109,"context_line":""},{"line_number":8110,"context_line":""},{"line_number":8111,"context_line":"class TestTimingStatsDecorators(unittest.TestCase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"45b7d279_f79c27bc","line":8108,"in_reply_to":"aba104f4_075e0b7a","updated":"2025-04-10 15:36:24.000000000","message":"Acknowledged","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":234,"context_line":"                           FakeLabeledStatsdClient):"},{"line_number":235,"context_line":"            app \u003d object_server.ObjectController(conf, logger\u003dself.logger)"},{"line_number":236,"context_line":"            statsd_client \u003d app.logger.logger.statsd_client"},{"line_number":237,"context_line":"            statsd \u003d app.statsd"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"            with mock.patch.object(statsd_client, \u0027random\u0027, return_value\u003d0), \\"},{"line_number":240,"context_line":"                    mock.patch.object(statsd, \u0027random\u0027, return_value\u003d0):"}],"source_content_type":"text/x-python","patch_set":3,"id":"58f3123a_01bd3838","line":237,"updated":"2025-03-19 17:38:02.000000000","message":"when I revert the changes in obj/server to make sure the legacy stats still work this test blows up pretty early.\n\nit worked out better for me to have two tests:\n\n```\n(vagrant-swift-all-in-one) cgerrard@NVStation:~/Workspace/vagrant-swift-all-in-one/swift$ git diff\ndiff --git a/test/unit/obj/test_server.py b/test/unit/obj/test_server.py\nindex 86f9b2702..e13858122 100644\n--- a/test/unit/obj/test_server.py\n+++ b/test/unit/obj/test_server.py\n@@ -224,7 +224,28 @@ class TestObjectController(BaseTestCase):\n             if out_body and (200 \u003c\u003d res \u003c 300):\n                 self.assertEqual(resp.body, out_body)\n \n-    def test_timing_stats(self):\n+    def test_legacy_timing_stats(self):\n+        conf \u003d {}\n+        now \u003d time()\n+        with mock.patch(\u0027swift.common.utils.time.time\u0027, return_value\u003dnow), \\\n+                mock.patch(\u0027swift.common.statsd_client.StatsdClient\u0027,\n+                           FakeStatsdClient), \\\n+                mock.patch(\u0027swift.common.statsd_client.LabeledStatsdClient\u0027,\n+                           FakeLabeledStatsdClient):\n+            app \u003d object_server.ObjectController(conf, logger\u003dself.logger)\n+            statsd_client \u003d app.logger.logger.statsd_client\n+\n+            with mock.patch.object(statsd_client, \u0027random\u0027, return_value\u003d0):\n+                req \u003d Request.blank(\n+                    \u0027/v1/a/c\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027})\n+                app.REPLICATE(req)\n+\n+        self.assertIsInstance(statsd_client, FakeStatsdClient)\n+        self.assertEqual({\u0027timing_since\u0027: [((\u0027REPLICATE.errors.timing\u0027, now), {\n+            \u0027sample_rate\u0027: 0.1\n+        })]}, statsd_client.calls)\n+\n+    def test_labeled_timing_stats(self):\n         conf \u003d {}\n         now \u003d time()\n         with mock.patch(\u0027swift.common.utils.time.time\u0027, return_value\u003dnow), \\\n```","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":234,"context_line":"                           FakeLabeledStatsdClient):"},{"line_number":235,"context_line":"            app \u003d object_server.ObjectController(conf, logger\u003dself.logger)"},{"line_number":236,"context_line":"            statsd_client \u003d app.logger.logger.statsd_client"},{"line_number":237,"context_line":"            statsd \u003d app.statsd"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"            with mock.patch.object(statsd_client, \u0027random\u0027, return_value\u003d0), \\"},{"line_number":240,"context_line":"                    mock.patch.object(statsd, \u0027random\u0027, return_value\u003d0):"}],"source_content_type":"text/x-python","patch_set":3,"id":"615aee4d_fb39c1eb","line":237,"in_reply_to":"58f3123a_01bd3838","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"21f772c73bea03df20fad000411fd164946353ef","unresolved":true,"context_lines":[{"line_number":240,"context_line":"                    mock.patch.object(statsd, \u0027random\u0027, return_value\u003d0):"},{"line_number":241,"context_line":"                req \u003d Request.blank("},{"line_number":242,"context_line":"                    \u0027/v1/a/c\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027})"},{"line_number":243,"context_line":"                app.REPLICATE(req)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.assertIsInstance(statsd_client, FakeStatsdClient)"},{"line_number":246,"context_line":"        self.assertIsInstance(statsd, FakeLabeledStatsdClient)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bb95c4d8_ef5396b7","line":243,"range":{"start_line":243,"start_character":16,"end_line":243,"end_character":34},"updated":"2025-03-19 16:35:16.000000000","message":"perhaps more intuitive to have ``REQUEST_METHOD: REPLICATE`` and then use ``req.get_response(app)``","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":240,"context_line":"                    mock.patch.object(statsd, \u0027random\u0027, return_value\u003d0):"},{"line_number":241,"context_line":"                req \u003d Request.blank("},{"line_number":242,"context_line":"                    \u0027/v1/a/c\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027})"},{"line_number":243,"context_line":"                app.REPLICATE(req)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.assertIsInstance(statsd_client, FakeStatsdClient)"},{"line_number":246,"context_line":"        self.assertIsInstance(statsd, FakeLabeledStatsdClient)"}],"source_content_type":"text/x-python","patch_set":3,"id":"32b478a0_8f51ef87","line":243,"range":{"start_line":243,"start_character":16,"end_line":243,"end_character":34},"in_reply_to":"bb95c4d8_ef5396b7","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b570b3f00408162e1dfe472c9ffd408fcfa8ba98","unresolved":true,"context_lines":[{"line_number":245,"context_line":"        self.assertIsInstance(statsd_client, FakeStatsdClient)"},{"line_number":246,"context_line":"        self.assertIsInstance(statsd, FakeLabeledStatsdClient)"},{"line_number":247,"context_line":"        self.assertEqual({\u0027timing_since\u0027: [((\u0027REPLICATE.errors.timing\u0027, now), {"},{"line_number":248,"context_line":"            \u0027sample_rate\u0027: 0.1"},{"line_number":249,"context_line":"        })]}, statsd_client.calls)"},{"line_number":250,"context_line":"        self.assertEqual("},{"line_number":251,"context_line":"            {\u0027timing_since\u0027: [((\u0027swift_object_request_timing\u0027, now), {"}],"source_content_type":"text/x-python","patch_set":3,"id":"347dc2af_8e21ce01","line":248,"updated":"2025-03-19 17:38:02.000000000","message":"look at those legacy metrics! nice!","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"14d23696909b4126906355b0735654a990f8b1c2","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        self.assertIsInstance(statsd_client, FakeStatsdClient)"},{"line_number":246,"context_line":"        self.assertIsInstance(statsd, FakeLabeledStatsdClient)"},{"line_number":247,"context_line":"        self.assertEqual({\u0027timing_since\u0027: [((\u0027REPLICATE.errors.timing\u0027, now), {"},{"line_number":248,"context_line":"            \u0027sample_rate\u0027: 0.1"},{"line_number":249,"context_line":"        })]}, statsd_client.calls)"},{"line_number":250,"context_line":"        self.assertEqual("},{"line_number":251,"context_line":"            {\u0027timing_since\u0027: [((\u0027swift_object_request_timing\u0027, now), {"}],"source_content_type":"text/x-python","patch_set":3,"id":"c319bebe_b577d7ff","line":248,"in_reply_to":"347dc2af_8e21ce01","updated":"2025-03-20 15:32:43.000000000","message":"Acknowledged","commit_id":"66a7ecd2916fbfc9ede6108d51cdfe8f60d9135f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":237,"context_line":""},{"line_number":238,"context_line":"            with mock.patch.object(statsd_client, \u0027random\u0027, return_value\u003d0), \\"},{"line_number":239,"context_line":"                    mock.patch.object(statsd, \u0027random\u0027, return_value\u003d0):"},{"line_number":240,"context_line":"                _, _, _ \u003d req.call_application(app)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        self.assertIsInstance(statsd_client, FakeStatsdClient)"},{"line_number":243,"context_line":"        self.assertIsInstance(statsd, FakeLabeledStatsdClient)"}],"source_content_type":"text/x-python","patch_set":5,"id":"60d61fbb_a567717f","line":240,"range":{"start_line":240,"start_character":16,"end_line":240,"end_character":25},"updated":"2025-04-02 16:07:18.000000000","message":"nit: the assignment isn\u0027t necessary","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":237,"context_line":""},{"line_number":238,"context_line":"            with mock.patch.object(statsd_client, \u0027random\u0027, return_value\u003d0), \\"},{"line_number":239,"context_line":"                    mock.patch.object(statsd, \u0027random\u0027, return_value\u003d0):"},{"line_number":240,"context_line":"                _, _, _ \u003d req.call_application(app)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        self.assertIsInstance(statsd_client, FakeStatsdClient)"},{"line_number":243,"context_line":"        self.assertIsInstance(statsd, FakeLabeledStatsdClient)"}],"source_content_type":"text/x-python","patch_set":5,"id":"d545c7b2_94b8a735","line":240,"range":{"start_line":240,"start_character":16,"end_line":240,"end_character":25},"in_reply_to":"60d61fbb_a567717f","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":248,"context_line":"            \u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027})"},{"line_number":249,"context_line":"        now \u003d time()"},{"line_number":250,"context_line":"        statsd_client, statsd \u003d self._do_test_timing_stats(req, now)"},{"line_number":251,"context_line":"        self.assertEqual({\u0027timing_since\u0027: [((\u0027GET.errors.timing\u0027, now), {"},{"line_number":252,"context_line":"        })]}, statsd_client.calls)"},{"line_number":253,"context_line":"        self.assertEqual("},{"line_number":254,"context_line":"            {\u0027timing_since\u0027: [((\u0027swift_object_request_timing\u0027, now), {"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fad09cb_f80f7c94","line":251,"range":{"start_line":251,"start_character":46,"end_line":251,"end_character":63},"updated":"2025-04-02 16:07:18.000000000","message":"so this patch is very helpfully adding test coverage for the existing legacy timing stat...kudos!\n\nBUT...to be complete we ought to have a test with a successful response code that hits the ``GET.timing`` metric too. i.e. we should aspire to:\n\n```\ntest_legacy_and_labeled_timing_stats_get_success()\ntest_legacy_and_labeled_timing_stats_get_error()\n\n```","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"7ea7622491995a46beba988df7fd58b5dba70c5a","unresolved":false,"context_lines":[{"line_number":248,"context_line":"            \u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027})"},{"line_number":249,"context_line":"        now \u003d time()"},{"line_number":250,"context_line":"        statsd_client, statsd \u003d self._do_test_timing_stats(req, now)"},{"line_number":251,"context_line":"        self.assertEqual({\u0027timing_since\u0027: [((\u0027GET.errors.timing\u0027, now), {"},{"line_number":252,"context_line":"        })]}, statsd_client.calls)"},{"line_number":253,"context_line":"        self.assertEqual("},{"line_number":254,"context_line":"            {\u0027timing_since\u0027: [((\u0027swift_object_request_timing\u0027, now), {"}],"source_content_type":"text/x-python","patch_set":5,"id":"a6fd41d5_b5c74317","line":251,"range":{"start_line":251,"start_character":46,"end_line":251,"end_character":63},"in_reply_to":"591061bf_bbbe6436","updated":"2025-04-10 16:30:47.000000000","message":"Per our discussion, working on this in follow-up patch","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"96529dae6eee5d0bcda15f52ff387d0838b4091d","unresolved":false,"context_lines":[{"line_number":248,"context_line":"            \u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027})"},{"line_number":249,"context_line":"        now \u003d time()"},{"line_number":250,"context_line":"        statsd_client, statsd \u003d self._do_test_timing_stats(req, now)"},{"line_number":251,"context_line":"        self.assertEqual({\u0027timing_since\u0027: [((\u0027GET.errors.timing\u0027, now), {"},{"line_number":252,"context_line":"        })]}, statsd_client.calls)"},{"line_number":253,"context_line":"        self.assertEqual("},{"line_number":254,"context_line":"            {\u0027timing_since\u0027: [((\u0027swift_object_request_timing\u0027, now), {"}],"source_content_type":"text/x-python","patch_set":5,"id":"591061bf_bbbe6436","line":251,"range":{"start_line":251,"start_character":46,"end_line":251,"end_character":63},"in_reply_to":"9fad09cb_f80f7c94","updated":"2025-04-10 15:36:24.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7be2c43aed0f58d242c3fa225b5c9206220568b","unresolved":true,"context_lines":[{"line_number":257,"context_line":"                    \u0027account\u0027: \u0027a\u0027,"},{"line_number":258,"context_line":"                    \u0027container\u0027: \u0027c\u0027,"},{"line_number":259,"context_line":"                    \u0027policy\u0027: mock.ANY,"},{"line_number":260,"context_line":"                    \u0027status\u0027: mock.ANY"},{"line_number":261,"context_line":"                },"},{"line_number":262,"context_line":"            })]},"},{"line_number":263,"context_line":"            statsd.calls)"}],"source_content_type":"text/x-python","patch_set":5,"id":"eeaf72b9_49b872a4","line":260,"updated":"2025-04-02 16:07:18.000000000","message":"if we know the policy and status then let\u0027s explicitly assert them, if only to avoid curious folks like me wondering what the ANY is ;-)","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"d4f7d8d1da7dc1931d25c528eb55cc5ed1de258b","unresolved":false,"context_lines":[{"line_number":257,"context_line":"                    \u0027account\u0027: \u0027a\u0027,"},{"line_number":258,"context_line":"                    \u0027container\u0027: \u0027c\u0027,"},{"line_number":259,"context_line":"                    \u0027policy\u0027: mock.ANY,"},{"line_number":260,"context_line":"                    \u0027status\u0027: mock.ANY"},{"line_number":261,"context_line":"                },"},{"line_number":262,"context_line":"            })]},"},{"line_number":263,"context_line":"            statsd.calls)"}],"source_content_type":"text/x-python","patch_set":5,"id":"dce290f3_f1f1b036","line":260,"in_reply_to":"eeaf72b9_49b872a4","updated":"2025-04-04 16:19:40.000000000","message":"Acknowledged","commit_id":"fd59017ed7b5efddf2b8111f8c57eea5019fd7db"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b97bb04002b92f8903f2735bc8c25cdd71f67e13","unresolved":true,"context_lines":[{"line_number":225,"context_line":"                self.assertEqual(resp.body, out_body)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def _do_test_timing_stats(self, req, now):"},{"line_number":228,"context_line":"        conf \u003d {}"},{"line_number":229,"context_line":"        with mock.patch(\u0027swift.common.utils.time.time\u0027, return_value\u003dnow), \\"},{"line_number":230,"context_line":"                mock.patch(\u0027swift.common.statsd_client.StatsdClient\u0027,"},{"line_number":231,"context_line":"                           FakeStatsdClient), \\"}],"source_content_type":"text/x-python","patch_set":9,"id":"fb69a6ad_79a1262c","line":228,"range":{"start_line":228,"start_character":8,"end_line":228,"end_character":17},"updated":"2025-04-09 12:30:49.000000000","message":"the empty is conf is why the response is a 507. The object server has no device mounts/dirs. Compare to the self.conf which sets a tempdir and disables mount_check.\n\nSo we could add tests for the \u0027success\u0027 case by using self.conf - see follow up","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"96529dae6eee5d0bcda15f52ff387d0838b4091d","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                self.assertEqual(resp.body, out_body)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def _do_test_timing_stats(self, req, now):"},{"line_number":228,"context_line":"        conf \u003d {}"},{"line_number":229,"context_line":"        with mock.patch(\u0027swift.common.utils.time.time\u0027, return_value\u003dnow), \\"},{"line_number":230,"context_line":"                mock.patch(\u0027swift.common.statsd_client.StatsdClient\u0027,"},{"line_number":231,"context_line":"                           FakeStatsdClient), \\"}],"source_content_type":"text/x-python","patch_set":9,"id":"f246caf7_2f8839fc","line":228,"range":{"start_line":228,"start_character":8,"end_line":228,"end_character":17},"in_reply_to":"fb69a6ad_79a1262c","updated":"2025-04-10 15:36:24.000000000","message":"Acknowledged","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b97bb04002b92f8903f2735bc8c25cdd71f67e13","unresolved":true,"context_lines":[{"line_number":255,"context_line":"            {\u0027timing_since\u0027: [((\u0027swift_object_request_timing\u0027, now), {"},{"line_number":256,"context_line":"                \u0027labels\u0027: {"},{"line_number":257,"context_line":"                    \u0027method\u0027: \u0027REPLICATE\u0027,"},{"line_number":258,"context_line":"                    \u0027policy\u0027: 0,"},{"line_number":259,"context_line":"                    \u0027skip_rehash\u0027: False,"},{"line_number":260,"context_line":"                    \u0027status\u0027: 507"},{"line_number":261,"context_line":"                },"},{"line_number":262,"context_line":"                \u0027sample_rate\u0027: 0.1"}],"source_content_type":"text/x-python","patch_set":9,"id":"3bacdb84_48c80c93","line":259,"range":{"start_line":258,"start_character":20,"end_line":259,"end_character":41},"updated":"2025-04-09 12:30:49.000000000","message":"it would be good to have test cases that vary policy and skip_rehash. See follow up","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"96529dae6eee5d0bcda15f52ff387d0838b4091d","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            {\u0027timing_since\u0027: [((\u0027swift_object_request_timing\u0027, now), {"},{"line_number":256,"context_line":"                \u0027labels\u0027: {"},{"line_number":257,"context_line":"                    \u0027method\u0027: \u0027REPLICATE\u0027,"},{"line_number":258,"context_line":"                    \u0027policy\u0027: 0,"},{"line_number":259,"context_line":"                    \u0027skip_rehash\u0027: False,"},{"line_number":260,"context_line":"                    \u0027status\u0027: 507"},{"line_number":261,"context_line":"                },"},{"line_number":262,"context_line":"                \u0027sample_rate\u0027: 0.1"}],"source_content_type":"text/x-python","patch_set":9,"id":"0890aba4_c57d7d8b","line":259,"range":{"start_line":258,"start_character":20,"end_line":259,"end_character":41},"in_reply_to":"3bacdb84_48c80c93","updated":"2025-04-10 15:36:24.000000000","message":"Acknowledged","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b97bb04002b92f8903f2735bc8c25cdd71f67e13","unresolved":true,"context_lines":[{"line_number":6987,"context_line":"                                                req, \u0027sda1\u0027, policy)"},{"line_number":6988,"context_line":"        self.assertEqual({"},{"line_number":6989,"context_line":"            \u0027debug\u0027: ["},{"line_number":6990,"context_line":"                \u0027Labeled statsd mode: disabled (test-object-controller)\u0027,"},{"line_number":6991,"context_line":"                \"Proxy X-Delete-At-Container \u0027%s\u0027 does not match expected \""},{"line_number":6992,"context_line":"                \"\u0027%s\u0027 for current expirer_config.\" % (unexpected_container,"},{"line_number":6993,"context_line":"                                                      expected_container)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5e79e394_44a3bf64","line":6990,"updated":"2025-04-09 12:30:49.000000000","message":"as per comment at line 7041. We should clear the logger before the test is sent so that this test isn\u0027t polluted. IIUC the intent of this assertion was that the request did not cause any log lines to be emitted.","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"96529dae6eee5d0bcda15f52ff387d0838b4091d","unresolved":false,"context_lines":[{"line_number":6987,"context_line":"                                                req, \u0027sda1\u0027, policy)"},{"line_number":6988,"context_line":"        self.assertEqual({"},{"line_number":6989,"context_line":"            \u0027debug\u0027: ["},{"line_number":6990,"context_line":"                \u0027Labeled statsd mode: disabled (test-object-controller)\u0027,"},{"line_number":6991,"context_line":"                \"Proxy X-Delete-At-Container \u0027%s\u0027 does not match expected \""},{"line_number":6992,"context_line":"                \"\u0027%s\u0027 for current expirer_config.\" % (unexpected_container,"},{"line_number":6993,"context_line":"                                                      expected_container)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3c3670f1_069291aa","line":6990,"in_reply_to":"5e79e394_44a3bf64","updated":"2025-04-10 15:36:24.000000000","message":"Acknowledged","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b97bb04002b92f8903f2735bc8c25cdd71f67e13","unresolved":true,"context_lines":[{"line_number":7019,"context_line":"            given_args.extend(args)"},{"line_number":7020,"context_line":""},{"line_number":7021,"context_line":"        self.object_controller.async_update \u003d fake_async_update"},{"line_number":7022,"context_line":"        self.object_controller.logger \u003d self.logger"},{"line_number":7023,"context_line":"        delete_at \u003d time()"},{"line_number":7024,"context_line":"        req_headers \u003d {"},{"line_number":7025,"context_line":"            \u0027X-Timestamp\u0027: 1,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3e21d6ad_af24c587","line":7022,"updated":"2025-04-09 12:30:49.000000000","message":"off-topic: self.object_controller.logger already is self.logger, see setUp() method","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"96529dae6eee5d0bcda15f52ff387d0838b4091d","unresolved":false,"context_lines":[{"line_number":7019,"context_line":"            given_args.extend(args)"},{"line_number":7020,"context_line":""},{"line_number":7021,"context_line":"        self.object_controller.async_update \u003d fake_async_update"},{"line_number":7022,"context_line":"        self.object_controller.logger \u003d self.logger"},{"line_number":7023,"context_line":"        delete_at \u003d time()"},{"line_number":7024,"context_line":"        req_headers \u003d {"},{"line_number":7025,"context_line":"            \u0027X-Timestamp\u0027: 1,"}],"source_content_type":"text/x-python","patch_set":9,"id":"7937b04c_0fde4061","line":7022,"in_reply_to":"3e21d6ad_af24c587","updated":"2025-04-10 15:36:24.000000000","message":"Acknowledged","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b97bb04002b92f8903f2735bc8c25cdd71f67e13","unresolved":true,"context_lines":[{"line_number":7038,"context_line":"                                                req, \u0027sda1\u0027, policy)"},{"line_number":7039,"context_line":"        self.assertEqual({"},{"line_number":7040,"context_line":"            \u0027debug\u0027: [\u0027Labeled statsd mode: disabled (test-object-controller)\u0027]"},{"line_number":7041,"context_line":"        }, self.logger.all_log_lines())"},{"line_number":7042,"context_line":"        self.assertEqual(given_args, [])"},{"line_number":7043,"context_line":""},{"line_number":7044,"context_line":"    def test_delete_at_update_put_with_info_but_empty_host(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"d2e87375_d5707972","line":7041,"updated":"2025-04-09 12:30:49.000000000","message":"this debug line is emitted when the object_controller is constructed. We can avoid it polluting the assertion here by clearing the debug logger after the controller is constructed but before the request is sent (around line 7022), so the test is asserting that the *request* did not cause any log lines.\n\nWe can assert that statsd label mode is logged in test_init.","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"96529dae6eee5d0bcda15f52ff387d0838b4091d","unresolved":false,"context_lines":[{"line_number":7038,"context_line":"                                                req, \u0027sda1\u0027, policy)"},{"line_number":7039,"context_line":"        self.assertEqual({"},{"line_number":7040,"context_line":"            \u0027debug\u0027: [\u0027Labeled statsd mode: disabled (test-object-controller)\u0027]"},{"line_number":7041,"context_line":"        }, self.logger.all_log_lines())"},{"line_number":7042,"context_line":"        self.assertEqual(given_args, [])"},{"line_number":7043,"context_line":""},{"line_number":7044,"context_line":"    def test_delete_at_update_put_with_info_but_empty_host(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"86316cba_68bbfaf6","line":7041,"in_reply_to":"26da4b38_75e6f42e","updated":"2025-04-10 15:36:24.000000000","message":"Thought that we could use assertIn, but didn\u0027t really understand why the original test was asserting on all log lines","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b4910a85dd95406f2b449ac9e3b61371dfccdaef","unresolved":true,"context_lines":[{"line_number":7038,"context_line":"                                                req, \u0027sda1\u0027, policy)"},{"line_number":7039,"context_line":"        self.assertEqual({"},{"line_number":7040,"context_line":"            \u0027debug\u0027: [\u0027Labeled statsd mode: disabled (test-object-controller)\u0027]"},{"line_number":7041,"context_line":"        }, self.logger.all_log_lines())"},{"line_number":7042,"context_line":"        self.assertEqual(given_args, [])"},{"line_number":7043,"context_line":""},{"line_number":7044,"context_line":"    def test_delete_at_update_put_with_info_but_empty_host(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"26da4b38_75e6f42e","line":7041,"in_reply_to":"d2e87375_d5707972","updated":"2025-04-09 15:43:47.000000000","message":"any change could always have *better* tests, but making existing tests harder to reason about by adding unrelated noise should have been a smell.\n\nYan I\u0027m curious if you could outline any possible alternatives you considered and rejected before you decided to change the assertion in `test_delete_at_update_put_with_info_but_missing_host` to require a debug message about the disabled label mode to be emitted?","commit_id":"5286ab2e0004136fd372d2b720804b8c19f50ff3"}]}