)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9ab714fef9eaac3e2da187adda763ffc9efa4df8","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-05-12 15:38:07 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing"},{"line_number":8,"context_line":"from get_logger to get_statsd_client"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"43d01a2a_cd4e1fea","line":8,"updated":"2024-05-13 16:46:47.000000000","message":"the commit message summary should be one line\n\nhttps://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"ab1e9290890d1f07c835acc08d84cb97e94d9498","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-05-12 15:38:07 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing"},{"line_number":8,"context_line":"from get_logger to get_statsd_client"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"f3740451_0f34d71a","line":8,"in_reply_to":"43d01a2a_cd4e1fea","updated":"2024-05-13 19:36:04.000000000","message":"Acknowledged","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a9862ff339f766839f8943776eb9ab7170d60bfd","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-05-14 16:02:12 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"77b02d91_b541791f","line":7,"updated":"2024-05-15 14:11:55.000000000","message":"The patch actually moves the config parsing into StatsdClient - get_statsd_client doesn\u0027t do much itself.\n\nThe issue I have with this is that the config expressed in legacy terms (log_statsd_host etc) doesn\u0027t make sense to pass into StatsdClient. This patch on its own makes a perfectly reasonable  StatsdClient constructor interface worse (IMHO).\n\nIf the end-goal is to have ``StatsdClient.__init__`` accept a conf dict with sensible keys like ``\u0027host\u0027``, ``\u0027port\u0027`` etc., then I think there\u0027s two ways forward:\n\n* make ``get_statsd_client`` extract ``log_statsd_*`` keys from the conf dict and pass them to the *unchanged* StatsdClient constructor (so more like what the commit message suggests). Later, StatsdClient constructor could change to accept a sanitised dict ``{\u0027host\u0027: \u0027foo\u0027, \u0027port\u0027: 1234, etc.}``.\n\nor: \n\n* make get_statsd_client translate the conf keys to  a ``{\u0027host\u0027: \u0027foo\u0027, \u0027port\u0027: 1234, etc.}`` dict and change the StatsdClient constructor to accept this dict, all in this patch.","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-05-14 16:02:12 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7b9e0824_1f4c2cb1","line":7,"in_reply_to":"77b02d91_b541791f","updated":"2024-05-15 15:53:35.000000000","message":"I think there\u0027s little value in changing the StatsdClient constructor to accept a \"conf\" dict if the *keys* in the dict aren\u0027t the same as the keys you\u0027d get from a config.\n\nI\u0027m mostly fine with the idea of kwargs in the statsdclient config, except we\u0027re going to be adding dynamic \"user label\" config options and maybe other things - so it might make sense if they StatsdClient just had access to the whole config dict including __file__.\n\nI agree the log_statsd_ is a weird prefix compared to the desired statsd_ prefix, but I would expect the config keys to have *some* kind of prefix and don\u0027t see anything wrong with a class taking a conf as a constructor as a general pattern that we use in lots of places for middleware/deaemons/servers/diskfile-manager/etc","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-05-14 16:02:12 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"d0acecbf_79c3eb1e","line":7,"in_reply_to":"7b9e0824_1f4c2cb1","updated":"2024-06-18 10:54:15.000000000","message":"Let\u0027s not lose sight of this comment, particularly Clay\u0027s point about adding dynamic \"user label\" config options in the future.\n\nMaybe we should see how that works out before merging this patch?\n\nI can imagine passing the conf (or a filtered version) as kwarg to the constructor to support arbitrary dynamic options, but I\u0027d need to see code to know for sure.","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e649f6693c31d259f0bc41bb21b282736ed28d4","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-05-14 16:02:12 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ea3fd93c_28fafacd","line":7,"in_reply_to":"d0acecbf_79c3eb1e","updated":"2024-06-24 11:09:47.000000000","message":"I looked ahead to https://review.opendev.org/c/openstack/swift/+/909882/21/swift/common/statsd_client.py which adds, amongst other things, support for user defined config (``default_labels``). These are parsed into a dict that can be passed to the StasdClient constructor so that looks fine.","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fa65c464e68e35e20057f4914f531271ef9fc9a","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-06-10 13:18:10 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Co-author: yanxiao@nvidia.com"},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"6c7d72b1_57fc92fb","line":7,"range":{"start_line":7,"start_character":52,"end_line":7,"end_character":69},"updated":"2024-06-12 09:10:46.000000000","message":"the conf parsing is done in the StatsdClient constructor, not get_statsd_client","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"6d78fe10898647be383de4ac1343de289d2871c6","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-06-10 13:18:10 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Co-author: yanxiao@nvidia.com"},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3969fcc6_3f0dbe4a","line":7,"range":{"start_line":7,"start_character":52,"end_line":7,"end_character":69},"in_reply_to":"6c7d72b1_57fc92fb","updated":"2024-06-13 00:07:51.000000000","message":"Agree, changing the commit message","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fa65c464e68e35e20057f4914f531271ef9fc9a","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-06-10 13:18:10 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Co-author: yanxiao@nvidia.com"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"1eb4af5c_78af4c8c","line":8,"updated":"2024-06-12 09:10:46.000000000","message":"nit: it wouldn\u0027t hurt to explain the motivation for the change (it\u0027s perhaps obvious in this case, particularly for those that have lived with this for weeks, but it\u0027s a good habit to practice). Something like:\n\n```\nIt doesn\u0027t make sense for StatsdClient config options to be parsed in the logs module. This patch moves the parsing to the StatsdClient constructor to better separate the concerns of logging and metrics reporting.\n\nThe get_statsd_client function is added to allow \u003cinsert reason\u003e.\n```","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"6d78fe10898647be383de4ac1343de289d2871c6","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-06-10 13:18:10 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move StatsdClient config parsing from get_logger to get_statsd_client"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Co-author: yanxiao@nvidia.com"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"e5fc1997_13be967b","line":8,"in_reply_to":"1eb4af5c_78af4c8c","updated":"2024-06-13 00:07:51.000000000","message":"Agree, I\u0027ll add some description","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98c39b091532bc877dd32ef3add2cf29a5098e98","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-06-16 21:00:27 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"get statsd shim, divide responsibilities differently"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Make get_statsd_client responsible for config parsing. This also gives"},{"line_number":10,"context_line":"  us a place where we could hope to deprecate the \"log_\" prefix."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"fe0d3142_1c425d8b","line":7,"updated":"2024-06-18 20:24:13.000000000","message":"I wonder if the whole commit message might be better as something like\n\n```\nAdd get_statsd_client function\n\nIt takes a config dict and some caller-specific options, similar to\nget_logger. Use this in get_logger, so our logging module doesn\u0027t\nneed to know anything about statsd config options.\n\nDrive-by: allow statsd config options to be specified without the\n\"log_\" prefix.\n```","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Shreeya Deshpande \u003cshreeyad@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-06-16 21:00:27 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"get statsd shim, divide responsibilities differently"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Make get_statsd_client responsible for config parsing. This also gives"},{"line_number":10,"context_line":"  us a place where we could hope to deprecate the \"log_\" prefix."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"4e1bd119_f3b7b408","line":7,"in_reply_to":"fe0d3142_1c425d8b","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Make get_statsd_client responsible for config parsing. This also gives"},{"line_number":10,"context_line":"  us a place where we could hope to deprecate the \"log_\" prefix."},{"line_number":11,"context_line":"- Get rid of StatsdClientConf, since the parsing is happening elsewhere."},{"line_number":12,"context_line":"- Roll back StatsdClient signature changes. get_statsd_client will take"},{"line_number":13,"context_line":"  a config dict, similar to get_logger, while StatsdClient continues to"},{"line_number":14,"context_line":"  have a self-explanatory signature."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"0a95d912_f022d66f","line":11,"range":{"start_line":11,"start_character":2,"end_line":11,"end_character":29},"updated":"2024-06-18 10:54:15.000000000","message":"the commit message needs to describe the change w.r.t. master but I think it is describing changes relative to previous patchsets. On master there has never been StatsdClientConf.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Make get_statsd_client responsible for config parsing. This also gives"},{"line_number":10,"context_line":"  us a place where we could hope to deprecate the \"log_\" prefix."},{"line_number":11,"context_line":"- Get rid of StatsdClientConf, since the parsing is happening elsewhere."},{"line_number":12,"context_line":"- Roll back StatsdClient signature changes. get_statsd_client will take"},{"line_number":13,"context_line":"  a config dict, similar to get_logger, while StatsdClient continues to"},{"line_number":14,"context_line":"  have a self-explanatory signature."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"a1b38a92_9aee9029","line":11,"range":{"start_line":11,"start_character":2,"end_line":11,"end_character":29},"in_reply_to":"0a95d912_f022d66f","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":9,"context_line":"- Make get_statsd_client responsible for config parsing. This also gives"},{"line_number":10,"context_line":"  us a place where we could hope to deprecate the \"log_\" prefix."},{"line_number":11,"context_line":"- Get rid of StatsdClientConf, since the parsing is happening elsewhere."},{"line_number":12,"context_line":"- Roll back StatsdClient signature changes. get_statsd_client will take"},{"line_number":13,"context_line":"  a config dict, similar to get_logger, while StatsdClient continues to"},{"line_number":14,"context_line":"  have a self-explanatory signature."},{"line_number":15,"context_line":"- get_statsd_client provides a thin wrapper around StatsdClient constructor"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"477479d8_fc90adf3","line":12,"range":{"start_line":12,"start_character":2,"end_line":12,"end_character":42},"updated":"2024-06-18 10:54:15.000000000","message":"On the contrary, there are no changes to the constructor.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":9,"context_line":"- Make get_statsd_client responsible for config parsing. This also gives"},{"line_number":10,"context_line":"  us a place where we could hope to deprecate the \"log_\" prefix."},{"line_number":11,"context_line":"- Get rid of StatsdClientConf, since the parsing is happening elsewhere."},{"line_number":12,"context_line":"- Roll back StatsdClient signature changes. get_statsd_client will take"},{"line_number":13,"context_line":"  a config dict, similar to get_logger, while StatsdClient continues to"},{"line_number":14,"context_line":"  have a self-explanatory signature."},{"line_number":15,"context_line":"- get_statsd_client provides a thin wrapper around StatsdClient constructor"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"8c8a6f93_3c676bcb","line":12,"range":{"start_line":12,"start_character":2,"end_line":12,"end_character":42},"in_reply_to":"477479d8_fc90adf3","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":14,"context_line":"  have a self-explanatory signature."},{"line_number":15,"context_line":"- get_statsd_client provides a thin wrapper around StatsdClient constructor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Co-author: yanxiao@nvidia.com"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"bb37f4b5_b09a96cd","line":17,"updated":"2024-06-18 10:54:15.000000000","message":"I think this should be ``Co-Authored-By:``","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":14,"context_line":"  have a self-explanatory signature."},{"line_number":15,"context_line":"- get_statsd_client provides a thin wrapper around StatsdClient constructor"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Co-author: yanxiao@nvidia.com"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I5ae2cc5c257fb8d7eab885977d9d9cf602224ec7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"ec8a6b24_221ffe7c","line":17,"in_reply_to":"bb37f4b5_b09a96cd","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4042e3a22b96885b9769834f78923ea720159ed0","unresolved":true,"context_lines":[{"line_number":11,"context_line":"need to know anything about statsd config options."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Drive-by: allow statsd config options to be specified without the"},{"line_number":14,"context_line":"\"log_\" prefix."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Co-Authored-By: yanxiao@nvidia.com"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"70d565c9_e276f503","line":14,"updated":"2024-06-19 11:22:52.000000000","message":"OK, but I think we may have a bit more work to do before we can actually use the modern option names in practice, because proxy_logging is still expecting the legacy names:\n\nhttps://github.com/openstack/swift/blob/3d5a97f76bbcc0131814b2e086605aad47924e6f/swift/common/middleware/proxy_logging.py#L141-L149\n\nWe probably just need to add the new names to the list of options  copied into ``access_log_conf`` and pass them all to get_logger/get_statsd_client.","commit_id":"7b7d3f536037fe4acd09baf18db5c3658cabdb85"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e649f6693c31d259f0bc41bb21b282736ed28d4","unresolved":false,"context_lines":[{"line_number":11,"context_line":"need to know anything about statsd config options."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Drive-by: allow statsd config options to be specified without the"},{"line_number":14,"context_line":"\"log_\" prefix."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Co-Authored-By: yanxiao@nvidia.com"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"8f84fb7b_4b2dd096","line":14,"in_reply_to":"70d565c9_e276f503","updated":"2024-06-24 11:09:47.000000000","message":"Moved to https://review.opendev.org/c/openstack/swift/+/922518/1?usp\u003drelated-change","commit_id":"7b7d3f536037fe4acd09baf18db5c3658cabdb85"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4eba1c6b_b758a2e4","updated":"2024-05-14 11:53:14.000000000","message":"I ran into some issues with the tests - we may want to check back on the way socket is imported since the refactor of utils.\n\nI don\u0027t think there is any direct call to get_statsd_client() or StatsdClient() in the test_statsd module.\n\nI don\u0027t like that before I could construct a ``StatsdClient(myhost, myport, ...)`` but now I\u0027d have to type ``StatsdClient({\u0027log_statsd_host\u0027: myhost, \u0027log_statsd_port\u0027: myport})``. This takes the legacy \u0027statsd-is-entwined-with-logger\u0027 confusion and embeds it deeper into the StatsdClient. I\u0027d prefer to see the ``log_statsd...`` prefixes eliminated from the StatsdClient interface.","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"39f8be5b4bcadd7f72c0ec181be95f120790157b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"65dc6088_24ad75da","updated":"2024-05-13 16:44:27.000000000","message":"I think this is probably fine","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9ab714fef9eaac3e2da187adda763ffc9efa4df8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c5f9b47b_8a445575","updated":"2024-05-13 16:46:47.000000000","message":"maybe fix the git commit summary to fit on one line.\n\nif you\u0027re going to do another rev, try and see if you can get rid of the FakeConfig thing in test_statsd","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19819b17a8e67e552b10bf458249baa0040489a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7d666b84_77f3c521","updated":"2024-05-13 16:36:47.000000000","message":"recheck","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"01eba704_36b69d81","updated":"2024-05-15 15:53:35.000000000","message":"I don\u0027t think moving config parsing into the class is making things \"worse\" than master; or at least I think it\u0027s a step in the correct direction to try and get statsd config parsing out of get_logger even if config options still have dumb (but backwards compatible) names.\n\nI would like to see this patch go a little further to try and isolate all of the statsd option param handling into the statsd_client module.\n\nI noticed some other things we should clean-up regardless of the progress on this patch; you may want to rebase on it:\n\n919752: make statsd_client more explicit | https://review.opendev.org/c/openstack/swift/+/919752","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7eb360ef_c1c923fc","updated":"2024-06-04 00:25:10.000000000","message":"This is looking reasonably good! I certainly wouldn\u0027t be above carrying -- though I want to get the `get_statsd_client` API cleaned up some more before merging (and address some other little cleanups).\n\nWe\u0027re getting there, though! Less and less statsd stuff in logs.py 😊","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"ea873f1fe7900db3414085587be900f827eddebf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"086cfc5b_5bcf4027","updated":"2024-06-10 16:12:04.000000000","message":"recheck","commit_id":"35d91a354eb9a13e923626acd6f9237913a0e073"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fa65c464e68e35e20057f4914f531271ef9fc9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"8a2604d5_5f0d5489","updated":"2024-06-12 09:10:46.000000000","message":"-1 primarily because get_statsd_client either needs to be removed or it must be tested.","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52f588a55c052191f36366a36b43f3db530814ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"717d4a50_5db9504d","updated":"2024-06-12 20:23:43.000000000","message":"I think everyone agrees this change needs more work to merge and also that\u0027s in pretty good shape and probably mostly functionally complete - good foundation for us to build on and work towards proxy-logging labeled metrics review","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"2d1c3f8818f45afec0ada453515dd5d7d5aff26c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"5fc05e21_b6835d10","updated":"2024-06-10 22:50:00.000000000","message":"recheck","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"b1dbcf21_382484da","updated":"2024-06-18 10:54:15.000000000","message":"I like this approach, but I am mindful of comments re future extensions, so I\u0027d like to see how they work out before we merge this.\n\nI\u0027m not sure all the changes are necessary, and I\u0027m concerned some of the test changes enable statsd where they were previously mocked or disabled.\n\nSuggestion for more concise parsing of modern vs legacy here https://review.opendev.org/c/openstack/swift/+/922199 . There\u0027s also more unit testing in that patch.\n\nSuggestion for warning re deprecation here https://review.opendev.org/c/openstack/swift/+/922200","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c29ddf50021454cd87b25e12ab7423b4aa2971d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"250fcd8c_ab82dc56","updated":"2024-06-18 20:29:34.000000000","message":"Oh, right! And since we\u0027re switching to `statsd_*` options instead of `log_statsd_*` options, we should update sample configs...","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4042e3a22b96885b9769834f78923ea720159ed0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"b1eaf43e_e916e69c","in_reply_to":"250fcd8c_ab82dc56","updated":"2024-06-19 11:22:52.000000000","message":"bump.\n\nthis didn\u0027t happen. \n\nlog_statsd_* is documented in quite a few of the sample conf files. Probably worth agreeing new wording before changing them all.\n\nAlso, the man pages and admin_guide.rst will need updating.\n\nMaybe:\n```\n# StatsD config options.\n# For backwards compatibility the following options may also be configured using\n# the same option name with a \u0027log_\u0027 prefix: \n#     statsd_host, statsd_port, statsd_default_sample_rate,\n#     statsd_sample_rate_factor, statsd_metric_prefix\n# For example, \u0027log_statsd_host\u0027 is supported in the absence of \u0027statsd_host\u0027.\n# StatsD metrics are enabled if statsd_host is configured.\n# statsd_host \u003d\n# statsd_port \u003d 8125\n# statsd_default_sample_rate \u003d 1.0\n# statsd_sample_rate_factor \u003d 1.0\n# statsd_metric_prefix \u003d\n```","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9332aab07e3c18d2fe9555d991c465edb350f62e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"c6c718c1_d87e426d","updated":"2024-06-19 11:34:27.000000000","message":"-1 because proxy_logging won\u0027t read the new options","commit_id":"7b7d3f536037fe4acd09baf18db5c3658cabdb85"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4042e3a22b96885b9769834f78923ea720159ed0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"29bce146_2f9c2887","updated":"2024-06-19 11:22:52.000000000","message":"Patch looks good but we need to bring the docs in line.\n\nFWIW we could break the patch up and merge the get_statsd_client part ahead of the log_ deprecation part.","commit_id":"7b7d3f536037fe4acd09baf18db5c3658cabdb85"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e649f6693c31d259f0bc41bb21b282736ed28d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"4eeafd45_5d2c2eeb","updated":"2024-06-24 11:09:47.000000000","message":"I\u0027m ok with this merging:\n\n1. It is focussed and makes progress away from the conflation of logging and metrics.\n2. ``get_statsd_client`` is *the* way to get a ``StatsdClient`` from conf, leaving us freedom to change the implementation behind that if we want/need. There is no ambiguity as to whether to call the function or the class constructor.\n3. We have a plan for deprecating ``log_`` prefix in a follow on.\n4. It enables progress towards labeled metrics.\n5. It enables further separation of logging and metrics and the deprecation of ``get_logger()`` and the legacy \u0027logger is also a stats client\u0027 pattern.","commit_id":"0c58afc7ee23fd5e5cca19318e3f055dcc79e021"}],"swift/common/statsd_client.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Get an instance of StatsdClient using config settings."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    **Log config and defaults**::"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5e707a8e_978714ba","line":29,"updated":"2024-05-14 11:53:14.000000000","message":"so this is primarily doing a LBYL check for the host conf option.\n\nStatsdClient could instead raise a ValueError if host is Falsey.","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"7642bab0ad28e0e992c54fe84030d6d47e076ce4","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Get an instance of StatsdClient using config settings."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    **Log config and defaults**::"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ef858910_c36ffa74","line":29,"in_reply_to":"5e707a8e_978714ba","updated":"2024-05-14 23:42:21.000000000","message":"Acknowledged","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    statsd_host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":45,"context_line":"    statsd_client \u003d None"},{"line_number":46,"context_line":"    if statsd_host:"},{"line_number":47,"context_line":"        statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"            conf,"},{"line_number":49,"context_line":"            tail_prefix\u003dtail_prefix, logger\u003dlogger)"}],"source_content_type":"text/x-python","patch_set":1,"id":"4ac872a5_3ec33536","line":46,"updated":"2024-05-14 11:53:14.000000000","message":"how about dropping the legacy prefix here:\n\n```\nconf \u003d {k[10:]: v for k, v in conf.items() if k.startswith(\u0027log_statsd_\u0027)}\n```\n\nAlso, I\u0027m not convinced that this helper function belons here rather than in logs.py - particularly with the change I suggest, it is specifically a helper for legacy get_logger, and not really useful elsewhere (why not just call StatsdClient() with a modern conf dict?)","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"7642bab0ad28e0e992c54fe84030d6d47e076ce4","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    statsd_host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":45,"context_line":"    statsd_client \u003d None"},{"line_number":46,"context_line":"    if statsd_host:"},{"line_number":47,"context_line":"        statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"            conf,"},{"line_number":49,"context_line":"            tail_prefix\u003dtail_prefix, logger\u003dlogger)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9520310c_caa72598","line":46,"in_reply_to":"4ac872a5_3ec33536","updated":"2024-05-14 23:42:21.000000000","message":"https://jirasw.nvidia.com/browse/NSVISCS-8353","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":50,"context_line":"    return statsd_client"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class StatsdClientConf(object):"},{"line_number":54,"context_line":"    def __init__(self, conf\u003dNone, tail_prefix\u003d\u0027\u0027):"},{"line_number":55,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"c0791ac5_42077456","line":53,"updated":"2024-05-14 11:53:14.000000000","message":"I don\u0027t understand why StatsdClient conf options have a ``log_statsd_`` prefix. I understand why they *did* but not why we\u0027d promote that as the future pattern too. If we add options to StatsdClient, are they also going to start with \"log_statsd_\"?\n\nPerhaps the get_statsd_client method could translate the legacy conf to a \"modern\" set of options that isn\u0027t muddled up with logging. That way we can support the old confs.","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"7642bab0ad28e0e992c54fe84030d6d47e076ce4","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    return statsd_client"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class StatsdClientConf(object):"},{"line_number":54,"context_line":"    def __init__(self, conf\u003dNone, tail_prefix\u003d\u0027\u0027):"},{"line_number":55,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"70aa4db4_051712c1","line":53,"in_reply_to":"c0791ac5_42077456","updated":"2024-05-14 23:42:21.000000000","message":"https://jirasw.nvidia.com/browse/NSVISCS-8353","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":54,"context_line":"    def __init__(self, conf\u003dNone, tail_prefix\u003d\u0027\u0027):"},{"line_number":55,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":58,"context_line":"        self.port \u003d int(conf.get(\u0027log_statsd_port\u0027, 8125))"},{"line_number":59,"context_line":"        self.base_prefix \u003d conf.get(\u0027log_statsd_metric_prefix\u0027, \u0027\u0027)"},{"line_number":60,"context_line":"        self.default_sample_rate \u003d float(conf.get("}],"source_content_type":"text/x-python","patch_set":1,"id":"91b76fd2_da4bc74a","line":57,"updated":"2024-05-14 11:53:14.000000000","message":"It\u0027s not ok for self.host to be None, so this ought to raise an exception if \u0027log_statsd_host\u0027 isn\u0027t in the conf dict. Maybe the exception comes out of _determine_sock_family?","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a9862ff339f766839f8943776eb9ab7170d60bfd","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    def __init__(self, conf\u003dNone, tail_prefix\u003d\u0027\u0027):"},{"line_number":55,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":58,"context_line":"        self.port \u003d int(conf.get(\u0027log_statsd_port\u0027, 8125))"},{"line_number":59,"context_line":"        self.base_prefix \u003d conf.get(\u0027log_statsd_metric_prefix\u0027, \u0027\u0027)"},{"line_number":60,"context_line":"        self.default_sample_rate \u003d float(conf.get("}],"source_content_type":"text/x-python","patch_set":1,"id":"51963b5c_bda57aaf","line":57,"in_reply_to":"91b76fd2_da4bc74a","updated":"2024-05-15 14:11:55.000000000","message":"Done","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"39f8be5b4bcadd7f72c0ec181be95f120790157b","unresolved":true,"context_lines":[{"line_number":61,"context_line":"            \u0027log_statsd_default_sample_rate\u0027, 1))"},{"line_number":62,"context_line":"        self.sample_rate_factor \u003d float(conf.get("},{"line_number":63,"context_line":"            \u0027log_statsd_sample_rate_factor\u0027, 1))"},{"line_number":64,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class StatsdClient(StatsdClientConf):"}],"source_content_type":"text/x-python","patch_set":1,"id":"931aa9dc_d9f55618","line":64,"updated":"2024-05-13 16:44:27.000000000","message":"this attribute isn\u0027t really part of the \"config\" - maybe it shouldn\u0027t be part of this abstraction and just a property of the StatsdClient itself.","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"ab1e9290890d1f07c835acc08d84cb97e94d9498","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            \u0027log_statsd_default_sample_rate\u0027, 1))"},{"line_number":62,"context_line":"        self.sample_rate_factor \u003d float(conf.get("},{"line_number":63,"context_line":"            \u0027log_statsd_sample_rate_factor\u0027, 1))"},{"line_number":64,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class StatsdClient(StatsdClientConf):"}],"source_content_type":"text/x-python","patch_set":1,"id":"99a4050d_942b481b","line":64,"in_reply_to":"931aa9dc_d9f55618","updated":"2024-05-13 19:36:04.000000000","message":"Acknowledged","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            \u0027log_statsd_default_sample_rate\u0027, 1))"},{"line_number":62,"context_line":"        self.sample_rate_factor \u003d float(conf.get("},{"line_number":63,"context_line":"            \u0027log_statsd_sample_rate_factor\u0027, 1))"},{"line_number":64,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class StatsdClient(StatsdClientConf):"}],"source_content_type":"text/x-python","patch_set":1,"id":"e0d7266c_f1803952","line":64,"in_reply_to":"99a4050d_942b481b","updated":"2024-05-14 11:53:14.000000000","message":"I don\u0027t think it is used other than to pass into _set_prefix() in the constructor, so it doesn\u0027t need to be an attribute.","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":41,"context_line":"    :return: an instance of ``StatsdClient``, or None if ``log_statsd_host``"},{"line_number":42,"context_line":"             is not specified in ``conf``"},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    statsd_host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":45,"context_line":"    statsd_client \u003d None"},{"line_number":46,"context_line":"    if statsd_host:"},{"line_number":47,"context_line":"        statsd_client \u003d StatsdClient("}],"source_content_type":"text/x-python","patch_set":2,"id":"70b13c2f_1b0635ae","line":44,"updated":"2024-05-15 15:53:35.000000000","message":"I don\u0027t think we should have pre-parsing going on in here if we\u0027re going to move the majority of config parsing into the constructor.\n\nEither just pass the conf through and catch the error - or do the parsing here and pass the named variables to the constructor.","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    :return: an instance of ``StatsdClient``, or None if ``log_statsd_host``"},{"line_number":42,"context_line":"             is not specified in ``conf``"},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    statsd_host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":45,"context_line":"    statsd_client \u003d None"},{"line_number":46,"context_line":"    if statsd_host:"},{"line_number":47,"context_line":"        statsd_client \u003d StatsdClient("}],"source_content_type":"text/x-python","patch_set":2,"id":"70d405f1_295d6051","line":44,"in_reply_to":"70b13c2f_1b0635ae","updated":"2024-05-16 22:20:50.000000000","message":"Acknowledged","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a9862ff339f766839f8943776eb9ab7170d60bfd","unresolved":true,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class StatsdClientConf(object):"},{"line_number":54,"context_line":"    def __init__(self, conf\u003dNone):"},{"line_number":55,"context_line":"        if conf:"},{"line_number":56,"context_line":"            conf \u003d conf"},{"line_number":57,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"4966456f_dea97f8d","line":54,"range":{"start_line":54,"start_character":23,"end_line":54,"end_character":32},"updated":"2024-05-15 14:11:55.000000000","message":"this shouldn\u0027t allow a default of None when that is actually an error\n\n```\n  def __init__(self, conf)\n```","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class StatsdClientConf(object):"},{"line_number":54,"context_line":"    def __init__(self, conf\u003dNone):"},{"line_number":55,"context_line":"        if conf:"},{"line_number":56,"context_line":"            conf \u003d conf"},{"line_number":57,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"668e797f_c9301e67","line":54,"range":{"start_line":54,"start_character":23,"end_line":54,"end_character":32},"in_reply_to":"4966456f_dea97f8d","updated":"2024-05-15 15:53:35.000000000","message":"100%","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class StatsdClientConf(object):"},{"line_number":54,"context_line":"    def __init__(self, conf\u003dNone):"},{"line_number":55,"context_line":"        if conf:"},{"line_number":56,"context_line":"            conf \u003d conf"},{"line_number":57,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"03479734_54e25661","line":54,"range":{"start_line":54,"start_character":23,"end_line":54,"end_character":32},"in_reply_to":"668e797f_c9301e67","updated":"2024-05-16 22:20:50.000000000","message":"Acknowledged","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a9862ff339f766839f8943776eb9ab7170d60bfd","unresolved":true,"context_lines":[{"line_number":56,"context_line":"            conf \u003d conf"},{"line_number":57,"context_line":"        else:"},{"line_number":58,"context_line":"            raise ValueError(\"Statsd Host does not exist\")"},{"line_number":59,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":60,"context_line":"        self.port \u003d int(conf.get(\u0027log_statsd_port\u0027, 8125))"},{"line_number":61,"context_line":"        self.base_prefix \u003d conf.get(\u0027log_statsd_metric_prefix\u0027, \u0027\u0027)"},{"line_number":62,"context_line":"        self.default_sample_rate \u003d float(conf.get("}],"source_content_type":"text/x-python","patch_set":2,"id":"40496af2_1968479b","line":59,"updated":"2024-05-15 14:11:55.000000000","message":"this could be simpler:\n\n```\n    self.host \u003d conf[\u0027log_statsd_host\u0027]\n```\n\nthis will raise a KeyError if ``log_statsd_host`` is missing, or a TypeError if conf is None","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            conf \u003d conf"},{"line_number":57,"context_line":"        else:"},{"line_number":58,"context_line":"            raise ValueError(\"Statsd Host does not exist\")"},{"line_number":59,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":60,"context_line":"        self.port \u003d int(conf.get(\u0027log_statsd_port\u0027, 8125))"},{"line_number":61,"context_line":"        self.base_prefix \u003d conf.get(\u0027log_statsd_metric_prefix\u0027, \u0027\u0027)"},{"line_number":62,"context_line":"        self.default_sample_rate \u003d float(conf.get("}],"source_content_type":"text/x-python","patch_set":2,"id":"f7e5b074_01f4eba7","line":59,"in_reply_to":"40496af2_1968479b","updated":"2024-05-16 22:20:50.000000000","message":"Acknowledged","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":68,"context_line":"class StatsdClient(StatsdClientConf):"},{"line_number":69,"context_line":"    def __init__(self, conf, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":70,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":71,"context_line":"        self.random \u003d random"},{"line_number":72,"context_line":"        self.logger \u003d logger"},{"line_number":73,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":74,"context_line":"        self._set_prefix(self.tail_prefix)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bb1cba13_333242f6","line":71,"updated":"2024-05-15 15:53:35.000000000","message":"what a surprising thing - I guess it makes monkey patching in some tests easier.","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":68,"context_line":"class StatsdClient(StatsdClientConf):"},{"line_number":69,"context_line":"    def __init__(self, conf, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":70,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":71,"context_line":"        self.random \u003d random"},{"line_number":72,"context_line":"        self.logger \u003d logger"},{"line_number":73,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":74,"context_line":"        self._set_prefix(self.tail_prefix)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9d7522f8_e1dc76e4","line":71,"in_reply_to":"bb1cba13_333242f6","updated":"2024-05-16 22:20:50.000000000","message":"Done","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a9862ff339f766839f8943776eb9ab7170d60bfd","unresolved":true,"context_lines":[{"line_number":70,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":71,"context_line":"        self.random \u003d random"},{"line_number":72,"context_line":"        self.logger \u003d logger"},{"line_number":73,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":74,"context_line":"        self._set_prefix(self.tail_prefix)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # Determine if host is IPv4 or IPv6"}],"source_content_type":"text/x-python","patch_set":2,"id":"b2a52e89_9d631251","line":73,"updated":"2024-05-15 14:11:55.000000000","message":"it isn\u0027t necessary to add this attribute","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":70,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":71,"context_line":"        self.random \u003d random"},{"line_number":72,"context_line":"        self.logger \u003d logger"},{"line_number":73,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":74,"context_line":"        self._set_prefix(self.tail_prefix)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # Determine if host is IPv4 or IPv6"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff32ba28_32427a7d","line":73,"in_reply_to":"b2a52e89_9d631251","updated":"2024-05-15 15:53:35.000000000","message":"yeah sweet let\u0027s dump it!","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":71,"context_line":"        self.random \u003d random"},{"line_number":72,"context_line":"        self.logger \u003d logger"},{"line_number":73,"context_line":"        self.tail_prefix \u003d tail_prefix"},{"line_number":74,"context_line":"        self._set_prefix(self.tail_prefix)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # Determine if host is IPv4 or IPv6"}],"source_content_type":"text/x-python","patch_set":2,"id":"b67e5194_205d6b62","line":73,"in_reply_to":"ff32ba28_32427a7d","updated":"2024-05-16 22:20:50.000000000","message":"Done","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":145,"context_line":"        \"\"\""},{"line_number":146,"context_line":"        This method is deprecated; use the ``tail_prefix`` argument of the"},{"line_number":147,"context_line":"        constructor when instantiating the class instead."},{"line_number":148,"context_line":"        \"\"\""},{"line_number":149,"context_line":"        warnings.warn("},{"line_number":150,"context_line":"            \u0027set_prefix() is deprecated; use the ``tail_prefix`` argument of \u0027"},{"line_number":151,"context_line":"            \u0027the constructor when instantiating the class instead.\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"e0816cd5_7771ca18","line":148,"updated":"2024-05-15 15:53:35.000000000","message":"good advice!","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        \"\"\""},{"line_number":146,"context_line":"        This method is deprecated; use the ``tail_prefix`` argument of the"},{"line_number":147,"context_line":"        constructor when instantiating the class instead."},{"line_number":148,"context_line":"        \"\"\""},{"line_number":149,"context_line":"        warnings.warn("},{"line_number":150,"context_line":"            \u0027set_prefix() is deprecated; use the ``tail_prefix`` argument of \u0027"},{"line_number":151,"context_line":"            \u0027the constructor when instantiating the class instead.\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c373651_ffb9d79b","line":148,"in_reply_to":"e0816cd5_7771ca18","updated":"2024-05-16 22:20:50.000000000","message":"Acknowledged","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone, name\u003dNone):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Get an instance of StatsdClient using config settings."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7c2c4d4d_0e5c692e","line":27,"range":{"start_line":27,"start_character":49,"end_line":27,"end_character":55},"updated":"2024-06-04 00:25:10.000000000","message":"Should get added to docstring.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone, name\u003dNone):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Get an instance of StatsdClient using config settings."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"22a747ae_389dbd28","line":27,"range":{"start_line":27,"start_character":49,"end_line":27,"end_character":55},"in_reply_to":"7c2c4d4d_0e5c692e","updated":"2024-06-04 22:36:08.000000000","message":"Acknowledged","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":42,"context_line":"             is not specified in ``conf``"},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    if tail_prefix is None:"},{"line_number":45,"context_line":"        tail_prefix \u003d name"},{"line_number":46,"context_line":"    # statsd_client \u003d None"},{"line_number":47,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"        conf,"}],"source_content_type":"text/x-python","patch_set":5,"id":"d406b1fe_4f3449a4","line":45,"updated":"2024-06-04 00:25:10.000000000","message":"So this is the only place we use `name`; we don\u0027t really offer any explanation for what `name` means, or how it\u0027s used; and *if* we use it, it completely replaces `tail_prefix`. I feel like this belongs in the caller.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dc8e16a8823fb62bc4c4e349ecaae2b5e16825e9","unresolved":true,"context_lines":[{"line_number":42,"context_line":"             is not specified in ``conf``"},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    if tail_prefix is None:"},{"line_number":45,"context_line":"        tail_prefix \u003d name"},{"line_number":46,"context_line":"    # statsd_client \u003d None"},{"line_number":47,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"        conf,"}],"source_content_type":"text/x-python","patch_set":5,"id":"2f36ee16_7f509eff","line":45,"in_reply_to":"07e99f55_4a6646d8","updated":"2024-06-07 20:12:54.000000000","message":"Oh, I see now! You already went for that sort of a change in https://review.opendev.org/c/openstack/swift/+/915483 ! Could we just bring that into this patch?","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"ea873f1fe7900db3414085587be900f827eddebf","unresolved":false,"context_lines":[{"line_number":42,"context_line":"             is not specified in ``conf``"},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    if tail_prefix is None:"},{"line_number":45,"context_line":"        tail_prefix \u003d name"},{"line_number":46,"context_line":"    # statsd_client \u003d None"},{"line_number":47,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"        conf,"}],"source_content_type":"text/x-python","patch_set":5,"id":"df80832b_2f525f7a","line":45,"in_reply_to":"2f36ee16_7f509eff","updated":"2024-06-10 16:12:04.000000000","message":"Okayy, got it. I\u0027ll get the same change in this patch too!\nAlthough in the next patch this block would go from logs.py to utils!","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5d03fd3840704bbb1946b7e5bcb9fa170b166347","unresolved":true,"context_lines":[{"line_number":42,"context_line":"             is not specified in ``conf``"},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    if tail_prefix is None:"},{"line_number":45,"context_line":"        tail_prefix \u003d name"},{"line_number":46,"context_line":"    # statsd_client \u003d None"},{"line_number":47,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"        conf,"}],"source_content_type":"text/x-python","patch_set":5,"id":"07e99f55_4a6646d8","line":45,"in_reply_to":"46cafe4e_91307e71","updated":"2024-06-07 20:07:45.000000000","message":"Maybe I should talk with Clay about it. I was thinking something like\n```\ndiff --git a/swift/common/statsd_client.py b/swift/common/statsd_client.py\nindex 1db7c3bbd..7d2a3edaa 100644\n--- a/swift/common/statsd_client.py\n+++ b/swift/common/statsd_client.py\n@@ -24,8 +24,7 @@ from eventlet.green import socket\n import six\n \n \n-def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone,\n-                      default_tail_prefix\u003dNone):\n+def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):\n     \"\"\"\n     Get an instance of StatsdClient using config settings.\n \n@@ -40,13 +39,9 @@ def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone,\n     :param conf: Configuration dict to read settings from\n     :param tail_prefix: tail prefix to pass to statsd client\n     :param logger: stdlib logger instance used by statsd client for logging\n-    :param default_tail_prefix: default value for tail prefix if not specified,\n-            usually from ``log_name` in config\n     :return: an instance of ``StatsdClient``, or None if ``log_statsd_host``\n              is not specified in ``conf``\n     \"\"\"\n-    if tail_prefix is None:\n-        tail_prefix \u003d default_tail_prefix\n     statsd_client \u003d StatsdClient(\n         conf,\n         tail_prefix\u003dtail_prefix, logger\u003dlogger)\ndiff --git a/swift/common/utils/logs.py b/swift/common/utils/logs.py\nindex 1e06b0973..54780f10d 100644\n--- a/swift/common/utils/logs.py\n+++ b/swift/common/utils/logs.py\n@@ -729,8 +729,10 @@ def get_logger(conf, name\u003dNone, log_to_console\u003dFalse, log_route\u003dNone,\n         getattr(logging, conf.get(\u0027log_level\u0027, \u0027INFO\u0027).upper(), logging.INFO))\n \n     # Setup logger with a StatsD client if so configured\n+    if statsd_tail_prefix is None:\n+        statsd_tail_prefix \u003d name\n     logger.statsd_client \u003d statsd_client.get_statsd_client(\n-        conf, statsd_tail_prefix, logger, default_tail_prefix\u003dname)\n+        conf, statsd_tail_prefix, logger)\n \n     adapted_logger \u003d LogAdapter(logger, name)\n     other_handlers \u003d conf.get(\u0027log_custom_handlers\u0027, None)\n```\nThen in https://review.opendev.org/c/openstack/swift/+/915483 it gets pulled out of `get_adapted_logger` and into the `LoggerStatsdFascade`-returning `get_logger` -- and *that* continues to be the one place where logs and stats need to bleed together like that.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":42,"context_line":"             is not specified in ``conf``"},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    if tail_prefix is None:"},{"line_number":45,"context_line":"        tail_prefix \u003d name"},{"line_number":46,"context_line":"    # statsd_client \u003d None"},{"line_number":47,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"        conf,"}],"source_content_type":"text/x-python","patch_set":5,"id":"46cafe4e_91307e71","line":45,"in_reply_to":"d406b1fe_4f3449a4","updated":"2024-06-04 22:36:08.000000000","message":"Yes, we had that previously. But in the last revision, Clay suggested to have name parameter and use it for tail prefix in get_statsd_client. Since the parameter name is ambiguous, I\u0027ll rename the parameter to \u0027default_tail_prefix\u0027.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    if tail_prefix is None:"},{"line_number":45,"context_line":"        tail_prefix \u003d name"},{"line_number":46,"context_line":"    # statsd_client \u003d None"},{"line_number":47,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"        conf,"},{"line_number":49,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"}],"source_content_type":"text/x-python","patch_set":5,"id":"b00fddb7_ee64ddd6","line":46,"updated":"2024-06-04 00:25:10.000000000","message":"No longer needed.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    if tail_prefix is None:"},{"line_number":45,"context_line":"        tail_prefix \u003d name"},{"line_number":46,"context_line":"    # statsd_client \u003d None"},{"line_number":47,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":48,"context_line":"        conf,"},{"line_number":49,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"}],"source_content_type":"text/x-python","patch_set":5,"id":"4aaecd44_c67e4f43","line":46,"in_reply_to":"b00fddb7_ee64ddd6","updated":"2024-06-04 22:36:08.000000000","message":"Acknowledged","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":54,"context_line":"    except ValueError as err:"},{"line_number":55,"context_line":"        if str(err) \u003d\u003d \"Statsd Host is Empty\":"},{"line_number":56,"context_line":"            return None"},{"line_number":57,"context_line":"        raise\u0027\u0027\u0027"},{"line_number":58,"context_line":"    return statsd_client"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"05e50032_09fbe020","line":57,"updated":"2024-06-04 00:25:10.000000000","message":"Pretty sure we can just take this out now.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    except ValueError as err:"},{"line_number":55,"context_line":"        if str(err) \u003d\u003d \"Statsd Host is Empty\":"},{"line_number":56,"context_line":"            return None"},{"line_number":57,"context_line":"        raise\u0027\u0027\u0027"},{"line_number":58,"context_line":"    return statsd_client"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"c72a2cb1_5a6d49bd","line":57,"in_reply_to":"05e50032_09fbe020","updated":"2024-06-04 22:36:08.000000000","message":"Done","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"class StatsdClient(StatsdClientConf):"},{"line_number":74,"context_line":"    def __init__(self, conf, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":75,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":76,"context_line":"        self.random \u003d random"},{"line_number":77,"context_line":"        self.logger \u003d logger"}],"source_content_type":"text/x-python","patch_set":5,"id":"31122de9_9ef7ed25","line":74,"range":{"start_line":74,"start_character":23,"end_line":74,"end_character":27},"updated":"2024-06-04 00:25:10.000000000","message":"All right, it\u0027s a significant signature change, but we don\u0027t really think anyone has been instantiating these directly, right? And the **official** API for getting a client will be `get_statsd_client`","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"class StatsdClient(StatsdClientConf):"},{"line_number":74,"context_line":"    def __init__(self, conf, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":75,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":76,"context_line":"        self.random \u003d random"},{"line_number":77,"context_line":"        self.logger \u003d logger"}],"source_content_type":"text/x-python","patch_set":5,"id":"085d6818_556e5203","line":74,"range":{"start_line":74,"start_character":23,"end_line":74,"end_character":27},"in_reply_to":"31122de9_9ef7ed25","updated":"2024-06-04 22:36:08.000000000","message":"It is directly used with a fakeStatsdClient from debug_logger","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        :param tail_prefix: The new value of tail_prefix"},{"line_number":138,"context_line":"        \"\"\""},{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"5e0450d5_4ec9d7b1","line":139,"updated":"2024-06-04 00:25:10.000000000","message":"This should always be overwritten by the time we return, yeah? Should we just drop this?\n\nSide note: where do we use this?","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        :param tail_prefix: The new value of tail_prefix"},{"line_number":138,"context_line":"        \"\"\""},{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"5acd267b_51d2da14","line":139,"in_reply_to":"5e0450d5_4ec9d7b1","updated":"2024-06-04 22:36:08.000000000","message":"Acknowledged","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"},{"line_number":143,"context_line":"        elif tail_prefix:"},{"line_number":144,"context_line":"            self._prefix \u003d tail_prefix + \u0027.\u0027"},{"line_number":145,"context_line":"            self.prefix \u003d tail_prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"cecd46ac_726d291e","line":142,"updated":"2024-06-04 00:25:10.000000000","message":"This will still have the trailing `.` -- was that intentional? I\u0027m still a little confused about why we track `self.prefix` in addition to `self._prefix`.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5d03fd3840704bbb1946b7e5bcb9fa170b166347","unresolved":true,"context_lines":[{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"},{"line_number":143,"context_line":"        elif tail_prefix:"},{"line_number":144,"context_line":"            self._prefix \u003d tail_prefix + \u0027.\u0027"},{"line_number":145,"context_line":"            self.prefix \u003d tail_prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"b9b78524_2877fb37","line":142,"in_reply_to":"7ce078f4_9a0d30ea","updated":"2024-06-07 20:07:45.000000000","message":"I\u0027m still confused about `self.prefix`. Applying something like\n```\ndiff --git a/swift/common/statsd_client.py b/swift/common/statsd_client.py\nindex 1db7c3bbd..a6c8a7bd6 100644\n--- a/swift/common/statsd_client.py\n+++ b/swift/common/statsd_client.py\n@@ -134,7 +134,7 @@ class StatsdClient(StatsdClientConf):\n         self.prefix \u003d None\n         if tail_prefix and self.base_prefix:\n             self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])\n-            self.prefix \u003d self._prefix\n+            self.prefix \u003d self._prefix[:-1]\n         elif tail_prefix:\n             self._prefix \u003d tail_prefix + \u0027.\u0027\n             self.prefix \u003d tail_prefix\n```\nall the tests still pass. Nothing seems to actually *look* at `StatsdClient.prefix`:\n```\n$ ack \u0027\\.prefix\u0027 swift\nswift/cli/container_deleter.py\n166:            args.marker, args.end_marker, args.prefix, args.timestamp):\n\nswift/common/statsd_client.py\n134:        self.prefix \u003d None\n137:            self.prefix \u003d self._prefix[:-1]\n140:            self.prefix \u003d tail_prefix\n143:            self.prefix \u003d self.base_prefix\n146:            self.prefix \u003d self._prefix\n```\nWhy do we have this?","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"4a42c62826f401ed8ff1a009dc63dd1a42c846fd","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"},{"line_number":143,"context_line":"        elif tail_prefix:"},{"line_number":144,"context_line":"            self._prefix \u003d tail_prefix + \u0027.\u0027"},{"line_number":145,"context_line":"            self.prefix \u003d tail_prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"56fe00cd_41d26603","line":142,"in_reply_to":"82bf1022_92368f45","updated":"2024-06-10 18:40:31.000000000","message":"Sounds reasonable not to have two \u0027almost\u0027 similar variables. I\u0027ll tweak the tests to not depend on self.prefix anymore so that we can get rid of it!","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"ea873f1fe7900db3414085587be900f827eddebf","unresolved":true,"context_lines":[{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"},{"line_number":143,"context_line":"        elif tail_prefix:"},{"line_number":144,"context_line":"            self._prefix \u003d tail_prefix + \u0027.\u0027"},{"line_number":145,"context_line":"            self.prefix \u003d tail_prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfd23f0e_ad542bc4","line":142,"in_reply_to":"b9b78524_2877fb37","updated":"2024-06-10 16:12:04.000000000","message":"I don\u0027t know why that was used. If you think it\u0027s a good idea to remove the trailing \u0027.\u0027 we can do it. I just don\u0027t know if it has some significance elsewhere in the codebase","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ccfd0bb9cdba9084972ad4483c9de28cd6f84cf","unresolved":true,"context_lines":[{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"},{"line_number":143,"context_line":"        elif tail_prefix:"},{"line_number":144,"context_line":"            self._prefix \u003d tail_prefix + \u0027.\u0027"},{"line_number":145,"context_line":"            self.prefix \u003d tail_prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"82bf1022_92368f45","line":142,"in_reply_to":"bfd23f0e_ad542bc4","updated":"2024-06-10 17:22:52.000000000","message":"Well, the (only sometimes present) trailing `\u0027.\u0027` was a smell -- but the larger issue I have now is the `self._prefix`/`self.prefix` split. I see that there are a couple assertions in `proxy/test_server.py` that use `.prefix` -- but why not have them look at `._prefix` instead? Why are we trying to track two almost-but-not-quite identical pieces of information? Especially when one of them is never read from within `swift/` code...","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        self.prefix \u003d None"},{"line_number":140,"context_line":"        if tail_prefix and self.base_prefix:"},{"line_number":141,"context_line":"            self._prefix \u003d \u0027.\u0027.join([self.base_prefix, tail_prefix, \u0027\u0027])"},{"line_number":142,"context_line":"            self.prefix \u003d self._prefix"},{"line_number":143,"context_line":"        elif tail_prefix:"},{"line_number":144,"context_line":"            self._prefix \u003d tail_prefix + \u0027.\u0027"},{"line_number":145,"context_line":"            self.prefix \u003d tail_prefix"}],"source_content_type":"text/x-python","patch_set":5,"id":"7ce078f4_9a0d30ea","line":142,"in_reply_to":"cecd46ac_726d291e","updated":"2024-06-04 22:36:08.000000000","message":"Afaik, it is used in all the unit tests and hence, I kept the trailing . as is.","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ccfd0bb9cdba9084972ad4483c9de28cd6f84cf","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":46,"context_line":"        conf,"},{"line_number":47,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"},{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff2c6494_3918287c","line":48,"updated":"2024-06-10 17:22:52.000000000","message":"I\u0027m really happy with all the simplifications we\u0027ve made here, but this is starting to look so anemic as to be suspicious. Is there a plan for this to be the place where the `log_`-prefix handling happens later or something? And `StatsdClientConf`/`StatsdClient` will just look at `statsd_`-prefixed values in the passed in conf? Or is this going to continue to be just a super-thin shim into `StatsdClient`?\n\nIf the latter, I\u0027m inclined to establish `StatsdClient` as the blessed API for getting a stats client. I *think* I\u0027m still comfortable with the signature changes there, as we don\u0027t really expect third parties to be getting a pure statsd client until 2.34.0+","commit_id":"35d91a354eb9a13e923626acd6f9237913a0e073"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52f588a55c052191f36366a36b43f3db530814ed","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":46,"context_line":"        conf,"},{"line_number":47,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"},{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"86abbfb7_a6e565ae","line":48,"in_reply_to":"4d24aa9e_4eceba28","updated":"2024-06-12 20:23:43.000000000","message":"I have a weak preference for keeping the symetry between `statsd_client.get_statsd_client` and `common.logs.get_adapted_logger` even if it\u0027s a (very) thin wrapper around the constructor.\n\n\u003e Once we ship this implementation it would be unnecessary, and backwards-incompatible, to move the log_* parsing out of the constructor\n\nI agree, but we may still want to do it that way - we should write that code ASAP to get a feel for it.","commit_id":"35d91a354eb9a13e923626acd6f9237913a0e073"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"42b9502bb35992ef99fd32c017ba18946b899bae","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":46,"context_line":"        conf,"},{"line_number":47,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"},{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fd35e673_0eb6b4ad","line":48,"in_reply_to":"86abbfb7_a6e565ae","updated":"2024-06-13 09:09:25.000000000","message":"\u003e I agree, but we may still want to do it that way - we should write that code ASAP to get a feel for it.\n\naah, I thought there had been a definite decision to push conf dict into the constructor.\n\nTo avoid confusion (but I\u0027m genuinely not just beating my own drum), in order of preference I would choose:\n\n1. get_statsd_client parses conf and instantiates StatsdClient using args. StatsdClient can have a docstring with ``:param:`` tags that documents its interface. More verbose to write but IMHO nicer to live with. Has similarities with get_logger that also parses the conf dict.\n\n2. this patch+comment i.e. get_statsd_client does nothing, StatsdClient parses a dict. There ought to be some kind of doc/comment that justifies the existence of get_statsd_client and why it should be preferred over just calling StatsdClient. Even consider making the class \"private\" with an underscore name _StatsdClient.\n\n3. get_statsd_client cleans up conf dict to remove log_* prefixes and passes a sanitised dict to StatsdClient which then parses the conf. Don\u0027t really like this because it means parsing a conf dict in two places.","commit_id":"35d91a354eb9a13e923626acd6f9237913a0e073"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fa65c464e68e35e20057f4914f531271ef9fc9a","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":46,"context_line":"        conf,"},{"line_number":47,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"},{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"4d24aa9e_4eceba28","line":48,"in_reply_to":"8e55927c_dc52bf58","updated":"2024-06-12 09:10:46.000000000","message":"tl;dr: I\u0027m not convinced we need this function.\n\nI agree with Tim - as it is, the function is redundant, so there needs to be some reason to establish this as the preferred interface.\n\nI had previously assumed that get_statsd_client would be a place to sanitise the ``log_*`` prefixed options, but I don\u0027t think I won that argument. Once we ship this implementation it would be unnecessary, and backwards-incompatible, to move the ``log_*`` parsing out of the constructor. If we want to support options without the ``log_`` prefix we could just add to the constructor parsing e.g. \n\n```\nself.host \u003d conf.get(\u0027statsd_host\u0027, conf.get(\u0027log_statsd_host\u0027))\n```\n\nRe. singletons: singletons typically don\u0027t have parameters, so it doesn\u0027t really make sense to be passing a conf dict to a singleton-constructor\nIf there is any thought of there being a singleton pattern in the future, then do we need to think about there being a singleton-per-unique-config? This is in fact an oxymoron 😊 and TIL it would be an \"interning\" pattern https://www.cs.sjsu.edu/~pearce/modules/patterns/creational/interning.htm.\n\nIn this case the config must be parsed to a canonical form before being mapped to an instance, because non-identical config dicts can be equivalent to the same StatsdClient attributes e.g. \n\n```\nconf1 \u003d {\u0027log_statsd_host\u0027: \u0027foo\u0027, \u0027log_statsd_default_sample_rate\u0027: \u00271\u0027}\n```\n\nresults in the same attributes as \n\n```\nconf2 \u003d {\u0027log_statsd_host\u0027: \u0027foo\u0027}\n```\n\nbut ``conf1 !\u003d conf2``.\n\nSo this would be an argument for parsing to happen in ``get_statsd_client()``, or (my preference) to have a ``StasdClient.instance_for_conf(conf)`` method.","commit_id":"35d91a354eb9a13e923626acd6f9237913a0e073"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"1d488588989d9d057a51f1a5a92a40aed3412e9d","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":46,"context_line":"        conf,"},{"line_number":47,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"},{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"8e55927c_dc52bf58","line":48,"in_reply_to":"a6f89f9b_46168f96","updated":"2024-06-10 21:26:25.000000000","message":"It seems the log_ -prefix handling could either be in get_statsd_client or StatsdClientConf/StatsdClient, so removing get_statsd_client should not be an issue for the implementation. Although wonder if we would possibly need to return a same StatsdClient instance or a singleton instance in future? if so, having get_statsd_client might provide more flexibility?","commit_id":"35d91a354eb9a13e923626acd6f9237913a0e073"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"73f3ecb809ed303f14049879be99140a5c3decd4","unresolved":true,"context_lines":[{"line_number":45,"context_line":"    statsd_client \u003d StatsdClient("},{"line_number":46,"context_line":"        conf,"},{"line_number":47,"context_line":"        tail_prefix\u003dtail_prefix, logger\u003dlogger)"},{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"a6f89f9b_46168f96","line":48,"in_reply_to":"ff2c6494_3918287c","updated":"2024-06-10 20:16:27.000000000","message":"I don\u0027t think we have exact plans for log_ prefix handling but we would surely have something here. With these changes, we have the flexibility to just have a StatsdClient shim or have log_ prefix handling with the upcoming patch. To have that flexibility, we can continue with this patch if we can carry it by this week.","commit_id":"35d91a354eb9a13e923626acd6f9237913a0e073"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fa65c464e68e35e20057f4914f531271ef9fc9a","unresolved":true,"context_lines":[{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Get an instance of StatsdClient using config settings."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"d4150021_0f4e3a1d","line":27,"updated":"2024-06-12 09:10:46.000000000","message":"AFAICT this is not called directly by any test.","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"6d78fe10898647be383de4ac1343de289d2871c6","unresolved":false,"context_lines":[{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Get an instance of StatsdClient using config settings."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"865a2c48_041663b8","line":27,"in_reply_to":"c91389c3_6e4857bc","updated":"2024-06-13 00:07:51.000000000","message":"Adding get_statsd_client in a couple of tests: One positive, one negetive","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52f588a55c052191f36366a36b43f3db530814ed","unresolved":true,"context_lines":[{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_statsd_client(conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Get an instance of StatsdClient using config settings."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"c91389c3_6e4857bc","line":27,"in_reply_to":"d4150021_0f4e3a1d","updated":"2024-06-12 20:23:43.000000000","message":"agree, needs tests.","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fa65c464e68e35e20057f4914f531271ef9fc9a","unresolved":true,"context_lines":[{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"},{"line_number":52,"context_line":"    def __init__(self, conf):"},{"line_number":53,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":54,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"960b4ec9_eda7bcaa","line":51,"updated":"2024-06-12 09:10:46.000000000","message":"I still don\u0027t understand why this superclass is necessary or desirable","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"52f588a55c052191f36366a36b43f3db530814ed","unresolved":true,"context_lines":[{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"},{"line_number":52,"context_line":"    def __init__(self, conf):"},{"line_number":53,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":54,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"f5329280_c0a60cc1","line":51,"in_reply_to":"960b4ec9_eda7bcaa","updated":"2024-06-12 20:23:43.000000000","message":"I forget too, but we had some notes from previous discussoins - Yan is going to elaborate.","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4042e3a22b96885b9769834f78923ea720159ed0","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"},{"line_number":52,"context_line":"    def __init__(self, conf):"},{"line_number":53,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":54,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"efb3fe1d_8d30b13f","line":51,"in_reply_to":"d3690c40_0afbb5fa","updated":"2024-06-19 11:22:52.000000000","message":"Done","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"aa45d75bcd1df5ecebedbd2654aea674dac408d7","unresolved":true,"context_lines":[{"line_number":48,"context_line":"    return statsd_client"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"},{"line_number":52,"context_line":"    def __init__(self, conf):"},{"line_number":53,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":54,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"d3690c40_0afbb5fa","line":51,"in_reply_to":"f5329280_c0a60cc1","updated":"2024-06-13 21:26:42.000000000","message":"It seems that there are a few patterns for parsing config dicts in the existing code base:  \n  - the class constructor takes a config dict, e.g. proxy or other apps\n  - the class constructor takes kwargs; config parsing is done somewhere else which transforms the config dict into kwargs, e.g. StatsdClient\n  - a config class parses the config dict into a config object; the class constructor either takes the config object or has a \"mixin config object\" pattern, e.g. ContainerSharder\n\nThe config class seems a better choice comparing to the other approaches, in that it provides a more concise and understandable API for the class. The \"mixin config object\" or super class would be a compromise to avoid changing many existing unit tests.","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4fa65c464e68e35e20057f4914f531271ef9fc9a","unresolved":true,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class StatsdClient(StatsdClientConf):"},{"line_number":64,"context_line":"    def __init__(self, conf, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":65,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":66,"context_line":"        self.random \u003d random"},{"line_number":67,"context_line":"        self.logger \u003d logger"},{"line_number":68,"context_line":"        self._set_prefix(tail_prefix)"}],"source_content_type":"text/x-python","patch_set":9,"id":"dfdc95b0_eb7b76c7","line":65,"updated":"2024-06-12 09:10:46.000000000","message":"I\u0027m used to seeing\n\n```\nsuper(StatsdClient, self).__init__(conf)\n```\n\nwhich tolerates the superclass changing in the class declaration.","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class StatsdClient(StatsdClientConf):"},{"line_number":64,"context_line":"    def __init__(self, conf, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":65,"context_line":"        StatsdClientConf.__init__(self, conf)"},{"line_number":66,"context_line":"        self.random \u003d random"},{"line_number":67,"context_line":"        self.logger \u003d logger"},{"line_number":68,"context_line":"        self._set_prefix(tail_prefix)"}],"source_content_type":"text/x-python","patch_set":9,"id":"af549046_c6e0e3cd","line":65,"in_reply_to":"dfdc95b0_eb7b76c7","updated":"2024-06-18 22:12:11.000000000","message":"Done","commit_id":"54fceaa54ab16a40cee5b2ab1844fd568ae0cd4c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"42b9502bb35992ef99fd32c017ba18946b899bae","unresolved":true,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"},{"line_number":52,"context_line":"    def __init__(self, conf):"},{"line_number":53,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":54,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":55,"context_line":"        self.port \u003d int(conf.get(\u0027log_statsd_port\u0027, 8125))"}],"source_content_type":"text/x-python","patch_set":10,"id":"ed7a627e_6fc39809","line":52,"updated":"2024-06-13 09:09:25.000000000","message":"can we please have the docstring here in addition to, or instead of, the shim function\n\n1. If I want to know the interface for StatsdClient then I\u0027ll look at StatsdClient.\n\n2. When a follow on patch adds options, it is unfortunate that the docstring has to be maintained in a different place than the actual class changes e.g. https://review.opendev.org/c/openstack/swift/+/909882/comment/0d15ae4d_d2bdcd9f/","commit_id":"29ac7bc8f533f05ad4a574f6d60e92282d942870"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class StatsdClientConf(object):"},{"line_number":52,"context_line":"    def __init__(self, conf):"},{"line_number":53,"context_line":"        conf \u003d conf if conf else {}"},{"line_number":54,"context_line":"        self.host \u003d conf.get(\u0027log_statsd_host\u0027)"},{"line_number":55,"context_line":"        self.port \u003d int(conf.get(\u0027log_statsd_port\u0027, 8125))"}],"source_content_type":"text/x-python","patch_set":10,"id":"f4da1e7f_31e196e4","line":52,"in_reply_to":"ed7a627e_6fc39809","updated":"2024-06-18 22:12:11.000000000","message":"Done","commit_id":"29ac7bc8f533f05ad4a574f6d60e92282d942870"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":42,"context_line":"    :param conf: Configuration dict to read settings from"},{"line_number":43,"context_line":"    :param tail_prefix: tail prefix to pass to statsd client"},{"line_number":44,"context_line":"    :param logger: stdlib logger instance used by statsd client for logging"},{"line_number":45,"context_line":"    :return: an instance of ``StatsdClient``, or None if ``log_statsd_host``"},{"line_number":46,"context_line":"             is not specified in ``conf``"},{"line_number":47,"context_line":"    \"\"\""},{"line_number":48,"context_line":"    conf \u003d conf or {}"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"346890b3_9f7b53c7","line":46,"range":{"start_line":45,"start_character":46,"end_line":46,"end_character":41},"updated":"2024-06-18 10:54:15.000000000","message":"no longer true","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":42,"context_line":"    :param conf: Configuration dict to read settings from"},{"line_number":43,"context_line":"    :param tail_prefix: tail prefix to pass to statsd client"},{"line_number":44,"context_line":"    :param logger: stdlib logger instance used by statsd client for logging"},{"line_number":45,"context_line":"    :return: an instance of ``StatsdClient``, or None if ``log_statsd_host``"},{"line_number":46,"context_line":"             is not specified in ``conf``"},{"line_number":47,"context_line":"    \"\"\""},{"line_number":48,"context_line":"    conf \u003d conf or {}"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"6fd1d26f_0f7dcbea","line":46,"range":{"start_line":45,"start_character":46,"end_line":46,"end_character":41},"in_reply_to":"346890b3_9f7b53c7","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":50,"context_line":"    if \u0027log_statsd_host\u0027 in conf and \u0027statsd_host\u0027 not in conf:"},{"line_number":51,"context_line":"        host \u003d conf[\u0027log_statsd_host\u0027]"},{"line_number":52,"context_line":"    else:"},{"line_number":53,"context_line":"        host \u003d conf.get(\u0027statsd_host\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    if \u0027log_statsd_port\u0027 in conf and \u0027statsd_port\u0027 not in conf:"},{"line_number":56,"context_line":"        port \u003d int(conf[\u0027log_statsd_port\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"221b2554_7e62d1b7","line":53,"updated":"2024-06-18 10:54:15.000000000","message":"shorter form would be \n\n```\nhost \u003d conf.get(\u0027statsd_host\u0027, conf.get(\u0027log_statsd_host\u0027))\n```","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    if \u0027log_statsd_host\u0027 in conf and \u0027statsd_host\u0027 not in conf:"},{"line_number":51,"context_line":"        host \u003d conf[\u0027log_statsd_host\u0027]"},{"line_number":52,"context_line":"    else:"},{"line_number":53,"context_line":"        host \u003d conf.get(\u0027statsd_host\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    if \u0027log_statsd_port\u0027 in conf and \u0027statsd_port\u0027 not in conf:"},{"line_number":56,"context_line":"        port \u003d int(conf[\u0027log_statsd_port\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"f2ddbd68_8311495c","line":53,"in_reply_to":"221b2554_7e62d1b7","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98c39b091532bc877dd32ef3add2cf29a5098e98","unresolved":true,"context_lines":[{"line_number":50,"context_line":"    if \u0027log_statsd_host\u0027 in conf and \u0027statsd_host\u0027 not in conf:"},{"line_number":51,"context_line":"        host \u003d conf[\u0027log_statsd_host\u0027]"},{"line_number":52,"context_line":"    else:"},{"line_number":53,"context_line":"        host \u003d conf.get(\u0027statsd_host\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    if \u0027log_statsd_port\u0027 in conf and \u0027statsd_port\u0027 not in conf:"},{"line_number":56,"context_line":"        port \u003d int(conf[\u0027log_statsd_port\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"6e575abb_90513c0b","line":53,"in_reply_to":"221b2554_7e62d1b7","updated":"2024-06-18 20:24:13.000000000","message":"I mainly left it expanded so we could add warnings -- but https://review.opendev.org/c/openstack/swift/+/922200 is an interesting way to do it, too.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        tail_prefix\u003dtail_prefix,"},{"line_number":82,"context_line":"        default_sample_rate\u003ddefault_sample_rate,"},{"line_number":83,"context_line":"        sample_rate_factor\u003dsample_rate_factor,"},{"line_number":84,"context_line":"        logger\u003dlogger)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"class StatsdClient(object):"}],"source_content_type":"text/x-python","patch_set":11,"id":"0e1c972e_aacd6019","line":84,"updated":"2024-06-18 10:54:15.000000000","message":"I quite like this approach. There are pros and cons:\n\npros:\n\n* there is no ambiguity about how to create a StatsdClient - if you have a conf dict then you *must* use get_statsd_client, you cannot shortcut to StatsdClient.\n* clear separation of conf parsing in get_statsd_client from the business of the StatsdClient.\n* legacy options dealt with in get_statsd_client\n* constructor doesn\u0027t change and remains regular args, kwargs pattern. Very easy to see what parameters the StatsdClient accepts.\n* We could/should write a StatsdClient docstring with :param: tags etc.\n\ncons:\n* it\u0027s more verbose: when adding a new option, we have to parse it from the conf in get_statsd_client AND add it to the constructor.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        tail_prefix\u003dtail_prefix,"},{"line_number":82,"context_line":"        default_sample_rate\u003ddefault_sample_rate,"},{"line_number":83,"context_line":"        sample_rate_factor\u003dsample_rate_factor,"},{"line_number":84,"context_line":"        logger\u003dlogger)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"class StatsdClient(object):"}],"source_content_type":"text/x-python","patch_set":11,"id":"253e6424_66659809","line":84,"in_reply_to":"0e1c972e_aacd6019","updated":"2024-06-18 22:12:11.000000000","message":"Done","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        self.port \u003d port"},{"line_number":92,"context_line":"        self.base_prefix \u003d base_prefix"},{"line_number":93,"context_line":"        self.default_sample_rate \u003d default_sample_rate"},{"line_number":94,"context_line":"        self.sample_rate_factor \u003d sample_rate_factor"},{"line_number":95,"context_line":"        self.random \u003d random"},{"line_number":96,"context_line":"        self.logger \u003d logger"},{"line_number":97,"context_line":"        self._set_prefix(tail_prefix)"}],"source_content_type":"text/x-python","patch_set":11,"id":"b102c8b5_45d4ad14","line":94,"updated":"2024-06-18 10:54:15.000000000","message":"is there a reason to remove the ``_`` prefixes from the attributes? It causes quite a bit of otherwise unnecessary churn in the tests; the patch would be smaller without this change","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        self.port \u003d port"},{"line_number":92,"context_line":"        self.base_prefix \u003d base_prefix"},{"line_number":93,"context_line":"        self.default_sample_rate \u003d default_sample_rate"},{"line_number":94,"context_line":"        self.sample_rate_factor \u003d sample_rate_factor"},{"line_number":95,"context_line":"        self.random \u003d random"},{"line_number":96,"context_line":"        self.logger \u003d logger"},{"line_number":97,"context_line":"        self._set_prefix(tail_prefix)"}],"source_content_type":"text/x-python","patch_set":11,"id":"753554be_71cad6a7","line":94,"in_reply_to":"314fbe18_561ad0f8","updated":"2024-06-18 22:12:11.000000000","message":"Reverting","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98c39b091532bc877dd32ef3add2cf29a5098e98","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        self.port \u003d port"},{"line_number":92,"context_line":"        self.base_prefix \u003d base_prefix"},{"line_number":93,"context_line":"        self.default_sample_rate \u003d default_sample_rate"},{"line_number":94,"context_line":"        self.sample_rate_factor \u003d sample_rate_factor"},{"line_number":95,"context_line":"        self.random \u003d random"},{"line_number":96,"context_line":"        self.logger \u003d logger"},{"line_number":97,"context_line":"        self._set_prefix(tail_prefix)"}],"source_content_type":"text/x-python","patch_set":11,"id":"314fbe18_561ad0f8","line":94,"in_reply_to":"b102c8b5_45d4ad14","updated":"2024-06-18 20:24:13.000000000","message":"+1","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":132,"context_line":"            # sockaddr: IPv4 has (address, port) while IPv6 has (address,"},{"line_number":133,"context_line":"            # port, flow info, scope id)."},{"line_number":134,"context_line":"            sockaddr \u003d addr_info[0][-1]"},{"line_number":135,"context_line":"            self._target \u003d (self.host,) + (sockaddr[1:])"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        else:"},{"line_number":138,"context_line":"            self._target \u003d (self.host, self.port)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def _set_prefix(self, tail_prefix):"},{"line_number":141,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"62b98d48_d8201e32","line":138,"range":{"start_line":135,"start_character":0,"end_line":138,"end_character":49},"updated":"2024-06-18 10:54:15.000000000","message":"I think this was ok as it was: host and port are passed to the method, so use them, no need to change","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            # sockaddr: IPv4 has (address, port) while IPv6 has (address,"},{"line_number":133,"context_line":"            # port, flow info, scope id)."},{"line_number":134,"context_line":"            sockaddr \u003d addr_info[0][-1]"},{"line_number":135,"context_line":"            self._target \u003d (self.host,) + (sockaddr[1:])"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        else:"},{"line_number":138,"context_line":"            self._target \u003d (self.host, self.port)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def _set_prefix(self, tail_prefix):"},{"line_number":141,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"d428b311_71772e8d","line":138,"range":{"start_line":135,"start_character":0,"end_line":138,"end_character":49},"in_reply_to":"62b98d48_d8201e32","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e649f6693c31d259f0bc41bb21b282736ed28d4","unresolved":true,"context_lines":[{"line_number":50,"context_line":"    default_sample_rate \u003d float("},{"line_number":51,"context_line":"        conf.get(\u0027log_statsd_default_sample_rate\u0027, 1))"},{"line_number":52,"context_line":"    sample_rate_factor \u003d float("},{"line_number":53,"context_line":"        conf.get(\u0027log_statsd_sample_rate_factor\u0027, 1))"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    return StatsdClient(host, port, base_prefix\u003dbase_prefix,"},{"line_number":56,"context_line":"                        tail_prefix\u003dtail_prefix,"}],"source_content_type":"text/x-python","patch_set":13,"id":"17f5ea80_55fabe20","line":53,"updated":"2024-06-24 11:09:47.000000000","message":"ok - moved without change from logs.py","commit_id":"0c58afc7ee23fd5e5cca19318e3f055dcc79e021"}],"swift/common/utils/logs.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":732,"context_line":"    if statsd_tail_prefix is None:"},{"line_number":733,"context_line":"        statsd_tail_prefix \u003d name"},{"line_number":734,"context_line":"    logger.statsd_client \u003d statsd_client.get_statsd_client("},{"line_number":735,"context_line":"        conf, statsd_tail_prefix, logger)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    adapted_logger \u003d LogAdapter(logger, name)"},{"line_number":738,"context_line":"    other_handlers \u003d conf.get(\u0027log_custom_handlers\u0027, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"89fcf9f1_d16a8483","line":735,"updated":"2024-05-14 11:53:14.000000000","message":"so much nicer!","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"7642bab0ad28e0e992c54fe84030d6d47e076ce4","unresolved":false,"context_lines":[{"line_number":732,"context_line":"    if statsd_tail_prefix is None:"},{"line_number":733,"context_line":"        statsd_tail_prefix \u003d name"},{"line_number":734,"context_line":"    logger.statsd_client \u003d statsd_client.get_statsd_client("},{"line_number":735,"context_line":"        conf, statsd_tail_prefix, logger)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    adapted_logger \u003d LogAdapter(logger, name)"},{"line_number":738,"context_line":"    other_handlers \u003d conf.get(\u0027log_custom_handlers\u0027, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"624192ab_44d54838","line":735,"in_reply_to":"89fcf9f1_d16a8483","updated":"2024-05-14 23:42:21.000000000","message":"Done","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":667,"context_line":"    if not conf:"},{"line_number":668,"context_line":"        conf \u003d {}"},{"line_number":669,"context_line":"    if name is None:"},{"line_number":670,"context_line":"        name \u003d conf.get(\u0027log_name\u0027, \u0027swift\u0027)"},{"line_number":671,"context_line":"    if not log_route:"},{"line_number":672,"context_line":"        log_route \u003d name"},{"line_number":673,"context_line":"    logger \u003d logging.getLogger(log_route)"}],"source_content_type":"text/x-python","patch_set":2,"id":"68935cec_0aa3c157","line":670,"updated":"2024-05-15 15:53:35.000000000","message":"it looks like most servers/daemons pass in `log_route\u003d\u0027whatever\u0027` - and I\u0027m not sure why the config wouldn\u0027t override the passed in option...","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":667,"context_line":"    if not conf:"},{"line_number":668,"context_line":"        conf \u003d {}"},{"line_number":669,"context_line":"    if name is None:"},{"line_number":670,"context_line":"        name \u003d conf.get(\u0027log_name\u0027, \u0027swift\u0027)"},{"line_number":671,"context_line":"    if not log_route:"},{"line_number":672,"context_line":"        log_route \u003d name"},{"line_number":673,"context_line":"    logger \u003d logging.getLogger(log_route)"}],"source_content_type":"text/x-python","patch_set":2,"id":"024c149f_f47502b9","line":670,"in_reply_to":"68935cec_0aa3c157","updated":"2024-06-04 22:36:08.000000000","message":"Most servers/daemons would pass log_route, although, in the existing code, it seems that the specified option in parameters would override the default value in the config, not the other way","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":730,"context_line":""},{"line_number":731,"context_line":"    # Setup logger with a StatsD client if so configured"},{"line_number":732,"context_line":"    if statsd_tail_prefix is None:"},{"line_number":733,"context_line":"        statsd_tail_prefix \u003d name"},{"line_number":734,"context_line":"    logger.statsd_client \u003d statsd_client.get_statsd_client("},{"line_number":735,"context_line":"        conf, statsd_tail_prefix, logger)"},{"line_number":736,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"f4f2e33e_14f3724f","line":733,"updated":"2024-05-15 15:53:35.000000000","message":"kind of annoying to to see special handling of statsd_ kwargs still in get_logger - maybe better to pass both name and statsd_tail_prefix into get_statsd_client.\n\nI can imagine a future where servers say:\n\n    self.logger \u003d get_logger(confg, name\u003d\u0027proxy-server\u0027)\n    self.stats \u003d get_statsd_client(confg, name\u003d\u0027proxy-server\u0027)\n    \n... but ops still has the option to override:\n\n    [DEFAULT]\n    log_name \u003d \u0027special-proxy-logs\u0027\n    statsd_name \u003d \u0027special-proxy-metrics\u0027\n    \nHopefully tail-prefix eventually becomes un-needed.","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":730,"context_line":""},{"line_number":731,"context_line":"    # Setup logger with a StatsD client if so configured"},{"line_number":732,"context_line":"    if statsd_tail_prefix is None:"},{"line_number":733,"context_line":"        statsd_tail_prefix \u003d name"},{"line_number":734,"context_line":"    logger.statsd_client \u003d statsd_client.get_statsd_client("},{"line_number":735,"context_line":"        conf, statsd_tail_prefix, logger)"},{"line_number":736,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"b3390b42_64feafb2","line":733,"in_reply_to":"f4f2e33e_14f3724f","updated":"2024-05-16 22:20:50.000000000","message":"Acknowledged","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"}],"test/debug_logger.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":148,"context_line":"            self.facility \u003d kwargs[\u0027facility\u0027]"},{"line_number":149,"context_line":"        self.statsd_client \u003d FakeStatsdClient({"},{"line_number":150,"context_line":"            \u0027log_statsd_host\u0027: \"host\","},{"line_number":151,"context_line":"            \u0027log_statsd_port\u0027: 8125})"},{"line_number":152,"context_line":"        self.thread_locals \u003d None"},{"line_number":153,"context_line":"        self.parent \u003d None"},{"line_number":154,"context_line":"        # ensure the NOTICE level has been named, in case it has not already"}],"source_content_type":"text/x-python","patch_set":2,"id":"446972af_7a9762f5","line":151,"updated":"2024-05-15 15:53:35.000000000","message":"hahaha, that log_ prefix looks pretty dumb!","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"b7937052a1dbea05f2f687aa4a26692599f75182","unresolved":false,"context_lines":[{"line_number":148,"context_line":"            self.facility \u003d kwargs[\u0027facility\u0027]"},{"line_number":149,"context_line":"        self.statsd_client \u003d FakeStatsdClient({"},{"line_number":150,"context_line":"            \u0027log_statsd_host\u0027: \"host\","},{"line_number":151,"context_line":"            \u0027log_statsd_port\u0027: 8125})"},{"line_number":152,"context_line":"        self.thread_locals \u003d None"},{"line_number":153,"context_line":"        self.parent \u003d None"},{"line_number":154,"context_line":"        # ensure the NOTICE level has been named, in case it has not already"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c375d1e_5d640f87","line":151,"in_reply_to":"446972af_7a9762f5","updated":"2024-05-16 22:20:50.000000000","message":"Acknowledged","commit_id":"84cc6123b290506915059484bfd3b5573fc0b8aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class FakeStatsdClient(statsd_client.StatsdClient):"},{"line_number":33,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":34,"context_line":"        super(FakeStatsdClient, self).__init__(*args, **kwargs)"},{"line_number":35,"context_line":"        self.clear()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        # Capture then call parent pubic stat functions"}],"source_content_type":"text/x-python","patch_set":11,"id":"7f6b9509_e0e02fc1","line":34,"updated":"2024-06-18 10:54:15.000000000","message":"this isn\u0027t strictly necessary for this patch but will probably make it easier if new args are added to the StatsdClient constructor","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class FakeStatsdClient(statsd_client.StatsdClient):"},{"line_number":33,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":34,"context_line":"        super(FakeStatsdClient, self).__init__(*args, **kwargs)"},{"line_number":35,"context_line":"        self.clear()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        # Capture then call parent pubic stat functions"}],"source_content_type":"text/x-python","patch_set":11,"id":"bde6022c_819f40f4","line":34,"in_reply_to":"7f6b9509_e0e02fc1","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        self.level \u003d logging.NOTSET"},{"line_number":147,"context_line":"        if \u0027facility\u0027 in kwargs:"},{"line_number":148,"context_line":"            self.facility \u003d kwargs[\u0027facility\u0027]"},{"line_number":149,"context_line":"        self.statsd_client \u003d FakeStatsdClient(\"host\", 8125)"},{"line_number":150,"context_line":"        self.thread_locals \u003d None"},{"line_number":151,"context_line":"        self.parent \u003d None"},{"line_number":152,"context_line":"        # ensure the NOTICE level has been named, in case it has not already"}],"source_content_type":"text/x-python","patch_set":11,"id":"af2d0b91_94459e0e","line":149,"updated":"2024-06-18 10:54:15.000000000","message":"I think we want the host to remain ``None`` to disable the metrics being emitted","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        self.level \u003d logging.NOTSET"},{"line_number":147,"context_line":"        if \u0027facility\u0027 in kwargs:"},{"line_number":148,"context_line":"            self.facility \u003d kwargs[\u0027facility\u0027]"},{"line_number":149,"context_line":"        self.statsd_client \u003d FakeStatsdClient(\"host\", 8125)"},{"line_number":150,"context_line":"        self.thread_locals \u003d None"},{"line_number":151,"context_line":"        self.parent \u003d None"},{"line_number":152,"context_line":"        # ensure the NOTICE level has been named, in case it has not already"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a9f13b3_ed282085","line":149,"in_reply_to":"af2d0b91_94459e0e","updated":"2024-06-18 22:12:11.000000000","message":"Done","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98c39b091532bc877dd32ef3add2cf29a5098e98","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        self.level \u003d logging.NOTSET"},{"line_number":147,"context_line":"        if \u0027facility\u0027 in kwargs:"},{"line_number":148,"context_line":"            self.facility \u003d kwargs[\u0027facility\u0027]"},{"line_number":149,"context_line":"        self.statsd_client \u003d FakeStatsdClient(\"host\", 8125)"},{"line_number":150,"context_line":"        self.thread_locals \u003d None"},{"line_number":151,"context_line":"        self.parent \u003d None"},{"line_number":152,"context_line":"        # ensure the NOTICE level has been named, in case it has not already"}],"source_content_type":"text/x-python","patch_set":11,"id":"2efb0705_c7a78335","line":149,"in_reply_to":"af2d0b91_94459e0e","updated":"2024-06-18 20:24:13.000000000","message":"FWIW, `FakeStatsdClient` is pretty-well neutered between overriding `_open_socket` to return itself and making `sendto` just capture calls.\n\nBut yeah, between patchset 3 and 4, https://github.com/openstack/swift/commit/b447234b landed, so we no longer need to pass a host to ensure the stats client gets instantiated \u0026 attached.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"}],"test/unit/common/test_statsd.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"39f8be5b4bcadd7f72c0ec181be95f120790157b","unresolved":true,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"from swift.common import utils"},{"line_number":32,"context_line":"from swift.common import statsd_client as common_statsd_client"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from test.debug_logger import debug_logger"},{"line_number":35,"context_line":"from test.unit.common.test_utils import MockUdpSocket, reset_logger_state"}],"source_content_type":"text/x-python","patch_set":1,"id":"130b55cb_d02d0679","line":32,"updated":"2024-05-13 16:44:27.000000000","message":"I think \"statsd_client\" would have been a reasonable namespace in this module","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"ab1e9290890d1f07c835acc08d84cb97e94d9498","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"from swift.common import utils"},{"line_number":32,"context_line":"from swift.common import statsd_client as common_statsd_client"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from test.debug_logger import debug_logger"},{"line_number":35,"context_line":"from test.unit.common.test_utils import MockUdpSocket, reset_logger_state"}],"source_content_type":"text/x-python","patch_set":1,"id":"32209c1d_efa644e9","line":32,"in_reply_to":"130b55cb_d02d0679","updated":"2024-05-13 19:36:04.000000000","message":"statsd_client is used as a variable at multiple places here and hence to distinguish statsd_client variable from swift.common.statsd_client, I\u0027ve changed names. I got errors saying \u0027statsd_client used before defining\u0027 but it couldn\u0027t use the imported one somehow. Hence the name common_statsd_client is used to tell that it\u0027s the swift.common.statsd_client one!\n\nIf you feel some other name/ term would be the better fit, I\u0027m happy to change that!","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"from swift.common import utils"},{"line_number":32,"context_line":"from swift.common import statsd_client as common_statsd_client"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from test.debug_logger import debug_logger"},{"line_number":35,"context_line":"from test.unit.common.test_utils import MockUdpSocket, reset_logger_state"}],"source_content_type":"text/x-python","patch_set":1,"id":"940ffe91_44adefc9","line":32,"in_reply_to":"32209c1d_efa644e9","updated":"2024-05-14 11:53:14.000000000","message":"I think you just need to use a different var name than statsd_client now that the imported module has that name - maybe just \u0027client\u0027 - it\u0027s in 3 or 4 tests\n\nhttps://paste.openstack.org/show/bxjA88rqyD1zuHRqGQ1W/","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":59,"context_line":"        self.mock_getaddrinfo \u003d self.getaddrinfo_patcher.start()"},{"line_number":60,"context_line":"        self.addCleanup(self.getaddrinfo_patcher.stop)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def test_get_logger_statsd_client_not_specified(self):"},{"line_number":63,"context_line":"        logger \u003d utils.get_logger({}, \u0027some-name\u0027, log_route\u003d\u0027some-route\u0027)"},{"line_number":64,"context_line":"        # white-box construction validation"},{"line_number":65,"context_line":"        self.assertIsNone(logger.logger.statsd_client)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0b2412b5_0605a72e","line":62,"updated":"2024-05-14 11:53:14.000000000","message":"this seems like a test that belongs in test_logs - it is implicitly asserting that statsd_client is *not* called","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"7642bab0ad28e0e992c54fe84030d6d47e076ce4","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        self.mock_getaddrinfo \u003d self.getaddrinfo_patcher.start()"},{"line_number":60,"context_line":"        self.addCleanup(self.getaddrinfo_patcher.stop)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def test_get_logger_statsd_client_not_specified(self):"},{"line_number":63,"context_line":"        logger \u003d utils.get_logger({}, \u0027some-name\u0027, log_route\u003d\u0027some-route\u0027)"},{"line_number":64,"context_line":"        # white-box construction validation"},{"line_number":65,"context_line":"        self.assertIsNone(logger.logger.statsd_client)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8e26924a_bded441a","line":62,"in_reply_to":"0b2412b5_0605a72e","updated":"2024-05-14 23:42:21.000000000","message":"Have to create test_logs.py and test_config.py and move quite a few tests there. But they are lower priority and I will create jira tasks to keep track of this!","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":65,"context_line":"        self.assertIsNone(logger.logger.statsd_client)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def test_get_logger_statsd_client_defaults(self):"},{"line_number":68,"context_line":"        logger \u003d utils.get_logger({\u0027log_statsd_host\u0027: \u0027some.host.com\u0027},"},{"line_number":69,"context_line":"                                  \u0027some-name\u0027, log_route\u003d\u0027some-route\u0027)"},{"line_number":70,"context_line":"        # white-box construction validation"},{"line_number":71,"context_line":"        self.assertIsInstance(logger.logger.statsd_client,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e49bfe5_d70d8476","line":68,"updated":"2024-05-14 11:53:14.000000000","message":"I understand that we\u0027ve inherited all the tests from test_utils, but it\u0027s now odd that this test module has zero calls to get_statsd_client and lots of calls to get_logger.\n\nWe do still need to verify that get_logger also installs a statsd_client, but we should make some tests now call get_statsd_client","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        self.assertIsNone(logger.logger.statsd_client)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def test_get_logger_statsd_client_defaults(self):"},{"line_number":68,"context_line":"        logger \u003d utils.get_logger({\u0027log_statsd_host\u0027: \u0027some.host.com\u0027},"},{"line_number":69,"context_line":"                                  \u0027some-name\u0027, log_route\u003d\u0027some-route\u0027)"},{"line_number":70,"context_line":"        # white-box construction validation"},{"line_number":71,"context_line":"        self.assertIsInstance(logger.logger.statsd_client,"}],"source_content_type":"text/x-python","patch_set":1,"id":"21bfa869_22ff0205","line":68,"in_reply_to":"4edc6016_0c279aee","updated":"2024-06-04 22:36:08.000000000","message":"I agree. I have a few upcoming jira tasks to move the tests around!","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5c8f8d90b162ee4672724055db702c0607dc05ce","unresolved":true,"context_lines":[{"line_number":65,"context_line":"        self.assertIsNone(logger.logger.statsd_client)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def test_get_logger_statsd_client_defaults(self):"},{"line_number":68,"context_line":"        logger \u003d utils.get_logger({\u0027log_statsd_host\u0027: \u0027some.host.com\u0027},"},{"line_number":69,"context_line":"                                  \u0027some-name\u0027, log_route\u003d\u0027some-route\u0027)"},{"line_number":70,"context_line":"        # white-box construction validation"},{"line_number":71,"context_line":"        self.assertIsInstance(logger.logger.statsd_client,"}],"source_content_type":"text/x-python","patch_set":1,"id":"4edc6016_0c279aee","line":68,"in_reply_to":"7e49bfe5_d70d8476","updated":"2024-05-14 15:25:42.000000000","message":"Looking ahead to the plan for a `utils.get_logger`, `statsd_client.get_statsd_client`, and `utils.logs.get_basic_logger` (or whatever we decide to name that last one), maybe some of these tests should find their way back into `test_utils.py`","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"06c1e95f30ed0aaf616438d9d11c3b560f9afdd2","unresolved":true,"context_lines":[{"line_number":249,"context_line":"        got_sock \u003d statsd_client._open_socket()"},{"line_number":250,"context_line":"        self.assertEqual(got_sock.family, socket.AF_INET6)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def test_bad_hostname_instantiation(self):"},{"line_number":253,"context_line":"        with mock.patch.object("},{"line_number":254,"context_line":"                common_statsd_client.socket, \u0027getaddrinfo\u0027,"},{"line_number":255,"context_line":"                side_effect\u003dcommon_statsd_client.socket.gaierror(\"whoops\")):"}],"source_content_type":"text/x-python","patch_set":1,"id":"0a522814_553555dc","line":252,"updated":"2024-05-14 11:53:14.000000000","message":"this test fails OMM (macos)\n\nAlso, for similar reason:\n\nTestStatsdLogging.test_ipv6_instantiation_and_socket_creation\nTestStatsdLogging.test_ipv4_or_ipv6_hostname_defaults_to_ipv4\n\n```\ntest_statsd.py::TestStatsdLogging::test_bad_hostname_instantiation FAILED [100%]\ntest/unit/common/test_statsd.py:251 (TestStatsdLogging.test_bad_hostname_instantiation)\nself \u003d \u003ctest.unit.common.test_statsd.TestStatsdLogging testMethod\u003dtest_bad_hostname_instantiation\u003e\n\n    def test_bad_hostname_instantiation(self):\n        with mock.patch.object(\n                common_statsd_client.socket, \u0027getaddrinfo\u0027,\n                side_effect\u003dcommon_statsd_client.socket.gaierror(\"whoops\")):\n\u003e           logger \u003d utils.get_logger({\n                \u0027log_statsd_host\u0027: \u0027i-am-not-a-hostname-or-ip\u0027,\n                \u0027log_statsd_port\u0027: \u00279876\u0027,\n            }, \u0027some-name\u0027, log_route\u003d\u0027some-route\u0027)\n\ntest_statsd.py:256: \n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \n../../../swift/common/utils/logs.py:709: in get_logger\n    handler \u003d ThreadSafeSysLogHandler(facility\u003dfacility)\n/Users/acoles/.pyenv/versions/3.11.1/lib/python3.11/logging/handlers.py:867: in __init__\n    self.createSocket()\n/Users/acoles/.pyenv/versions/3.11.1/lib/python3.11/logging/handlers.py:919: in createSocket\n    ress \u003d socket.getaddrinfo(host, port, 0, socktype)\n/Users/acoles/.pyenv/versions/swift-3.11.1/lib/python3.11/site-packages/mock/mock.py:1149: in __call__\n    return _mock_self._mock_call(*args, **kwargs)\n/Users/acoles/.pyenv/versions/swift-3.11.1/lib/python3.11/site-packages/mock/mock.py:1153: in _mock_call\n    return _mock_self._execute_mock_call(*args, **kwargs)\n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \n\n_mock_self \u003d \u003cMagicMock name\u003d\u0027getaddrinfo\u0027 id\u003d\u00274398469136\u0027\u003e\nargs \u003d (\u0027localhost\u0027, 514, 0, \u003cSocketKind.SOCK_DGRAM: 2\u003e), kwargs \u003d {}\nself \u003d \u003cMagicMock name\u003d\u0027getaddrinfo\u0027 id\u003d\u00274398469136\u0027\u003e\neffect \u003d gaierror(\u0027whoops\u0027)\n\n    def _execute_mock_call(_mock_self, *args, **kwargs):\n        self \u003d _mock_self\n        # separate from _increment_mock_call so that awaited functions are\n        # executed separately from their call, also AsyncMock overrides this method\n    \n        effect \u003d self.side_effect\n        if effect is not None:\n            if _is_exception(effect):\n\u003e               raise effect\nE               socket.gaierror: whoops\n\n/Users/acoles/.pyenv/versions/swift-3.11.1/lib/python3.11/site-packages/mock/mock.py:1210: gaierror\n\n\n```\n\nI think the problem is that the test patches common_statsd_client.socket.getaddrinfo to raise an exception, but the same method is used when the logger creates a ThreadSafeSysLogHandler, so that blows up.\n\nMaster does pass OMM. How? on master the test patches utils.socket which is actually ``from eventlet.green import socket`` so that\u0027s a different module! And in fact the mock does nothing w.r.t. the test on master since statsd_client was broken out of utils.__init__ !\n\nIDK why the test passes on other machines.\n\nApart from the test issue, it makes me wonder if we slipped up when we created statsd_client.py and used ``import socket`` rather than ``from eventlet.green import socket`` ??","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5c8f8d90b162ee4672724055db702c0607dc05ce","unresolved":true,"context_lines":[{"line_number":249,"context_line":"        got_sock \u003d statsd_client._open_socket()"},{"line_number":250,"context_line":"        self.assertEqual(got_sock.family, socket.AF_INET6)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def test_bad_hostname_instantiation(self):"},{"line_number":253,"context_line":"        with mock.patch.object("},{"line_number":254,"context_line":"                common_statsd_client.socket, \u0027getaddrinfo\u0027,"},{"line_number":255,"context_line":"                side_effect\u003dcommon_statsd_client.socket.gaierror(\"whoops\")):"}],"source_content_type":"text/x-python","patch_set":1,"id":"58cf8c30_705980e3","line":252,"in_reply_to":"0a522814_553555dc","updated":"2024-05-14 15:25:42.000000000","message":"\u003e Apart from the test issue, it makes me wonder if we slipped up when we created statsd_client.py and used `import socket` rather than `from eventlet.green import socket` ??\n\nFWIW, in a real process, `socket` should be monkey-patched, so they\u0027d be the same module -- so (I think) it\u0027s purely a testing issue.","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        got_sock \u003d statsd_client._open_socket()"},{"line_number":250,"context_line":"        self.assertEqual(got_sock.family, socket.AF_INET6)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def test_bad_hostname_instantiation(self):"},{"line_number":253,"context_line":"        with mock.patch.object("},{"line_number":254,"context_line":"                common_statsd_client.socket, \u0027getaddrinfo\u0027,"},{"line_number":255,"context_line":"                side_effect\u003dcommon_statsd_client.socket.gaierror(\"whoops\")):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d2c43606_3945b4ca","line":252,"in_reply_to":"393496a2_992acd01","updated":"2024-06-04 22:36:08.000000000","message":"Fixed with https://review.opendev.org/c/openstack/swift/+/919752 and resolved","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f3f701d5e7ace126b762edff4649cd356e64b8a2","unresolved":true,"context_lines":[{"line_number":249,"context_line":"        got_sock \u003d statsd_client._open_socket()"},{"line_number":250,"context_line":"        self.assertEqual(got_sock.family, socket.AF_INET6)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def test_bad_hostname_instantiation(self):"},{"line_number":253,"context_line":"        with mock.patch.object("},{"line_number":254,"context_line":"                common_statsd_client.socket, \u0027getaddrinfo\u0027,"},{"line_number":255,"context_line":"                side_effect\u003dcommon_statsd_client.socket.gaierror(\"whoops\")):"}],"source_content_type":"text/x-python","patch_set":1,"id":"c746de2a_88c5f01e","line":252,"in_reply_to":"58cf8c30_705980e3","updated":"2024-05-15 15:53:35.000000000","message":"I agree - daemon/wsgi call monkey patch before we create a StatsdClient and common.statsd_client just imports the module (so when it later references socket.socket it\u0027ll be patched)\n\nI\u0027m not sure I understand why this misplaced patching seems to be working - i have to assume OMM (in a vsaio) tests are doing some monkey patching and by the time the patcher runs the statsd_client.socket module *is* the patched utils.socket module?\n\nWe should still fix the import in statsd_client to be more explicit:\n\nhttps://review.opendev.org/c/openstack/swift/+/919752\n\n^ maybe that accidently helps the test work more consistently cross platform as well.","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"50c6a46828edbcde00e1e9d0efb523cea2965eff","unresolved":true,"context_lines":[{"line_number":249,"context_line":"        got_sock \u003d statsd_client._open_socket()"},{"line_number":250,"context_line":"        self.assertEqual(got_sock.family, socket.AF_INET6)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def test_bad_hostname_instantiation(self):"},{"line_number":253,"context_line":"        with mock.patch.object("},{"line_number":254,"context_line":"                common_statsd_client.socket, \u0027getaddrinfo\u0027,"},{"line_number":255,"context_line":"                side_effect\u003dcommon_statsd_client.socket.gaierror(\"whoops\")):"}],"source_content_type":"text/x-python","patch_set":1,"id":"393496a2_992acd01","line":252,"in_reply_to":"c746de2a_88c5f01e","updated":"2024-05-16 15:21:51.000000000","message":"Alistair, this reminds me a lot of https://review.opendev.org/c/openstack/swift/+/780939 ...","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"}],"test/unit/common/test_statsd_client.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98c39b091532bc877dd32ef3add2cf29a5098e98","unresolved":true,"context_lines":[{"line_number":100,"context_line":"            mock.call(),"},{"line_number":101,"context_line":"            mock.call().sendto(b\u0027tunafish:1|c\u0027, (\u0027myhost1\u0027, 1235)),"},{"line_number":102,"context_line":"            mock.call().close(),"},{"line_number":103,"context_line":"        ])"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def test_init_host_is_none(self):"},{"line_number":106,"context_line":"        client \u003d StatsdClient(None, None)"}],"source_content_type":"text/x-python","patch_set":11,"id":"240ceca0_a08da87c","line":103,"updated":"2024-06-18 20:24:13.000000000","message":"I think I\u0027d prefer to see a separate `test_get_statsd_client_with_host` rather than have this test all interleaved.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            mock.call(),"},{"line_number":101,"context_line":"            mock.call().sendto(b\u0027tunafish:1|c\u0027, (\u0027myhost1\u0027, 1235)),"},{"line_number":102,"context_line":"            mock.call().close(),"},{"line_number":103,"context_line":"        ])"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def test_init_host_is_none(self):"},{"line_number":106,"context_line":"        client \u003d StatsdClient(None, None)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ef90c4b9_fb766b35","line":103,"in_reply_to":"240ceca0_a08da87c","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98c39b091532bc877dd32ef3add2cf29a5098e98","unresolved":true,"context_lines":[{"line_number":114,"context_line":"        with mock.patch.object(client1, \u0027_open_socket\u0027) as mock_open1:"},{"line_number":115,"context_line":"            self.assertIsNone(client1.increment(\u0027tunafish\u0027))"},{"line_number":116,"context_line":"        self.assertFalse(mock_open.mock_calls)"},{"line_number":117,"context_line":"        self.assertFalse(mock_open1.mock_calls)"},{"line_number":118,"context_line":"        self.assertFalse(self.getaddrinfo_calls)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"8263fff9_5c812385","line":117,"updated":"2024-06-18 20:24:13.000000000","message":"Similar here.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        with mock.patch.object(client1, \u0027_open_socket\u0027) as mock_open1:"},{"line_number":115,"context_line":"            self.assertIsNone(client1.increment(\u0027tunafish\u0027))"},{"line_number":116,"context_line":"        self.assertFalse(mock_open.mock_calls)"},{"line_number":117,"context_line":"        self.assertFalse(mock_open1.mock_calls)"},{"line_number":118,"context_line":"        self.assertFalse(self.getaddrinfo_calls)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"84822bb3_8d307945","line":117,"in_reply_to":"8263fff9_5c812385","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4042e3a22b96885b9769834f78923ea720159ed0","unresolved":true,"context_lines":[{"line_number":786,"context_line":"        self.logger \u003d debug_logger()"},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"    def test_get_statsd_client_defaults(self):"},{"line_number":789,"context_line":"        # legacy options..."},{"line_number":790,"context_line":"        client \u003d statsd_client.get_statsd_client({})"},{"line_number":791,"context_line":"        self.assertIsInstance(client, StatsdClient)"},{"line_number":792,"context_line":"        self.assertIsNone(client._host)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5e19e597_99b3e4e8","line":789,"updated":"2024-06-19 11:22:52.000000000","message":"nit: comment not relevant, my mistake","commit_id":"7b7d3f536037fe4acd09baf18db5c3658cabdb85"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e649f6693c31d259f0bc41bb21b282736ed28d4","unresolved":false,"context_lines":[{"line_number":786,"context_line":"        self.logger \u003d debug_logger()"},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"    def test_get_statsd_client_defaults(self):"},{"line_number":789,"context_line":"        # legacy options..."},{"line_number":790,"context_line":"        client \u003d statsd_client.get_statsd_client({})"},{"line_number":791,"context_line":"        self.assertIsInstance(client, StatsdClient)"},{"line_number":792,"context_line":"        self.assertIsNone(client._host)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1d798e47_2f057912","line":789,"in_reply_to":"5e19e597_99b3e4e8","updated":"2024-06-24 11:09:47.000000000","message":"Done","commit_id":"7b7d3f536037fe4acd09baf18db5c3658cabdb85"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"39f8be5b4bcadd7f72c0ec181be95f120790157b","unresolved":true,"context_lines":[{"line_number":2282,"context_line":"        class _FakeStatsdClientConf(StatsdClientConf):"},{"line_number":2283,"context_line":"            def __init__(self, conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":2284,"context_line":"                StatsdClientConf.__init__(self, conf,"},{"line_number":2285,"context_line":"                                          tail_prefix\u003dtail_prefix)"},{"line_number":2286,"context_line":""},{"line_number":2287,"context_line":"        # defaults..."},{"line_number":2288,"context_line":"        conf_sections \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"ee5758ef_597210dc","line":2285,"updated":"2024-05-13 16:44:27.000000000","message":"i don\u0027t understand what this class is doing...","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"ab1e9290890d1f07c835acc08d84cb97e94d9498","unresolved":false,"context_lines":[{"line_number":2282,"context_line":"        class _FakeStatsdClientConf(StatsdClientConf):"},{"line_number":2283,"context_line":"            def __init__(self, conf\u003dNone, tail_prefix\u003d\u0027\u0027, logger\u003dNone):"},{"line_number":2284,"context_line":"                StatsdClientConf.__init__(self, conf,"},{"line_number":2285,"context_line":"                                          tail_prefix\u003dtail_prefix)"},{"line_number":2286,"context_line":""},{"line_number":2287,"context_line":"        # defaults..."},{"line_number":2288,"context_line":"        conf_sections \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"f15a5afd_eb373158","line":2285,"in_reply_to":"ee5758ef_597210dc","updated":"2024-05-13 19:36:04.000000000","message":"Makes it easier to patch statsd_client and to assert host, port and tail prefix!","commit_id":"53dd9b86dbeec5bf71df7a573f3e11b4c355cffb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"099af8624dae360e53aed77520802cfe5b1bb402","unresolved":true,"context_lines":[{"line_number":2302,"context_line":"        self.assertEqual(app.logger.logger.statsd_client.port,"},{"line_number":2303,"context_line":"                         8125)"},{"line_number":2304,"context_line":"        self.assertEqual(app.logger.logger.statsd_client.prefix,"},{"line_number":2305,"context_line":"                         \u0027proxy-server\u0027)"},{"line_number":2306,"context_line":""},{"line_number":2307,"context_line":"        conf_sections \u003d \"\"\""},{"line_number":2308,"context_line":"        [DEFAULT]"}],"source_content_type":"text/x-python","patch_set":5,"id":"dc394be1_66bbac48","line":2305,"updated":"2024-06-04 00:25:10.000000000","message":"Probably want at least a\n```\nself.assertIsNotNone(app.logger.logger.statsd_client.logger)\n```\nthough something stronger like\n```\nself.assertIs(app.logger.logger.statsd_client.logger, app.logger.logger)\n```\nwould maybe be nice. (I *think* I got that right? Or will it just be `app.logger`?)","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"c894015fd416ee3fec8cce1cccce0d2da52aae07","unresolved":false,"context_lines":[{"line_number":2302,"context_line":"        self.assertEqual(app.logger.logger.statsd_client.port,"},{"line_number":2303,"context_line":"                         8125)"},{"line_number":2304,"context_line":"        self.assertEqual(app.logger.logger.statsd_client.prefix,"},{"line_number":2305,"context_line":"                         \u0027proxy-server\u0027)"},{"line_number":2306,"context_line":""},{"line_number":2307,"context_line":"        conf_sections \u003d \"\"\""},{"line_number":2308,"context_line":"        [DEFAULT]"}],"source_content_type":"text/x-python","patch_set":5,"id":"97605c60_123b1e69","line":2305,"in_reply_to":"dc394be1_66bbac48","updated":"2024-06-04 22:36:08.000000000","message":"Yep, its app.logger.logger. Added this!","commit_id":"8b6ef411cf44da1d62b776a4909f6be510803a85"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":2293,"context_line":"        \"\"\" % self.tempdir"},{"line_number":2294,"context_line":"        conf_path \u003d self._write_conf(dedent(conf_sections))"},{"line_number":2295,"context_line":""},{"line_number":2296,"context_line":"        app \u003d loadapp(conf_path, allow_modify_pipeline\u003dFalse)"},{"line_number":2297,"context_line":"        # logger name is hard-wired \u0027proxy-server\u0027"},{"line_number":2298,"context_line":"        self.assertEqual(\u0027proxy-server\u0027, app.logger.name)"},{"line_number":2299,"context_line":"        self.assertEqual(\u0027swift\u0027, app.logger.server)"}],"source_content_type":"text/x-python","patch_set":11,"id":"067c6914_448756dc","line":2296,"updated":"2024-06-18 10:54:15.000000000","message":"why is it no longer necessary to mock StatsdClient?","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"98c39b091532bc877dd32ef3add2cf29a5098e98","unresolved":true,"context_lines":[{"line_number":2293,"context_line":"        \"\"\" % self.tempdir"},{"line_number":2294,"context_line":"        conf_path \u003d self._write_conf(dedent(conf_sections))"},{"line_number":2295,"context_line":""},{"line_number":2296,"context_line":"        app \u003d loadapp(conf_path, allow_modify_pipeline\u003dFalse)"},{"line_number":2297,"context_line":"        # logger name is hard-wired \u0027proxy-server\u0027"},{"line_number":2298,"context_line":"        self.assertEqual(\u0027proxy-server\u0027, app.logger.name)"},{"line_number":2299,"context_line":"        self.assertEqual(\u0027swift\u0027, app.logger.server)"}],"source_content_type":"text/x-python","patch_set":11,"id":"baff11f0_de381e9e","line":2296,"in_reply_to":"067c6914_448756dc","updated":"2024-06-18 20:24:13.000000000","message":"I think it\u0027s a question of whether we want to make assertions on the call to instantiate a `StatsdClient`, or on the properties of the instance created.","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":2293,"context_line":"        \"\"\" % self.tempdir"},{"line_number":2294,"context_line":"        conf_path \u003d self._write_conf(dedent(conf_sections))"},{"line_number":2295,"context_line":""},{"line_number":2296,"context_line":"        app \u003d loadapp(conf_path, allow_modify_pipeline\u003dFalse)"},{"line_number":2297,"context_line":"        # logger name is hard-wired \u0027proxy-server\u0027"},{"line_number":2298,"context_line":"        self.assertEqual(\u0027proxy-server\u0027, app.logger.name)"},{"line_number":2299,"context_line":"        self.assertEqual(\u0027swift\u0027, app.logger.server)"}],"source_content_type":"text/x-python","patch_set":11,"id":"b4c48c82_621f4bd9","line":2296,"in_reply_to":"baff11f0_de381e9e","updated":"2024-06-18 22:12:11.000000000","message":"I got a few errors with mocking but I\u0027ve resolved them and adding mocking back in the next patch","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":2302,"context_line":"        self.assertEqual(app.logger.logger.statsd_client.port,"},{"line_number":2303,"context_line":"                         8125)"},{"line_number":2304,"context_line":"        self.assertEqual(app.logger.logger.statsd_client._prefix,"},{"line_number":2305,"context_line":"                         \u0027proxy-server.\u0027)"},{"line_number":2306,"context_line":""},{"line_number":2307,"context_line":"        conf_sections \u003d \"\"\""},{"line_number":2308,"context_line":"        [DEFAULT]"}],"source_content_type":"text/x-python","patch_set":11,"id":"e3fed19b_2014328b","line":2305,"updated":"2024-06-18 10:54:15.000000000","message":"with the latest patchset (StatsdClient constructor is unchanged) maybe these assertions didn\u0027t need to change??","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":2302,"context_line":"        self.assertEqual(app.logger.logger.statsd_client.port,"},{"line_number":2303,"context_line":"                         8125)"},{"line_number":2304,"context_line":"        self.assertEqual(app.logger.logger.statsd_client._prefix,"},{"line_number":2305,"context_line":"                         \u0027proxy-server.\u0027)"},{"line_number":2306,"context_line":""},{"line_number":2307,"context_line":"        conf_sections \u003d \"\"\""},{"line_number":2308,"context_line":"        [DEFAULT]"}],"source_content_type":"text/x-python","patch_set":11,"id":"d12f3226_b07361c7","line":2305,"in_reply_to":"e3fed19b_2014328b","updated":"2024-06-18 22:12:11.000000000","message":"Done","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":2318,"context_line":"        \"\"\" % self.tempdir"},{"line_number":2319,"context_line":"        conf_path \u003d self._write_conf(dedent(conf_sections))"},{"line_number":2320,"context_line":""},{"line_number":2321,"context_line":"        app \u003d loadapp(conf_path, allow_modify_pipeline\u003dFalse)"},{"line_number":2322,"context_line":"        # logger name is hard-wired \u0027proxy-server\u0027"},{"line_number":2323,"context_line":"        self.assertEqual(\u0027proxy-server\u0027, app.logger.name)"},{"line_number":2324,"context_line":"        # server is defined by log_name option"}],"source_content_type":"text/x-python","patch_set":11,"id":"72a28f4c_6471831e","line":2321,"updated":"2024-06-18 10:54:15.000000000","message":"ditto re comment above","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":2318,"context_line":"        \"\"\" % self.tempdir"},{"line_number":2319,"context_line":"        conf_path \u003d self._write_conf(dedent(conf_sections))"},{"line_number":2320,"context_line":""},{"line_number":2321,"context_line":"        app \u003d loadapp(conf_path, allow_modify_pipeline\u003dFalse)"},{"line_number":2322,"context_line":"        # logger name is hard-wired \u0027proxy-server\u0027"},{"line_number":2323,"context_line":"        self.assertEqual(\u0027proxy-server\u0027, app.logger.name)"},{"line_number":2324,"context_line":"        # server is defined by log_name option"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a53b20d_2055daeb","line":2321,"in_reply_to":"72a28f4c_6471831e","updated":"2024-06-18 22:12:11.000000000","message":"Done","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d49cedd0109ecb4a76eff1be016f06e9fa46a68d","unresolved":true,"context_lines":[{"line_number":2330,"context_line":"        self.assertEqual(app.logger.logger.statsd_client._prefix,"},{"line_number":2331,"context_line":"                         \u0027proxy-server.\u0027)"},{"line_number":2332,"context_line":"        self.assertIs(app.logger.logger.statsd_client.logger,"},{"line_number":2333,"context_line":"                      app.logger.logger)"},{"line_number":2334,"context_line":""},{"line_number":2335,"context_line":""},{"line_number":2336,"context_line":"class TestProxyServerConfigStringLoading(TestProxyServerConfigLoading):"}],"source_content_type":"text/x-python","patch_set":11,"id":"a37ef7a5_37297e5f","line":2333,"updated":"2024-06-18 10:54:15.000000000","message":"ditto","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":35790,"name":"Shreeya Deshpande","email":"shreeyad@nvidia.com","username":"shreeyad"},"change_message_id":"57b6449b4c9883335b2dd5d34995b45a021f7186","unresolved":false,"context_lines":[{"line_number":2330,"context_line":"        self.assertEqual(app.logger.logger.statsd_client._prefix,"},{"line_number":2331,"context_line":"                         \u0027proxy-server.\u0027)"},{"line_number":2332,"context_line":"        self.assertIs(app.logger.logger.statsd_client.logger,"},{"line_number":2333,"context_line":"                      app.logger.logger)"},{"line_number":2334,"context_line":""},{"line_number":2335,"context_line":""},{"line_number":2336,"context_line":"class TestProxyServerConfigStringLoading(TestProxyServerConfigLoading):"}],"source_content_type":"text/x-python","patch_set":11,"id":"9781fb6f_83ca4a23","line":2333,"in_reply_to":"a37ef7a5_37297e5f","updated":"2024-06-18 22:12:11.000000000","message":"Acknowledged","commit_id":"46a8a5b371109fb746be0cb8981c81dcd885170a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e649f6693c31d259f0bc41bb21b282736ed28d4","unresolved":true,"context_lines":[{"line_number":2300,"context_line":"        self.assertEqual(\u0027swift\u0027, app.logger.server)"},{"line_number":2301,"context_line":"        mock_statsd.assert_called_once_with("},{"line_number":2302,"context_line":"            \u0027example.com\u0027, 8125, base_prefix\u003d\u0027\u0027, tail_prefix\u003d\u0027proxy-server\u0027,"},{"line_number":2303,"context_line":"            default_sample_rate\u003d1.0, sample_rate_factor\u003d1.0,"},{"line_number":2304,"context_line":"            logger\u003dapp.logger.logger)"},{"line_number":2305,"context_line":""},{"line_number":2306,"context_line":"        conf_sections \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"d63b58e5_cd92eb98","line":2303,"updated":"2024-06-24 11:09:47.000000000","message":"ok, ``get_statsd_client`` explicitly spells out the named args where previously the call from logs.py didn\u0027t","commit_id":"0c58afc7ee23fd5e5cca19318e3f055dcc79e021"}]}