)]}'
{"ceilometer/compute/pollsters/cpu.py":[{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a72fb265ff8e22f25fd01dcdc08c7d27059a1f1c","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"class CPUPollster(plugin.ComputePollster):"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def get_samples(self, manager, cache, resources):"},{"line_number":34,"context_line":"        for instance in resources:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2FmKU%3D","line":31,"updated":"2014-03-15 14:21:41.000000000","message":"Hello!:) Let me describe my thoughts here and they may be applied to whole change request. \nI don\u0027t like 2 things here: 1. different debug-levels for \u0027checking instance\u0027 and \u0027Exceptions in getting samples\u0027  2. Logging polling result into INFO level.\nI\u0027d like to describe 1. item again. \nLet\u0027s assume that we have an integration test that checks that pollsters work ok. We have very small interval, e.g. 1 second.  So we\u0027ve created a test that boots an instance and sends getSample request to Ceilometer. The result of getSamples is []. We go to INFO-level logs and see the message \u0027checking instance id-of-booted-instance-from-test\u0027. It means for us that Ceilometer just has lost the sample in collector. We go to collector and so on and so on. But actually we have forgotten to wait ACTIVE state of instance and it was deleted during polling. And we could realise that we are bad QA-engeneers after first glance to logs if \"Exception while getting samples\" and \"checking the instance\" were on the same level. Actually, level is not really important. DEBUG for both is ok too: we come to logs, do not see any useful messages and first thought \"enable debug logs\". After that we will see the whole picture.","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f146b22fbcf4c432cf001365fd26c239ca1df85b","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"class CPUPollster(plugin.ComputePollster):"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def get_samples(self, manager, cache, resources):"},{"line_number":34,"context_line":"        for instance in resources:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Fl2A%3D","line":31,"in_reply_to":"AAAAWH%2F%2FmKU%3D","updated":"2014-03-15 18:24:01.000000000","message":"hi Nadya, so i think you should be using debug logs and not info logs.\n\nlogging levels are set up to be a subsets of each other. it goes CRITICAL-\u003eERROR-\u003eWARNING-\u003eINFO-\u003eDEBUG. if you set logger level to CRITICAL, you will see just critical logs. if you set logger level to INFO, you will see CRITICAL, ERROR, WARNING and INFO logs (but they\u0027ll be tag specifically).\n\nlogging is done for not just tests but general deployment. in general deployment, i don\u0027t really care what happens underhood. i just want to know things are working (status), and when errors happen. for integration tests, you should be looking at debug level logger (which will include debug, info, warning... logs). \n\nlogs shouldn\u0027t be the same level just because they are tracking the same system action. the level should be based on the status at that point in code and the information the log provides.","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f146b22fbcf4c432cf001365fd26c239ca1df85b","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def get_samples(self, manager, cache, resources):"},{"line_number":34,"context_line":"        for instance in resources:"},{"line_number":35,"context_line":"            LOG.info(_(\u0027checking instance %s\u0027), instance.id)"},{"line_number":36,"context_line":"            instance_name \u003d util.instance_name(instance)"},{"line_number":37,"context_line":"            try:"},{"line_number":38,"context_line":"                cpu_info \u003d manager.inspector.inspect_cpus(instance_name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Fl14%3D","line":35,"updated":"2014-03-15 18:24:01.000000000","message":"when logging individual logs, the level chosen should be specific to the purpose of that log. ie LOG.info(_(\u0027checking instance %s\u0027) is really just a status log, it provides no information on the environment at that point.","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f146b22fbcf4c432cf001365fd26c239ca1df85b","unresolved":false,"context_lines":[{"line_number":36,"context_line":"            instance_name \u003d util.instance_name(instance)"},{"line_number":37,"context_line":"            try:"},{"line_number":38,"context_line":"                cpu_info \u003d manager.inspector.inspect_cpus(instance_name)"},{"line_number":39,"context_line":"                LOG.info(_(\"CPUTIME USAGE: %(instance)s %(time)d\") % ("},{"line_number":40,"context_line":"                         {\u0027instance\u0027: instance.__dict__,"},{"line_number":41,"context_line":"                          \u0027time\u0027: cpu_info.time}))"},{"line_number":42,"context_line":"                cpu_num \u003d {\u0027cpu_number\u0027: cpu_info.number}"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Fl1Q%3D","line":39,"updated":"2014-03-15 18:24:01.000000000","message":"this i feel could be either info or debug. for me it\u0027s a bit verbose to have this displayed in general deployment.","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a34b0ea06d04783f23e5202d65a29c17223b60c8","unresolved":false,"context_lines":[{"line_number":36,"context_line":"            instance_name \u003d util.instance_name(instance)"},{"line_number":37,"context_line":"            try:"},{"line_number":38,"context_line":"                cpu_info \u003d manager.inspector.inspect_cpus(instance_name)"},{"line_number":39,"context_line":"                LOG.info(_(\"CPUTIME USAGE: %(instance)s %(time)d\") % ("},{"line_number":40,"context_line":"                         {\u0027instance\u0027: instance.__dict__,"},{"line_number":41,"context_line":"                          \u0027time\u0027: cpu_info.time}))"},{"line_number":42,"context_line":"                cpu_num \u003d {\u0027cpu_number\u0027: cpu_info.number}"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Flvs%3D","line":39,"in_reply_to":"AAAAWH%2F%2Fl1Q%3D","updated":"2014-03-15 19:56:32.000000000","message":"Yep, I agree that it should be debug. I\u0027m afraid that if we have high load and small polling interval we may get too many messages in INFO","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f146b22fbcf4c432cf001365fd26c239ca1df85b","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                )"},{"line_number":51,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":52,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":53,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"},{"line_number":54,"context_line":"            except NotImplementedError:"},{"line_number":55,"context_line":"                # Selected inspector does not implement this pollster."},{"line_number":56,"context_line":"                LOG.debug(_(\u0027Obtaining CPU time is not implemented for %s\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Fl1A%3D","line":53,"updated":"2014-03-15 18:24:01.000000000","message":"this is debug because it\u0027s a plausible (semi-regular) error. no need throw warning or show it in regular info logs.. but if you do want to see it, then you can check/enable debug level.","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"a34b0ea06d04783f23e5202d65a29c17223b60c8","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                )"},{"line_number":51,"context_line":"            except virt_inspector.InstanceNotFoundException as err:"},{"line_number":52,"context_line":"                # Instance was deleted while getting samples. Ignore it."},{"line_number":53,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"},{"line_number":54,"context_line":"            except NotImplementedError:"},{"line_number":55,"context_line":"                # Selected inspector does not implement this pollster."},{"line_number":56,"context_line":"                LOG.debug(_(\u0027Obtaining CPU time is not implemented for %s\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Flvk%3D","line":53,"in_reply_to":"AAAAWH%2F%2Fl1A%3D","updated":"2014-03-15 19:56:32.000000000","message":"Ok, I explained my example above so if you found it not convincing it may be a problem in my world\u0027s picture :) I will not argue about it anymore, don\u0027t think that it is critical enough","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f146b22fbcf4c432cf001365fd26c239ca1df85b","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                LOG.debug(_(\u0027Exception while getting samples %s\u0027), err)"},{"line_number":54,"context_line":"            except NotImplementedError:"},{"line_number":55,"context_line":"                # Selected inspector does not implement this pollster."},{"line_number":56,"context_line":"                LOG.debug(_(\u0027Obtaining CPU time is not implemented for %s\u0027"},{"line_number":57,"context_line":"                            ), manager.inspector.__class__.__name__)"},{"line_number":58,"context_line":"            except Exception as err:"},{"line_number":59,"context_line":"                LOG.exception(_(\u0027could not get CPU time for %(id)s: %(e)s\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Fl08%3D","line":56,"updated":"2014-03-15 18:24:01.000000000","message":"this is debug because it\u0027s expected... not useful for regular status logs but if you do want to see it, then you can check/enable debug level.","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"},{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f146b22fbcf4c432cf001365fd26c239ca1df85b","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                LOG.debug(_(\u0027Obtaining CPU time is not implemented for %s\u0027"},{"line_number":57,"context_line":"                            ), manager.inspector.__class__.__name__)"},{"line_number":58,"context_line":"            except Exception as err:"},{"line_number":59,"context_line":"                LOG.exception(_(\u0027could not get CPU time for %(id)s: %(e)s\u0027),"},{"line_number":60,"context_line":"                              {\u0027id\u0027: instance.id, \u0027e\u0027: err})"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F%2Fl00%3D","line":59,"updated":"2014-03-15 18:24:01.000000000","message":"this is unexpected and the level is raised accordingly.","commit_id":"9e35a2dc637704b0fda4f58f40be7f89924f8a4b"}],"ceilometer/compute/virt/inspector.py":[{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"cc07565b5a3da647ba322b8b0ff15c76f7234045","unresolved":false,"context_lines":[{"line_number":146,"context_line":"# Main virt inspector abstraction layering over the hypervisor API."},{"line_number":147,"context_line":"#"},{"line_number":148,"context_line":"class Inspector(object):"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @staticmethod"},{"line_number":151,"context_line":"    def inspect_instances():"},{"line_number":152,"context_line":"        \"\"\"List the instances on the current host.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2BywY%3D","line":149,"updated":"2014-03-24 10:17:43.000000000","message":"Gordon, please explain me is it normal practice to override @staticmethods by non-static ones? With your patch I see a lot of \"Signature of method does not match signature of base method\u0027 warnings","commit_id":"8b79c184195be2465440e02989edd2b836bb9b15"},{"author":{"_account_id":7478,"name":"Nadya Shakhat","email":"nadmi4@gmail.com","username":"nprivalova"},"change_message_id":"827e80634325a9106fefde5ff2f576023da7f05c","unresolved":false,"context_lines":[{"line_number":146,"context_line":"# Main virt inspector abstraction layering over the hypervisor API."},{"line_number":147,"context_line":"#"},{"line_number":148,"context_line":"class Inspector(object):"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @staticmethod"},{"line_number":151,"context_line":"    def inspect_instances():"},{"line_number":152,"context_line":"        \"\"\"List the instances on the current host.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2ByLM%3D","line":149,"in_reply_to":"AAAAWH%2F%2Byp8%3D","updated":"2014-03-24 11:47:11.000000000","message":"ok, let\u0027s wait for Julien here :) The problem is that implementations are not actually static...I would understand this change if they are","commit_id":"8b79c184195be2465440e02989edd2b836bb9b15"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"4cfed39237fb0c5e82bb9539ceffc590a79067b1","unresolved":false,"context_lines":[{"line_number":146,"context_line":"# Main virt inspector abstraction layering over the hypervisor API."},{"line_number":147,"context_line":"#"},{"line_number":148,"context_line":"class Inspector(object):"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @staticmethod"},{"line_number":151,"context_line":"    def inspect_instances():"},{"line_number":152,"context_line":"        \"\"\"List the instances on the current host.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2Byp8%3D","line":149,"in_reply_to":"AAAAWH%2F%2BywY%3D","updated":"2014-03-24 10:37:07.000000000","message":"There was a short discussion on IRC last week about this, where Julien more or less explained us, why we should use @staticmethod here. Please see the log for details: http://eavesdrop.openstack.org/irclogs/%23openstack-ceilometer/%23openstack-ceilometer.2014-03-17.log\n\nThe change was originally requested by Julien here: https://review.openstack.org/#/c/74702/14/ceilometer/compute/virt/inspector.py\n\nThe Connection class for storage drivers in /storage/base.py works in the same way, there are a few functions, where should be signature issues also.","commit_id":"8b79c184195be2465440e02989edd2b836bb9b15"}]}