)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"51c2ca0e_993dada1","updated":"2025-07-14 17:05:42.000000000","message":"I think we should try to get some tracing primitives in place as part of the remove-eventlet effort\n\n1) I think most of the tracing libs will be happier with either threads or async than with eventlet\n2) I think having the ability to instrument code that we\u0027re refactoring will give us confidence we\u0027re not introducing a performance regression (like some kind of serialization/blocking)\n\nI also think 954379: add minimal trace to reconstructor get_hashes | https://review.opendev.org/c/openstack/swift/+/954379 would be slightly easier (but look mostly the same same) if we already had otel tracing available in master.\n\nI don\u0027t think we should try to carry/merge all of 899397: request tracing: Add Otel request tracing support (decorator) | https://review.opendev.org/c/openstack/swift/+/899397 as a pre-req to getting some throw away debug timing information out of the reconstructor/get_hashes.  So this is interesting, but not in the critical path right now.","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"}],"swift/obj/diskfile.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":1195,"context_line":"            def hexdigest(self):"},{"line_number":1196,"context_line":"                return self.md5.hexdigest()"},{"line_number":1197,"context_line":"        hashes \u003d defaultdict(shim)"},{"line_number":1198,"context_line":"        with new_trace_span(env, \"_hash_suffix_dir\"):"},{"line_number":1199,"context_line":"            try:"},{"line_number":1200,"context_line":"                path_contents \u003d sorted(os.listdir(path))"},{"line_number":1201,"context_line":"            except OSError as err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"c80f5070_a4b50879","line":1198,"updated":"2025-07-14 17:05:42.000000000","message":"this isn\u0027t a \"hash\" of the suffix dir - it\u0027s just a \"list\"\n\nthe \"hash\" happens based on the io done in cleanup_ondisk_files - which isn\u0027t part of this span","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":1212,"context_line":"                    partition_path \u003d dirname(path)"},{"line_number":1213,"context_line":"                    objects_path \u003d dirname(partition_path)"},{"line_number":1214,"context_line":"                    device_path \u003d dirname(objects_path)"},{"line_number":1215,"context_line":"                    if err.errno \u003d\u003d errno.ENOTDIR:"},{"line_number":1216,"context_line":"                        # The made-up filename is so that the eventual"},{"line_number":1217,"context_line":"                        # dirpath() will result in this object directory that"},{"line_number":1218,"context_line":"                        # we care about. Some failures will result in an"}],"source_content_type":"text/x-python","patch_set":1,"id":"0b116b68_4158c034","line":1215,"updated":"2025-07-14 17:05:42.000000000","message":"honestly I\u0027m not sure it\u0027s fair to include this error handling as part of the \"cleanup\" span","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":1240,"context_line":"                                device_path, hsh_path)"},{"line_number":1241,"context_line":"                            orig_path \u003d path"},{"line_number":1242,"context_line":"                            msg \u003d (f\u0027Quarantined {orig_path} to {quar_path} \u0027"},{"line_number":1243,"context_line":"                                   \u0027because it could not be listed\u0027)"},{"line_number":1244,"context_line":"                        logging.exception(msg)"},{"line_number":1245,"context_line":"                        trace_add(\"quarantined\", msg, env)"},{"line_number":1246,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"80e8ac39_f89a7084","line":1243,"updated":"2025-07-14 17:05:42.000000000","message":"this is getting indented pretty far\n\n1) is it possible to open/close a span explicitly w/o a context-manager/extra-indention\n2) if we put the new_trace_span ctx IN the try block (so it just wraps cleanup) it\u0027d a) be more accurate/obvious b) not indent all this error handling\n3) we could always method extract if the indention gets too deep (this is too deep for me)","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":1260,"context_line":"            for key in (k for k in (\u0027meta_info\u0027, \u0027ts_info\u0027)"},{"line_number":1261,"context_line":"                        if k in ondisk_info):"},{"line_number":1262,"context_line":"                info \u003d ondisk_info[key]"},{"line_number":1263,"context_line":"                hashes[None].update(info[\u0027timestamp\u0027].internal + info[\u0027ext\u0027])"},{"line_number":1264,"context_line":""},{"line_number":1265,"context_line":"            # delegate to subclass for data file related updates..."},{"line_number":1266,"context_line":"            with new_trace_span(env, \"update_suffix_hashes\"):"}],"source_content_type":"text/x-python","patch_set":1,"id":"75576c85_c2db97ce","line":1263,"updated":"2025-07-14 17:05:42.000000000","message":"this is also CPU - and probably also not what\u0027s slowing this method down","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":1264,"context_line":""},{"line_number":1265,"context_line":"            # delegate to subclass for data file related updates..."},{"line_number":1266,"context_line":"            with new_trace_span(env, \"update_suffix_hashes\"):"},{"line_number":1267,"context_line":"                self._update_suffix_hashes(hashes, ondisk_info)"},{"line_number":1268,"context_line":""},{"line_number":1269,"context_line":"            if \u0027ctype_info\u0027 in ondisk_info:"},{"line_number":1270,"context_line":"                # We have a distinct content-type timestamp so update the"}],"source_content_type":"text/x-python","patch_set":1,"id":"5460a3c1_a0bcc96a","line":1267,"updated":"2025-07-14 17:05:42.000000000","message":"I like this part of the diff better - tighter context\n\nIMHO *this* is the part of this method that is doing the \"hash\" - and at this point it should just be CPU","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":1274,"context_line":"                # avoid any confusion."},{"line_number":1275,"context_line":"                info \u003d ondisk_info[\u0027ctype_info\u0027]"},{"line_number":1276,"context_line":"                hashes[None].update(info[\u0027ctype_timestamp\u0027].internal"},{"line_number":1277,"context_line":"                                    + \u0027_ctype\u0027)"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        with new_trace_span(env, \"rmdir\"):"},{"line_number":1280,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e56e5be_2f93e4a3","line":1277,"updated":"2025-07-14 17:05:42.000000000","message":"it\u0027s actually a little \"weird\" that all this hashing happens in this method - I guess it\u0027s \"common\" to both replicated/ec hashsing - but I think from a logical flow i\u0027d be better to have a single method:\n\nself._update_suffix_hashes(hashes, ondisk_files)\n\nthat does all the tombstones \u0026 ctypes (common) as well as farming out to `_update_suffix_hashes_datafiles(hashes, ondiskf_files)` or something so that subclasses can differentiate.\n\nThen we could get a single span over *all* the CPU and not just some of it.","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":1314,"context_line":"        hashes.pop(\u0027valid\u0027, None)"},{"line_number":1315,"context_line":"        return hashed, hashes"},{"line_number":1316,"context_line":""},{"line_number":1317,"context_line":"    @trace_function"},{"line_number":1318,"context_line":"    def __get_hashes(self, device, partition, policy, recalculate\u003dNone,"},{"line_number":1319,"context_line":"                     do_listdir\u003dFalse, env\u003dNone):"},{"line_number":1320,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f010519_4cbc0b2c","line":1317,"updated":"2025-07-14 17:05:42.000000000","message":"i can\u0027t figure out why *this method alone* gets the full \"trace_function\" treatment and everyone else just does sub-spans...\n\nI\u0027m guessing understand from a grouping/organizational perspective would benifit from having all traced methods be wrapped up in their own span...","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"da1f9a57d164e85dbc77a9fd6e864d73ee13455c","unresolved":true,"context_lines":[{"line_number":3220,"context_line":""},{"line_number":3221,"context_line":"        :param path: full path to directory"},{"line_number":3222,"context_line":"        :param policy: storage policy used to store the files"},{"line_number":3223,"context_line":"        :param env: Request env containing the tracing context"},{"line_number":3224,"context_line":"        :raises PathNotDir: if given path is not a valid directory"},{"line_number":3225,"context_line":"        :raises OSError: for non-ENOTDIR errors"},{"line_number":3226,"context_line":"        :returns: md5 of files in suffix"}],"source_content_type":"text/x-python","patch_set":1,"id":"67b32f00_d70deaad","line":3223,"updated":"2025-07-14 17:05:42.000000000","message":"this might be a scenario where i\u0027d be easier/better to use a thread local instead of \"just\" updating the signature for the purpose of tracing.\n\nOTOH, maybe having \"two ways to do it\" isn\u0027t as good as \"update all the signatures\"\n\nif we *are* going to \"update all the signatures\" I\u0027d prefer to name the new param \"ctx\" so that we can future proof ourselves against other \"per req/thread context\" objects needing additional plumbing.","commit_id":"762d59b5864c69b31a87202b0fa9b91a69eef8d8"}]}