)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3e4520f435dfc71e940806ed365c94244dcf7dc8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d301ad7e_e81cd84a","updated":"2023-04-26 16:16:46.000000000","message":"I have so many questions 😊\n\n* It seems like ceilometer generally expects accounts of the form `\u003creseller_prefix\u003e\u003cuuid\u003e`, and this patch helps when there isn\u0027t an expected reseller prefix -- but what if there **is** a valid reseller prefix, and the remainder is not a uuid?\n* What\u0027s the multi-auth story like for ceilometer? If I\u0027ve got two or more auth systems configured, do we just pick one reseller prefix to get accurate metrics and punt everyone else to this path? Or do we configure multiple ceilometer middlewares, each with different reseller prefix?\n* What\u0027s the entity we\u0027re trying to identify with a \"resource ID\"? It seems like we get very different cardinalities for number of IDs depending on whether there is or is not a valid reseller prefix.\n* What\u0027s the upgrade story? It seems like there may be an assumption that no one is looking at these metrics, and so they\u0027re safe to change -- in which case, why are we emitting them at all? Could we just do an early return?","commit_id":"ee583528d3277ab36d910c9b63dc816870da409e"},{"author":{"_account_id":26274,"name":"Florent Vennetier","email":"florent.vennetier@ovhcloud.com","username":"fvennetier"},"change_message_id":"572f7156109b2d22c2813f9db2607f5921e72821","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"751da066_9f8c4d5c","in_reply_to":"d301ad7e_e81cd84a","updated":"2023-05-03 14:57:51.000000000","message":"Thanks for your review!\n\n1. Indeed, I did not address this case.\n2. ceilometermiddleware configuration allows only one `reseller_prefix`. In case of multi auth systems, we need to put multiple instances in the pipeline.\n3. From my understanding, we are trying to identify the account. I don\u0027t know what ceilometer does with resources looking like paths.\n4. You are right, we should implement some config option to ignore resources that do not match the `reseller_prefix`. That would solve my initial problem.","commit_id":"ee583528d3277ab36d910c9b63dc816870da409e"}],"ceilometermiddleware/swift.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3e4520f435dfc71e940806ed365c94244dcf7dc8","unresolved":true,"context_lines":[{"line_number":345,"context_line":"                    header.upper())"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        # build object store details"},{"line_number":348,"context_line":"        resource_id \u003d account.partition(self.reseller_prefix)[2]"},{"line_number":349,"context_line":"        if not resource_id:"},{"line_number":350,"context_line":"            resource_id \u003d str(uuid.uuid5(uuid.NAMESPACE_URL, path))"},{"line_number":351,"context_line":"        target \u003d cadf_resource.Resource("}],"source_content_type":"text/x-python","patch_set":2,"id":"aeb8ad49_744c2982","line":348,"updated":"2023-04-26 16:16:46.000000000","message":"What if the account *does* have a matching reseller prefix and the remainder is **not** a UUID? This is common with some non-Keystone auth systems. Won\u0027t we still trip the warning?\n\nSeparately, I\u0027m not sure `partition()` is the right tool here, though that predates the change. I\u0027d be concerned about a cluster that has multiple auth systems -- if one of them uses an auth prefix that is a substring of another auth prefix (e.g., `AUTH_` and `V2AUTH_` or something), this may not do the right/expected thing.","commit_id":"ee583528d3277ab36d910c9b63dc816870da409e"},{"author":{"_account_id":26274,"name":"Florent Vennetier","email":"florent.vennetier@ovhcloud.com","username":"fvennetier"},"change_message_id":"572f7156109b2d22c2813f9db2607f5921e72821","unresolved":true,"context_lines":[{"line_number":345,"context_line":"                    header.upper())"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        # build object store details"},{"line_number":348,"context_line":"        resource_id \u003d account.partition(self.reseller_prefix)[2]"},{"line_number":349,"context_line":"        if not resource_id:"},{"line_number":350,"context_line":"            resource_id \u003d str(uuid.uuid5(uuid.NAMESPACE_URL, path))"},{"line_number":351,"context_line":"        target \u003d cadf_resource.Resource("}],"source_content_type":"text/x-python","patch_set":2,"id":"61f11c1f_2e966388","line":348,"in_reply_to":"aeb8ad49_744c2982","updated":"2023-05-03 14:57:51.000000000","message":"Indeed, the account may not be a UUID. Tempauth\u0027s account are usually not UUIDs... I have not answer to that problem.\n\nCeilometermiddleware seems to handle only one `reseller_prefix`. If we want to handle several, we need to put several instance of this middleware in the pipeline. And yes, we should probably emit events only if the `reseller_prefix` matches.","commit_id":"ee583528d3277ab36d910c9b63dc816870da409e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3e4520f435dfc71e940806ed365c94244dcf7dc8","unresolved":true,"context_lines":[{"line_number":347,"context_line":"        # build object store details"},{"line_number":348,"context_line":"        resource_id \u003d account.partition(self.reseller_prefix)[2]"},{"line_number":349,"context_line":"        if not resource_id:"},{"line_number":350,"context_line":"            resource_id \u003d str(uuid.uuid5(uuid.NAMESPACE_URL, path))"},{"line_number":351,"context_line":"        target \u003d cadf_resource.Resource("},{"line_number":352,"context_line":"            typeURI\u003d\u0027service/storage/object\u0027,"},{"line_number":353,"context_line":"            id\u003dresource_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"daa22038_34c8e616","line":350,"updated":"2023-04-26 16:16:46.000000000","message":"Why `path` and not `account`? It seems like the \"resource ID\" is supposed to roughly correspond to the account/project/tenant level -- having this drill down further to the container (and even object!) level likely means we can\u0027t aggregate in the way we want.\n\nI\u0027d also be a little wary of using `NAMESPACE_URL` with something that\u0027s not actually a full URL, but I\u0027m not familiar enough with the usage of UUID5 to say much for certain.","commit_id":"ee583528d3277ab36d910c9b63dc816870da409e"},{"author":{"_account_id":26274,"name":"Florent Vennetier","email":"florent.vennetier@ovhcloud.com","username":"fvennetier"},"change_message_id":"572f7156109b2d22c2813f9db2607f5921e72821","unresolved":true,"context_lines":[{"line_number":347,"context_line":"        # build object store details"},{"line_number":348,"context_line":"        resource_id \u003d account.partition(self.reseller_prefix)[2]"},{"line_number":349,"context_line":"        if not resource_id:"},{"line_number":350,"context_line":"            resource_id \u003d str(uuid.uuid5(uuid.NAMESPACE_URL, path))"},{"line_number":351,"context_line":"        target \u003d cadf_resource.Resource("},{"line_number":352,"context_line":"            typeURI\u003d\u0027service/storage/object\u0027,"},{"line_number":353,"context_line":"            id\u003dresource_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"542f746a_e6ae41f9","line":350,"in_reply_to":"daa22038_34c8e616","updated":"2023-05-03 14:57:51.000000000","message":"I chose to not change the default behavior: use the request path if we are not able to extract an account.","commit_id":"ee583528d3277ab36d910c9b63dc816870da409e"}],"ceilometermiddleware/tests/test_swift.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3e4520f435dfc71e940806ed365c94244dcf7dc8","unresolved":true,"context_lines":[{"line_number":368,"context_line":"            list(app(req.environ, self.start_response))"},{"line_number":369,"context_line":"            self.assertEqual(1, len(notify.call_args_list))"},{"line_number":370,"context_line":"            data \u003d notify.call_args_list[0][0]"},{"line_number":371,"context_line":"            self.assertEqual(\"1.0/admin/bucket\", data[2][\u0027target\u0027][\u0027id\u0027])"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"    def test_ignore_requests_from_project(self):"},{"line_number":374,"context_line":"        app \u003d swift.Swift(FakeApp(), {\u0027ignore_projects\u0027: \u0027skip_proj\u0027})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9e161f6e_bff296f8","side":"PARENT","line":371,"updated":"2023-04-26 16:16:46.000000000","message":"Could there be deployers who somehow *have* managed to get some useful signal from this kind of ID, and now we\u0027ve broken their graphs? Should we at least put this behind some config option?","commit_id":"fecef0bd8429066c1b7d6c57cea5a2e3f98dbdfb"}]}