)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"042abc7b56c52e4372f8792a0207ad8d4b40ff5a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3539f61e_5d60941b","updated":"2024-06-24 06:50:56.000000000","message":"I like the idea more than using the tcp publisher from the compute agent. I see you have a lot of pep8 errors. You can execute the pep8 test locally with `tox -e pep8`. One small thing, I see you used 8000 as default prometheus port. Why? I think prometheus should use 9090 by default.","commit_id":"b772bdc60d97de4a582053c86d85800136f158fe"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"22a9dfc05726d8aeb5147c3141fcca8ded6f7957","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3d6f7189_fd3c4bbb","in_reply_to":"3539f61e_5d60941b","updated":"2024-06-24 11:01:19.000000000","message":"You are right, I have submitted a new patchset with the pep8 errors solved. I dont think however that a prometheus exporter must use by default the Prometheus api port, nor that it should use a specific port. \n\nJust looking at examples, node_exporter uses 9100, kubelet exports metrics on 10250, blackbox exporter uses 9115 and push gateway uses 9091. This one uses 8000 because I liked it, but it might make more sense to use 9101, what do you think?","commit_id":"b772bdc60d97de4a582053c86d85800136f158fe"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"31c20ca58044f9a06aae630f16f55d7495d0d45a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"007d89ed_abacd6b9","in_reply_to":"3d6f7189_fd3c4bbb","updated":"2024-06-24 12:08:57.000000000","message":"ah, you\u0027re right. I spent the last week looking into tracing, which are being pushed into storage as opposed to Prometheus, which is pulling metrics from the scrape endpoints. Ignore the 9090 port suggestion then, it doesn\u0027t matter at all what port is used as long as it isn\u0027t the same with some other openstack service.","commit_id":"b772bdc60d97de4a582053c86d85800136f158fe"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"05da626b2f73567aec39601d82e8a4ca9f1ca328","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"95e398f2_9c9d75b2","updated":"2024-06-25 06:17:24.000000000","message":"Taking another look. I see a single configuration for the prometheus port. What happens when the compute-agent and central-agent run together on the same host? This would happen for example in devstack, which is used in CI. I imagine we\u0027d want to write some further tests for this feature in the telemetry-tempest-plugin, but for that we\u0027ll need to have both agents running and exposing metrics on a single host.","commit_id":"52b6aabf989f1703c04792c2153697fe4a92a02e"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"db6ca35d82c0e394a73190a7f97233ea947181d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"47b5f0b6_ee0eef58","in_reply_to":"95e398f2_9c9d75b2","updated":"2024-06-25 11:06:51.000000000","message":"That\u0027s true, both of them up and running on the same node (devstack) was not working. Now I have added the possibility of configure the exporter for central or compute, with different ports.\n\nBased on our previous conversation, I have also moved the default ports to 9101 and 9102, which seems more coherent with node_exporter 9100 port.","commit_id":"52b6aabf989f1703c04792c2153697fe4a92a02e"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"f665336af8cc75ad8b6827616b2db34fdf612e6b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3589ef3d_ca46d9ab","updated":"2024-07-19 12:19:32.000000000","message":"And I agree with Jaromir, that having built-in exporter seems better to me too than leveraging tcp transport in polling agent (it should stay notification\u0027s agent job).","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"80fde4ceb03c5d628cea1c8d56bcfff7576715d6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"d1636626_071d45e3","updated":"2024-07-19 12:12:30.000000000","message":"Just few nits, otherwise this seems lgtm.","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"148bb2d2a104e90097c4ec0a8a79193313e44fd5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b9aff4ca_c819deaa","updated":"2024-07-25 15:08:58.000000000","message":"ba-dum-tss","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"01a18efc1468f0b591139634a27bfd981a5258b9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"8187cc69_f0809305","updated":"2024-08-16 10:00:23.000000000","message":"I read the conversation with Martin and I have a similar question, for which I didn\u0027t find an answer. I see having multiple options for ports as beneficial (it was my suggestion after all). But do we need the \"prometheus_exporter_*\" for each of the namespaces? Would having only one option e.g \"prometheus_exporter_enabled\" for all of them change anything?","commit_id":"1d6ee26a69cc0e5022d2824319384283ba95a57c"},{"author":{"_account_id":30893,"name":"Chris Sibbitt","email":"csibbitt@redhat.com","username":"csibbitt"},"change_message_id":"bc2bad099af334b8f430cb1277b7d6698b736a47","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"48f72361_7212d25b","updated":"2024-08-29 16:17:59.000000000","message":"I\u0027d give this my +1 except for https://review.opendev.org/c/openstack/ceilometer/+/921955/12/ceilometer/polling/manager.py#327","commit_id":"1d6ee26a69cc0e5022d2824319384283ba95a57c"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"01f22583_dd196066","updated":"2024-09-20 10:51:43.000000000","message":"I\u0027m struggling to understand why these are under polling manager rather than publishers like every other output to the backends. We even have already prometheus publisher for the push model.\n\nNeed few more tests for the actual mechanisms, not just label generation. For example ensuring that the http servers are not running if the config has the exporter disabled, that they are binding to correct interfaces and not spilling all the private data (like user ids/names, project ids/names and resources associated to those) to all networks attached to the server.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"5865da30_03a5a4e2","updated":"2024-09-23 11:20:37.000000000","message":"Thanks Juan,\n\nSome ideas and clarifications inline.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"364bdcc7_d76a3c13","in_reply_to":"01f22583_dd196066","updated":"2024-09-23 09:05:10.000000000","message":"What we want here is to avoid having compute nodes to communicate via RabbitMQ messages with the notification agent in the control plane. Due to the usage of oslo.messaging in Ceilometer, RabbitMQ is the only way pollers and publishers can be communicated.\n\nWhat I am intending here is to just get the poller to export its own Prometheus metrics. This would make ceilometer-agent-compute not to rely on sending messages to the control plane. \n\nI take note of the test improving advice and a new patch will subsequently follow.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f72f0113a581e45396e333b2840114a58b5a48c0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"029ca668_5a709bf6","updated":"2024-10-01 07:33:39.000000000","message":"I know this is already merged but am leaving a few comments which I\u0027ll address by follow-up","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"95d07d5473dc02cf44a041a128d6cbd55a9113d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"caad72ed_1c2936a3","updated":"2024-09-26 12:46:43.000000000","message":"One nit which we can fix in another patch. Otherwise this looks good. Thanks for incorporating the ListOps setting.","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"}],"ceilometer/polling/manager.py":[{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"f665336af8cc75ad8b6827616b2db34fdf612e6b","unresolved":true,"context_lines":[{"line_number":82,"context_line":"                default\u003dTrue,"},{"line_number":83,"context_line":"                help\u003d\u0027Whether the polling service should be sending \u0027"},{"line_number":84,"context_line":"                     \u0027notifications to RabbitMQ after polling cycles.\u0027),"},{"line_number":85,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_central\u0027,"},{"line_number":86,"context_line":"                default\u003dFalse,"},{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer central polling instance to \u0027"},{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_central \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"},{"line_number":95,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_compute\u0027,"},{"line_number":96,"context_line":"                default\u003dFalse,"},{"line_number":97,"context_line":"                help\u003d\u0027Allow this ceilometer compute polling instance to \u0027"},{"line_number":98,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":99,"context_line":"                     \u0027format.\u0027),"},{"line_number":100,"context_line":"    cfg.IntOpt(\u0027prometheus_port_compute\u0027,"},{"line_number":101,"context_line":"               default\u003d9102,"},{"line_number":102,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":103,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_compute \u0027"},{"line_number":104,"context_line":"                    \u0027is True.\u0027)"},{"line_number":105,"context_line":"]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5e62bbe2_d30a77ba","line":104,"range":{"start_line":85,"start_character":0,"end_line":104,"end_character":31},"updated":"2024-07-19 12:19:32.000000000","message":"On the other thought, do we need to separate on central and compute? Can\u0027t we have just have `enable_protmetheus_exporter` and `prometheus_port` options? It should be fine whether the agent is running with namespaces value of central, compute, ipmi, [central, compute] or [central, compute, ipmi] ... it is either exporting or not ... IMO","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"9d7c275dfbada3921f147ed410d25dd7f9737c22","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                default\u003dTrue,"},{"line_number":83,"context_line":"                help\u003d\u0027Whether the polling service should be sending \u0027"},{"line_number":84,"context_line":"                     \u0027notifications to RabbitMQ after polling cycles.\u0027),"},{"line_number":85,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_central\u0027,"},{"line_number":86,"context_line":"                default\u003dFalse,"},{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer central polling instance to \u0027"},{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_central \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"},{"line_number":95,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_compute\u0027,"},{"line_number":96,"context_line":"                default\u003dFalse,"},{"line_number":97,"context_line":"                help\u003d\u0027Allow this ceilometer compute polling instance to \u0027"},{"line_number":98,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":99,"context_line":"                     \u0027format.\u0027),"},{"line_number":100,"context_line":"    cfg.IntOpt(\u0027prometheus_port_compute\u0027,"},{"line_number":101,"context_line":"               default\u003d9102,"},{"line_number":102,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":103,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_compute \u0027"},{"line_number":104,"context_line":"                    \u0027is True.\u0027)"},{"line_number":105,"context_line":"]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"cfcb20c4_f08ff512","line":104,"range":{"start_line":85,"start_character":0,"end_line":104,"end_character":31},"in_reply_to":"5e62bbe2_d30a77ba","updated":"2024-07-26 07:42:40.000000000","message":"If we do that, both compute and central will share the same port, rendering one of them not able to start. This happens in situations where central and compute are in the same node, such as devstack.\n\nAllowing the user to configure and enable/disable ports by namespace, we allow it to work on these kind of situations.\n\nI will add also entries for ipmi, which will always be in the same node as compute, so this issue will happen.","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"43671dad8cf37b74524e6062ee2b16eb2c63a895","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                default\u003dTrue,"},{"line_number":83,"context_line":"                help\u003d\u0027Whether the polling service should be sending \u0027"},{"line_number":84,"context_line":"                     \u0027notifications to RabbitMQ after polling cycles.\u0027),"},{"line_number":85,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_central\u0027,"},{"line_number":86,"context_line":"                default\u003dFalse,"},{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer central polling instance to \u0027"},{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_central \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"},{"line_number":95,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_compute\u0027,"},{"line_number":96,"context_line":"                default\u003dFalse,"},{"line_number":97,"context_line":"                help\u003d\u0027Allow this ceilometer compute polling instance to \u0027"},{"line_number":98,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":99,"context_line":"                     \u0027format.\u0027),"},{"line_number":100,"context_line":"    cfg.IntOpt(\u0027prometheus_port_compute\u0027,"},{"line_number":101,"context_line":"               default\u003d9102,"},{"line_number":102,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":103,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_compute \u0027"},{"line_number":104,"context_line":"                    \u0027is True.\u0027)"},{"line_number":105,"context_line":"]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"6005bc8d_aedb6189","line":104,"range":{"start_line":85,"start_character":0,"end_line":104,"end_character":31},"in_reply_to":"6ca19607_1cfb265d","updated":"2024-07-30 08:49:44.000000000","message":"We could use the DictOpt option, but it seems more confusing than adding a new parameter for each namespace. \n\nAfter your first comment, I indeed added new parameters to enable/expose IPMI metrics in their own port: https://review.opendev.org/c/openstack/ceilometer/+/921955/9/ceilometer/polling/manager.py#110\n\nIf you still think that it is more preferable to have a single DictOpt parameter, I will implement it.","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"41651630e17731bc6f314bba7b8f5b16f1957419","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                default\u003dTrue,"},{"line_number":83,"context_line":"                help\u003d\u0027Whether the polling service should be sending \u0027"},{"line_number":84,"context_line":"                     \u0027notifications to RabbitMQ after polling cycles.\u0027),"},{"line_number":85,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_central\u0027,"},{"line_number":86,"context_line":"                default\u003dFalse,"},{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer central polling instance to \u0027"},{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_central \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"},{"line_number":95,"context_line":"    cfg.BoolOpt(\u0027prometheus_exporter_compute\u0027,"},{"line_number":96,"context_line":"                default\u003dFalse,"},{"line_number":97,"context_line":"                help\u003d\u0027Allow this ceilometer compute polling instance to \u0027"},{"line_number":98,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":99,"context_line":"                     \u0027format.\u0027),"},{"line_number":100,"context_line":"    cfg.IntOpt(\u0027prometheus_port_compute\u0027,"},{"line_number":101,"context_line":"               default\u003d9102,"},{"line_number":102,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":103,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_compute \u0027"},{"line_number":104,"context_line":"                    \u0027is True.\u0027)"},{"line_number":105,"context_line":"]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"6ca19607_1cfb265d","line":104,"range":{"start_line":85,"start_character":0,"end_line":104,"end_character":31},"in_reply_to":"cfcb20c4_f08ff512","updated":"2024-07-29 10:38:24.000000000","message":"Yeah agree, that\u0027s actually ancient deployment issue/decision rather than code issue. The case where both are deployed on one node should be started as one service with two `--namespace` parameters.\n\nWe could still have single option for ports of all namespaces using DictOpt [1] or we need to add port option for IPMI polling agent. In a matter of flexibility, I would rather try the first option though.\n\n[1] https://docs.openstack.org/ocata/config-reference/config-format.html","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"80fde4ceb03c5d628cea1c8d56bcfff7576715d6","unresolved":true,"context_lines":[{"line_number":306,"context_line":"                        % ({\u0027name\u0027: pollster.name, \u0027error\u0027: err}),"},{"line_number":307,"context_line":"                        exc_info\u003dTrue)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def _send_notification(self, samples):"},{"line_number":310,"context_line":"        if self.manager.conf.polling.enable_notifications:"},{"line_number":311,"context_line":"            self.manager.notifier.sample("},{"line_number":312,"context_line":"                {},"}],"source_content_type":"text/x-python","patch_set":8,"id":"a3782e8a_90cb3596","line":309,"updated":"2024-07-19 12:12:30.000000000","message":"Nit: This method is not just about notification anymore, so maybe we should rename it as a part of this patch. Something line _collect_or_notify might express the purpose better.","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"9d7c275dfbada3921f147ed410d25dd7f9737c22","unresolved":false,"context_lines":[{"line_number":306,"context_line":"                        % ({\u0027name\u0027: pollster.name, \u0027error\u0027: err}),"},{"line_number":307,"context_line":"                        exc_info\u003dTrue)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def _send_notification(self, samples):"},{"line_number":310,"context_line":"        if self.manager.conf.polling.enable_notifications:"},{"line_number":311,"context_line":"            self.manager.notifier.sample("},{"line_number":312,"context_line":"                {},"}],"source_content_type":"text/x-python","patch_set":8,"id":"efe0befb_759d257c","line":309,"in_reply_to":"a3782e8a_90cb3596","updated":"2024-07-26 07:42:40.000000000","message":"Acknowledged","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"80fde4ceb03c5d628cea1c8d56bcfff7576715d6","unresolved":true,"context_lines":[{"line_number":380,"context_line":"        for namespace in namespaces:"},{"line_number":381,"context_line":"            if namespace \u003d\u003d \u0027compute\u0027:"},{"line_number":382,"context_line":"                if self.conf.polling.prometheus_exporter_compute:"},{"line_number":383,"context_line":"                    prom_exporter.export("},{"line_number":384,"context_line":"                        self.conf.polling.prometheus_port_compute)"},{"line_number":385,"context_line":"            if namespace \u003d\u003d \u0027central\u0027:"},{"line_number":386,"context_line":"                if self.conf.polling.prometheus_exporter_central:"}],"source_content_type":"text/x-python","patch_set":8,"id":"61bde508_0330c65d","line":383,"updated":"2024-07-19 12:12:30.000000000","message":"Nit: Not sure if we need ipmi polling exporter, but if we do, maybe we should do this block of code more generic (also for other future namespaces):\n\n```\ndef_port \u003d \u003csome_default_port\u003e\nfor ns in namespaces:\n  ex \u003d getattr(self.conf.polling, f\u0027prometheus_exporter_{ns}\u0027, False)\n  if ex:\n    prom.exporter.export(\n      getattr(self.conf.polling, f\u0027prometheus_port_{ns}\u0027, def_port))\n    def_port +\u003d 1\n    \n```","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"9d7c275dfbada3921f147ed410d25dd7f9737c22","unresolved":false,"context_lines":[{"line_number":380,"context_line":"        for namespace in namespaces:"},{"line_number":381,"context_line":"            if namespace \u003d\u003d \u0027compute\u0027:"},{"line_number":382,"context_line":"                if self.conf.polling.prometheus_exporter_compute:"},{"line_number":383,"context_line":"                    prom_exporter.export("},{"line_number":384,"context_line":"                        self.conf.polling.prometheus_port_compute)"},{"line_number":385,"context_line":"            if namespace \u003d\u003d \u0027central\u0027:"},{"line_number":386,"context_line":"                if self.conf.polling.prometheus_exporter_central:"}],"source_content_type":"text/x-python","patch_set":8,"id":"6ec72191_00ab22e3","line":383,"in_reply_to":"61bde508_0330c65d","updated":"2024-07-26 07:42:40.000000000","message":"I need to add the ipmi namespace, but I feel it is better to allow the user configure the ports as he wants than just configure by default a string of consecutive ports. \n\nIf more namespaces appear in the future, even if it is doubtful, we would need to consider whether they are exporting prometheus metrics directly from poller and add here their own exporting port.","commit_id":"f6caae694f93748527f94ab67451dda9b6fafb20"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"01a18efc1468f0b591139634a27bfd981a5258b9","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                {\u0027samples\u0027: samples}"},{"line_number":325,"context_line":"            )"},{"line_number":326,"context_line":"        if (self.manager.conf.polling.prometheus_exporter_central or"},{"line_number":327,"context_line":"                self.manager.conf.polling.prometheus_exporter_compute):"},{"line_number":328,"context_line":"            prom_exporter.collect_metrics(samples)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"012db442_f7bcdcc3","line":327,"updated":"2024-08-16 10:00:23.000000000","message":"isn\u0027t this missing the following?\n`or self.manager.conf.polling.prometheus_exporter_ipmi`","commit_id":"1d6ee26a69cc0e5022d2824319384283ba95a57c"},{"author":{"_account_id":30893,"name":"Chris Sibbitt","email":"csibbitt@redhat.com","username":"csibbitt"},"change_message_id":"bc2bad099af334b8f430cb1277b7d6698b736a47","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                {\u0027samples\u0027: samples}"},{"line_number":325,"context_line":"            )"},{"line_number":326,"context_line":"        if (self.manager.conf.polling.prometheus_exporter_central or"},{"line_number":327,"context_line":"                self.manager.conf.polling.prometheus_exporter_compute):"},{"line_number":328,"context_line":"            prom_exporter.collect_metrics(samples)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"39d4e152_89019a53","line":327,"in_reply_to":"012db442_f7bcdcc3","updated":"2024-08-29 16:17:59.000000000","message":"+1 - I agree with your other comment, this could probably just be \"prometheus_exporter_enabled\", though I\u0027m fine with either solution.","commit_id":"1d6ee26a69cc0e5022d2824319384283ba95a57c"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"0fd32388584675e138c0dcb4335bc0b1a31d08e4","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                {\u0027samples\u0027: samples}"},{"line_number":325,"context_line":"            )"},{"line_number":326,"context_line":"        if (self.manager.conf.polling.prometheus_exporter_central or"},{"line_number":327,"context_line":"                self.manager.conf.polling.prometheus_exporter_compute):"},{"line_number":328,"context_line":"            prom_exporter.collect_metrics(samples)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"e8bb731b_410c821e","line":327,"in_reply_to":"39d4e152_89019a53","updated":"2024-09-03 14:13:54.000000000","message":"Absolutely right. I chose to go with a single variable enable_prometheus_exporter and that would export on a port that depends on the namespace.","commit_id":"1d6ee26a69cc0e5022d2824319384283ba95a57c"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"499b2707_23309f70","line":91,"range":{"start_line":91,"start_character":23,"end_line":91,"end_character":27},"updated":"2024-09-20 10:51:43.000000000","message":"This port is also used by Internet Printing Protocol (IPP) expecting a lots of traffic if exposed. What was the rationale when assigning the default port numbers?","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"02bb9d385fe079699273c211f96aa7f32d47b8d5","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"70a72140_0d4e9365","line":91,"range":{"start_line":91,"start_character":23,"end_line":91,"end_character":27},"in_reply_to":"42c5d39f_e34b08b5","updated":"2024-09-23 13:37:21.000000000","message":"Acknowledged","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"f6c12cf8_402f1a2f","line":91,"range":{"start_line":91,"start_character":23,"end_line":91,"end_character":27},"in_reply_to":"499b2707_23309f70","updated":"2024-09-23 09:05:10.000000000","message":"Because node_exporter default port exposes in 9100, and we do not want to collide with that. Consecutive numbers from node_exporter seemed like a good mnemonic, but I am open to other ideas.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":true,"context_lines":[{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"42c5d39f_e34b08b5","line":91,"range":{"start_line":91,"start_character":23,"end_line":91,"end_character":27},"in_reply_to":"f6c12cf8_402f1a2f","updated":"2024-09-23 11:20:37.000000000","message":"Ok, that is sensible default then. Deployer can always change the port if needed.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer polling instance to \u0027"},{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"},{"line_number":95,"context_line":"    cfg.IntOpt(\u0027prometheus_port_compute\u0027,"},{"line_number":96,"context_line":"               default\u003d9102,"},{"line_number":97,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":98,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":99,"context_line":"                    \u0027is True.\u0027),"},{"line_number":100,"context_line":"    cfg.IntOpt(\u0027prometheus_port_ipmi\u0027,"},{"line_number":101,"context_line":"               default\u003d9103,"},{"line_number":102,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":103,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":104,"context_line":"                    \u0027is True.\u0027)"},{"line_number":105,"context_line":"]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"aad92bd0_96cf13e5","line":104,"range":{"start_line":90,"start_character":2,"end_line":104,"end_character":31},"updated":"2024-09-20 10:51:43.000000000","message":"The config options for the exporter should be under it\u0027s namespace. Mixing polling configs with publisher configs is very confusing.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer polling instance to \u0027"},{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"},{"line_number":95,"context_line":"    cfg.IntOpt(\u0027prometheus_port_compute\u0027,"},{"line_number":96,"context_line":"               default\u003d9102,"},{"line_number":97,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":98,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":99,"context_line":"                    \u0027is True.\u0027),"},{"line_number":100,"context_line":"    cfg.IntOpt(\u0027prometheus_port_ipmi\u0027,"},{"line_number":101,"context_line":"               default\u003d9103,"},{"line_number":102,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":103,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":104,"context_line":"                    \u0027is True.\u0027)"},{"line_number":105,"context_line":"]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"50ed3a00_2fd55629","line":104,"range":{"start_line":90,"start_character":2,"end_line":104,"end_character":31},"in_reply_to":"8f19349f_cb93b636","updated":"2024-09-23 11:20:37.000000000","message":"I don\u0027t like it, but I do understand the reasoning now.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer polling instance to \u0027"},{"line_number":88,"context_line":"                     \u0027expose directly the retrieved metrics in Prometheus \u0027"},{"line_number":89,"context_line":"                     \u0027format.\u0027),"},{"line_number":90,"context_line":"    cfg.IntOpt(\u0027prometheus_port_central\u0027,"},{"line_number":91,"context_line":"               default\u003d9101,"},{"line_number":92,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":93,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":94,"context_line":"                    \u0027is True.\u0027),"},{"line_number":95,"context_line":"    cfg.IntOpt(\u0027prometheus_port_compute\u0027,"},{"line_number":96,"context_line":"               default\u003d9102,"},{"line_number":97,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":98,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":99,"context_line":"                    \u0027is True.\u0027),"},{"line_number":100,"context_line":"    cfg.IntOpt(\u0027prometheus_port_ipmi\u0027,"},{"line_number":101,"context_line":"               default\u003d9103,"},{"line_number":102,"context_line":"               help\u003d\u0027The port the Prometheus exported metrics can be scraped \u0027"},{"line_number":103,"context_line":"                    \u0027from. It only has effect if prometheus_exporter_enabled \u0027"},{"line_number":104,"context_line":"                    \u0027is True.\u0027)"},{"line_number":105,"context_line":"]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"8f19349f_cb93b636","line":104,"range":{"start_line":90,"start_character":2,"end_line":104,"end_character":31},"in_reply_to":"aad92bd0_96cf13e5","updated":"2024-09-23 09:05:10.000000000","message":"As stated above, this change is explicitly intended to make the poller to not use the publisher pipelines to avoid RabbitMQ usage, so these are specific polling configs.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":377,"context_line":"                publisher_id\u003d\"ceilometer.polling\")"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        if self.conf.polling.enable_prometheus_exporter:"},{"line_number":380,"context_line":"            for namespace in namespaces:"},{"line_number":381,"context_line":"                if namespace \u003d\u003d \u0027compute\u0027:"},{"line_number":382,"context_line":"                    prom_exporter.export("},{"line_number":383,"context_line":"                        self.conf.polling.prometheus_port_compute)"},{"line_number":384,"context_line":"                if namespace \u003d\u003d \u0027central\u0027:"},{"line_number":385,"context_line":"                    prom_exporter.export("},{"line_number":386,"context_line":"                        self.conf.polling.prometheus_port_central)"},{"line_number":387,"context_line":"                if namespace \u003d\u003d \u0027ipmi\u0027:"},{"line_number":388,"context_line":"                    prom_exporter.export("},{"line_number":389,"context_line":"                        self.conf.polling.prometheus_port_ipmi)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        self._keystone \u003d None"},{"line_number":392,"context_line":"        self._keystone_last_exception \u003d None"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bf0d051d_5769e5b8","line":390,"range":{"start_line":380,"start_character":0,"end_line":390,"end_character":1},"updated":"2024-09-20 10:51:43.000000000","message":"We start 3 WSGI HTTP servers here. What determines the metrics exported through each one of them? Can we have test to ensure those are correct, please.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":true,"context_lines":[{"line_number":377,"context_line":"                publisher_id\u003d\"ceilometer.polling\")"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        if self.conf.polling.enable_prometheus_exporter:"},{"line_number":380,"context_line":"            for namespace in namespaces:"},{"line_number":381,"context_line":"                if namespace \u003d\u003d \u0027compute\u0027:"},{"line_number":382,"context_line":"                    prom_exporter.export("},{"line_number":383,"context_line":"                        self.conf.polling.prometheus_port_compute)"},{"line_number":384,"context_line":"                if namespace \u003d\u003d \u0027central\u0027:"},{"line_number":385,"context_line":"                    prom_exporter.export("},{"line_number":386,"context_line":"                        self.conf.polling.prometheus_port_central)"},{"line_number":387,"context_line":"                if namespace \u003d\u003d \u0027ipmi\u0027:"},{"line_number":388,"context_line":"                    prom_exporter.export("},{"line_number":389,"context_line":"                        self.conf.polling.prometheus_port_ipmi)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        self._keystone \u003d None"},{"line_number":392,"context_line":"        self._keystone_last_exception \u003d None"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"de122de6_d03e7e81","line":390,"range":{"start_line":380,"start_character":0,"end_line":390,"end_character":1},"in_reply_to":"384c385e_2da48a5b","updated":"2024-09-23 11:20:37.000000000","message":"The default is [\u0027compute\u0027, \u0027central\u0027]\n\nIf this is supposed to launch only one server per bind (IP) per process, perhaps we should simplify it for the operators. Instead of using this compute, central, ipmi plus what the future might bring, we could have just list of \"host:port\"s in the config options, defaulting to localhost:9001 (127.0.0.1:9001 or [::1]:9001 depending which IP stack is in use), the deployment could then decide on which networks they need to expose the endpoint in a single place rather than aligning multiple config options or ending up with multiple servers is default value is used? Bit more logic in the code, much better user experience.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[{"line_number":377,"context_line":"                publisher_id\u003d\"ceilometer.polling\")"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        if self.conf.polling.enable_prometheus_exporter:"},{"line_number":380,"context_line":"            for namespace in namespaces:"},{"line_number":381,"context_line":"                if namespace \u003d\u003d \u0027compute\u0027:"},{"line_number":382,"context_line":"                    prom_exporter.export("},{"line_number":383,"context_line":"                        self.conf.polling.prometheus_port_compute)"},{"line_number":384,"context_line":"                if namespace \u003d\u003d \u0027central\u0027:"},{"line_number":385,"context_line":"                    prom_exporter.export("},{"line_number":386,"context_line":"                        self.conf.polling.prometheus_port_central)"},{"line_number":387,"context_line":"                if namespace \u003d\u003d \u0027ipmi\u0027:"},{"line_number":388,"context_line":"                    prom_exporter.export("},{"line_number":389,"context_line":"                        self.conf.polling.prometheus_port_ipmi)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        self._keystone \u003d None"},{"line_number":392,"context_line":"        self._keystone_last_exception \u003d None"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"384c385e_2da48a5b","line":390,"range":{"start_line":380,"start_character":0,"end_line":390,"end_character":1},"in_reply_to":"bf0d051d_5769e5b8","updated":"2024-09-23 09:05:10.000000000","message":"In reality, Ceilometer is normally started with just a single namespace. Even if it support being started as a multi-namespace instance, I never saw that happening so only one WSGI HTTP server would be up at any time for a Ceilometer instance. \n\nThe metrics that are exported via these servers are the metrics that the poller is gathering from the configured namespace.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"02bb9d385fe079699273c211f96aa7f32d47b8d5","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                publisher_id\u003d\"ceilometer.polling\")"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        if self.conf.polling.enable_prometheus_exporter:"},{"line_number":380,"context_line":"            for namespace in namespaces:"},{"line_number":381,"context_line":"                if namespace \u003d\u003d \u0027compute\u0027:"},{"line_number":382,"context_line":"                    prom_exporter.export("},{"line_number":383,"context_line":"                        self.conf.polling.prometheus_port_compute)"},{"line_number":384,"context_line":"                if namespace \u003d\u003d \u0027central\u0027:"},{"line_number":385,"context_line":"                    prom_exporter.export("},{"line_number":386,"context_line":"                        self.conf.polling.prometheus_port_central)"},{"line_number":387,"context_line":"                if namespace \u003d\u003d \u0027ipmi\u0027:"},{"line_number":388,"context_line":"                    prom_exporter.export("},{"line_number":389,"context_line":"                        self.conf.polling.prometheus_port_ipmi)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        self._keystone \u003d None"},{"line_number":392,"context_line":"        self._keystone_last_exception \u003d None"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"f87fcb3b_37079c54","line":390,"range":{"start_line":380,"start_character":0,"end_line":390,"end_character":1},"in_reply_to":"de122de6_d03e7e81","updated":"2024-09-23 13:37:21.000000000","message":"Acknowledged","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f72f0113a581e45396e333b2840114a58b5a48c0","unresolved":true,"context_lines":[{"line_number":78,"context_line":"                     \u0027recommended that ceilometer be configured with a \u0027"},{"line_number":79,"context_line":"                     \u0027caching backend to reduce the number of calls \u0027"},{"line_number":80,"context_line":"                     \u0027made to keystone.\u0027),"},{"line_number":81,"context_line":"    cfg.BoolOpt(\u0027enable_notifications\u0027,"},{"line_number":82,"context_line":"                default\u003dTrue,"},{"line_number":83,"context_line":"                help\u003d\u0027Whether the polling service should be sending \u0027"},{"line_number":84,"context_line":"                     \u0027notifications to RabbitMQ after polling cycles.\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad7924d_5711698d","line":81,"range":{"start_line":81,"start_character":17,"end_line":81,"end_character":37},"updated":"2024-10-01 07:33:39.000000000","message":"IMO this option is redundant because users can disable notifications by using noop notification driver.","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"cc0a73e4a5f2b8845ccb78f9d50742beddedcaa6","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                     \u0027recommended that ceilometer be configured with a \u0027"},{"line_number":79,"context_line":"                     \u0027caching backend to reduce the number of calls \u0027"},{"line_number":80,"context_line":"                     \u0027made to keystone.\u0027),"},{"line_number":81,"context_line":"    cfg.BoolOpt(\u0027enable_notifications\u0027,"},{"line_number":82,"context_line":"                default\u003dTrue,"},{"line_number":83,"context_line":"                help\u003d\u0027Whether the polling service should be sending \u0027"},{"line_number":84,"context_line":"                     \u0027notifications to RabbitMQ after polling cycles.\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"e4342b22_33d78280","line":81,"range":{"start_line":81,"start_character":17,"end_line":81,"end_character":37},"in_reply_to":"9ad7924d_5711698d","updated":"2024-10-01 07:43:11.000000000","message":"Re-reviewing this I noticed this may be required because the same notifier is used by multiple modules.","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"d1ebb3abee41474213c754c140a26311752afefb","unresolved":true,"context_lines":[{"line_number":81,"context_line":"    cfg.BoolOpt(\u0027enable_notifications\u0027,"},{"line_number":82,"context_line":"                default\u003dTrue,"},{"line_number":83,"context_line":"                help\u003d\u0027Whether the polling service should be sending \u0027"},{"line_number":84,"context_line":"                     \u0027notifications to RabbitMQ after polling cycles.\u0027),"},{"line_number":85,"context_line":"    cfg.BoolOpt(\u0027enable_prometheus_exporter\u0027,"},{"line_number":86,"context_line":"                default\u003dFalse,"},{"line_number":87,"context_line":"                help\u003d\u0027Allow this ceilometer polling instance to \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"96e7ddc1_fd9754c4","line":84,"range":{"start_line":84,"start_character":39,"end_line":84,"end_character":48},"updated":"2024-10-04 04:23:26.000000000","message":"Notification backend can be changed according to notification driver so this needs to be removed. See https://review.opendev.org/c/openstack/ceilometer/+/931360","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f72f0113a581e45396e333b2840114a58b5a48c0","unresolved":true,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        if self.conf.polling.enable_prometheus_exporter:"},{"line_number":369,"context_line":"            for addr in self.conf.polling.prometheus_listen_addresses:"},{"line_number":370,"context_line":"                address \u003d addr.split(\":\")"},{"line_number":371,"context_line":"                if len(address) \u003d\u003d 2:"},{"line_number":372,"context_line":"                    prom_exporter.export(address[0], address[1])"},{"line_number":373,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"2b5c4fff_acf08054","line":370,"range":{"start_line":370,"start_character":16,"end_line":370,"end_character":41},"updated":"2024-10-01 07:33:39.000000000","message":"This does not work for ipv6 address.","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"}],"ceilometer/polling/prom_exporter.py":[{"author":{"_account_id":30893,"name":"Chris Sibbitt","email":"csibbitt@redhat.com","username":"csibbitt"},"change_message_id":"fa62ece2b1d8cde63969a46b04d04997d25f26a5","unresolved":true,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"},{"line_number":30,"context_line":"    for sample in samples:"},{"line_number":31,"context_line":"        name \u003d \"ceilometer_\" + sample[\u0027counter_name\u0027].replace(\u0027.\u0027, \u0027_\u0027)"},{"line_number":32,"context_line":"        type \u003d sample[\u0027counter_type\u0027]"},{"line_number":33,"context_line":"        value \u003d sample[\u0027counter_volume\u0027]"},{"line_number":34,"context_line":"        labels \u003d _gen_labels(sample)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fec057d6_9296c6c2","line":31,"range":{"start_line":31,"start_character":53,"end_line":31,"end_character":71},"updated":"2024-06-27 20:00:14.000000000","message":"If only prometheus supported `.`, right? 😊 https://github.com/prometheus/prometheus/issues/13095","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"0292a84f2bd0177e546fb9e4802b1ee0cf477b00","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"},{"line_number":30,"context_line":"    for sample in samples:"},{"line_number":31,"context_line":"        name \u003d \"ceilometer_\" + sample[\u0027counter_name\u0027].replace(\u0027.\u0027, \u0027_\u0027)"},{"line_number":32,"context_line":"        type \u003d sample[\u0027counter_type\u0027]"},{"line_number":33,"context_line":"        value \u003d sample[\u0027counter_volume\u0027]"},{"line_number":34,"context_line":"        labels \u003d _gen_labels(sample)"}],"source_content_type":"text/x-python","patch_set":7,"id":"775ad462_f07aa66c","line":31,"range":{"start_line":31,"start_character":53,"end_line":31,"end_character":71},"in_reply_to":"fec057d6_9296c6c2","updated":"2024-06-28 10:05:54.000000000","message":"If only a man could dream...","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":30893,"name":"Chris Sibbitt","email":"csibbitt@redhat.com","username":"csibbitt"},"change_message_id":"fa62ece2b1d8cde63969a46b04d04997d25f26a5","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    for sample in samples:"},{"line_number":31,"context_line":"        name \u003d \"ceilometer_\" + sample[\u0027counter_name\u0027].replace(\u0027.\u0027, \u0027_\u0027)"},{"line_number":32,"context_line":"        type \u003d sample[\u0027counter_type\u0027]"},{"line_number":33,"context_line":"        value \u003d sample[\u0027counter_volume\u0027]"},{"line_number":34,"context_line":"        labels \u003d _gen_labels(sample)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        metric \u003d CEILOMETER_REGISTRY._names_to_collectors.get(name, None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"382fc10c_154950fe","line":33,"range":{"start_line":33,"start_character":24,"end_line":33,"end_character":38},"updated":"2024-06-27 20:00:14.000000000","message":"I had to go look this up because I thought for sure it was a typo on \"counter_value\", but no, apparently this is correct. 😊","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"0292a84f2bd0177e546fb9e4802b1ee0cf477b00","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    for sample in samples:"},{"line_number":31,"context_line":"        name \u003d \"ceilometer_\" + sample[\u0027counter_name\u0027].replace(\u0027.\u0027, \u0027_\u0027)"},{"line_number":32,"context_line":"        type \u003d sample[\u0027counter_type\u0027]"},{"line_number":33,"context_line":"        value \u003d sample[\u0027counter_volume\u0027]"},{"line_number":34,"context_line":"        labels \u003d _gen_labels(sample)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        metric \u003d CEILOMETER_REGISTRY._names_to_collectors.get(name, None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"6f3bb5f2_adbb67fa","line":33,"range":{"start_line":33,"start_character":24,"end_line":33,"end_character":38},"in_reply_to":"382fc10c_154950fe","updated":"2024-06-28 10:05:54.000000000","message":"It happened the same to me, it took an embarrasing amount of time until I finally understood that the value of the metric came in \"counter_volume\". Thankfully I have sg-core code to look at.","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":30893,"name":"Chris Sibbitt","email":"csibbitt@redhat.com","username":"csibbitt"},"change_message_id":"fa62ece2b1d8cde63969a46b04d04997d25f26a5","unresolved":true,"context_lines":[{"line_number":60,"context_line":"    plugin \u003d cNameShards[0]"},{"line_number":61,"context_line":"    pluginVal \u003d sample[\u0027resource_id\u0027]"},{"line_number":62,"context_line":"    if len(cNameShards) \u003e 2:"},{"line_number":63,"context_line":"        pluginVal \u003d cNameShards[2]"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    if len(cNameShards) \u003e 1:"},{"line_number":66,"context_line":"        ctype \u003d cNameShards[1]"}],"source_content_type":"text/x-python","patch_set":7,"id":"2991ac6f_528c7dc1","line":63,"updated":"2024-06-27 20:00:14.000000000","message":"This looked weird to me so I read ahead to the tests and commented there. I\u0027m not sure this makes sense in the tested example of \"disk.device.read.latency\", making a label key and value that are both part of the name already. Is there another case where this makes more sense?","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"0292a84f2bd0177e546fb9e4802b1ee0cf477b00","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    plugin \u003d cNameShards[0]"},{"line_number":61,"context_line":"    pluginVal \u003d sample[\u0027resource_id\u0027]"},{"line_number":62,"context_line":"    if len(cNameShards) \u003e 2:"},{"line_number":63,"context_line":"        pluginVal \u003d cNameShards[2]"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    if len(cNameShards) \u003e 1:"},{"line_number":66,"context_line":"        ctype \u003d cNameShards[1]"}],"source_content_type":"text/x-python","patch_set":7,"id":"4d9ae82a_e4f40bcc","line":63,"in_reply_to":"2991ac6f_528c7dc1","updated":"2024-06-28 10:05:54.000000000","message":"Well, this is just doing exactly the same sg-core is doing, because under my understanding it was better to export exactly the same metrics with the same labels for this to be a drop-in replacement for ceilometer + sg-core.\n\nThis is to allow for us to replace one solution for the other without impacting anything else. The person who decided that this was going to be this way was Paul Leimer back in the day.","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":30893,"name":"Chris Sibbitt","email":"csibbitt@redhat.com","username":"csibbitt"},"change_message_id":"fa62ece2b1d8cde63969a46b04d04997d25f26a5","unresolved":true,"context_lines":[{"line_number":101,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027user_id\u0027])"},{"line_number":102,"context_line":"        index +\u003d 1"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    if (sample.get(\u0027user_name\u0027) !\u003d \u0027\u0027 and"},{"line_number":105,"context_line":"            sample.get(\u0027user_name\u0027) is not None):"},{"line_number":106,"context_line":"        labels[\u0027keys\u0027].append(\"user_name\")"},{"line_number":107,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027user_name\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"81a6e476_6b10e0ee","line":104,"range":{"start_line":104,"start_character":15,"end_line":104,"end_character":31},"updated":"2024-06-27 20:00:14.000000000","message":"I\u0027ve noticed that these next three labels aren\u0027t specifying the default parameter to get(), it there something different about these sample attributes?","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"0292a84f2bd0177e546fb9e4802b1ee0cf477b00","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027user_id\u0027])"},{"line_number":102,"context_line":"        index +\u003d 1"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    if (sample.get(\u0027user_name\u0027) !\u003d \u0027\u0027 and"},{"line_number":105,"context_line":"            sample.get(\u0027user_name\u0027) is not None):"},{"line_number":106,"context_line":"        labels[\u0027keys\u0027].append(\"user_name\")"},{"line_number":107,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027user_name\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"fb5ae991_1869e0e1","line":104,"range":{"start_line":104,"start_character":15,"end_line":104,"end_character":31},"in_reply_to":"81a6e476_6b10e0ee","updated":"2024-06-28 10:05:54.000000000","message":"You discovered an issue sir, fixed in the next patchset.","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":32240,"name":"Yadnesh Kulkarni","email":"yadnesh_kulkarni@proton.me","username":"ykulkarn"},"change_message_id":"15764f013923e8bf43c4d64fc4c609a3509437fa","unresolved":true,"context_lines":[{"line_number":119,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027resource_id\u0027])"},{"line_number":120,"context_line":"        index +\u003d 1"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    if (sample.get(\u0027resource_metadata\u0027, \u0027\u0027) !\u003d \u0027\u0027 and"},{"line_number":123,"context_line":"            sample.get(\u0027resource_metadata\u0027) is not None and"},{"line_number":124,"context_line":"            sample[\u0027resource_metadata\u0027].get(\u0027host\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":125,"context_line":"        labels[\u0027keys\u0027].append(\"vm_instance\")"},{"line_number":126,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027][\u0027host\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"54589f4c_06163b75","line":123,"range":{"start_line":122,"start_character":0,"end_line":123,"end_character":59},"updated":"2024-08-05 07:15:41.000000000","message":"These 2 conditions are common for the next 2 if statements, can we merge them into one and have nested if statement for the third condition ?","commit_id":"266047a8b3fd40bb4c103406464cb98142d51604"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"2930ab243cf7f2c141b3b88c6cb48c02c33abbe1","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027resource_id\u0027])"},{"line_number":120,"context_line":"        index +\u003d 1"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    if (sample.get(\u0027resource_metadata\u0027, \u0027\u0027) !\u003d \u0027\u0027 and"},{"line_number":123,"context_line":"            sample.get(\u0027resource_metadata\u0027) is not None and"},{"line_number":124,"context_line":"            sample[\u0027resource_metadata\u0027].get(\u0027host\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":125,"context_line":"        labels[\u0027keys\u0027].append(\"vm_instance\")"},{"line_number":126,"context_line":"        labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027][\u0027host\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"647373a1_5d78c316","line":123,"range":{"start_line":122,"start_character":0,"end_line":123,"end_character":59},"in_reply_to":"54589f4c_06163b75","updated":"2024-08-05 13:10:39.000000000","message":"Acknowledged","commit_id":"266047a8b3fd40bb4c103406464cb98142d51604"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"19380e99_1a9b2b47","updated":"2024-09-20 10:51:43.000000000","message":"I think this belongs under ceilometer/publisher/ rather than ceilometer/polling/","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"291550eb_96b289a7","in_reply_to":"19380e99_1a9b2b47","updated":"2024-09-23 09:05:10.000000000","message":"No, this is not a publisher but a poller config, see above.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# Copyright 2024 Juan Larriba"},{"line_number":3,"context_line":"# Copyright 2014-2024 Red Hat, Inc"},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":6,"context_line":"# not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":13,"id":"ab088cdb_1c5d05dc","line":3,"updated":"2024-09-20 10:51:43.000000000","message":"Just initial publish/significant change year. No range needed. https://wiki.openstack.org/wiki/Documentation/Copyright","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# Copyright 2024 Juan Larriba"},{"line_number":3,"context_line":"# Copyright 2014-2024 Red Hat, Inc"},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":6,"context_line":"# not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":13,"id":"f504e787_f8ac57fa","line":3,"in_reply_to":"ab088cdb_1c5d05dc","updated":"2024-09-23 09:05:10.000000000","message":"Acknowledged","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":14,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":15,"context_line":"# under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from prometheus_client import CollectorRegistry"},{"line_number":18,"context_line":"from prometheus_client import Counter"},{"line_number":19,"context_line":"from prometheus_client import Gauge"},{"line_number":20,"context_line":"from prometheus_client import start_http_server"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"CEILOMETER_REGISTRY \u003d CollectorRegistry()"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"cd7dc30d_ef2544e6","line":20,"range":{"start_line":17,"start_character":0,"end_line":20,"end_character":47},"updated":"2024-09-20 10:51:43.000000000","message":"Do not import objects, only modules.\nhttps://docs.openstack.org/hacking/latest/user/hacking.html#imports","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":false,"context_lines":[{"line_number":14,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":15,"context_line":"# under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from prometheus_client import CollectorRegistry"},{"line_number":18,"context_line":"from prometheus_client import Counter"},{"line_number":19,"context_line":"from prometheus_client import Gauge"},{"line_number":20,"context_line":"from prometheus_client import start_http_server"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"CEILOMETER_REGISTRY \u003d CollectorRegistry()"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"c19eacf1_1f9416e2","line":20,"range":{"start_line":17,"start_character":0,"end_line":20,"end_character":47},"in_reply_to":"cd7dc30d_ef2544e6","updated":"2024-09-23 09:05:10.000000000","message":"Acknowledged","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"fe37d967_0f67b246","line":26,"range":{"start_line":26,"start_character":4,"end_line":26,"end_character":68},"updated":"2024-09-20 10:51:43.000000000","message":"This will listen on all networks. We definitely need a mechanism to limit that.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":22,"context_line":"CEILOMETER_REGISTRY \u003d CollectorRegistry()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"5b7c6553_fad263ed","line":26,"range":{"start_line":25,"start_character":0,"end_line":26,"end_character":68},"updated":"2024-09-20 10:51:43.000000000","message":"This will start python daemon thread, we probably should pipe that to the process managers for reloads.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"02bb9d385fe079699273c211f96aa7f32d47b8d5","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"f3c57d3e_b1b4bf4a","line":26,"range":{"start_line":26,"start_character":4,"end_line":26,"end_character":68},"in_reply_to":"3796c6e3_38f3b168","updated":"2024-09-23 13:37:21.000000000","message":"Acknowledged","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":true,"context_lines":[{"line_number":22,"context_line":"CEILOMETER_REGISTRY \u003d CollectorRegistry()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"754c5c85_21504337","line":26,"range":{"start_line":25,"start_character":0,"end_line":26,"end_character":68},"in_reply_to":"3f18c018_e9c7cee0","updated":"2024-09-23 11:20:37.000000000","message":"Ok, so python daemon threads are generally means to fire and forget way of launching threads from the process that gets killed automatically by kernel when the parent process is killed. If the config is changed and reload is issued instead these daemon threads stays lingering around. The prometheus client returns the daemon thread control object which we probably should save somewhere in the parent process to ensure those threads are still wanted upon reload.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[{"line_number":22,"context_line":"CEILOMETER_REGISTRY \u003d CollectorRegistry()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f18c018_e9c7cee0","line":26,"range":{"start_line":25,"start_character":0,"end_line":26,"end_character":68},"in_reply_to":"5b7c6553_fad263ed","updated":"2024-09-23 09:05:10.000000000","message":"I don\u0027t know what that means, sorry, could you elaborate what I would need to do here?","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"02bb9d385fe079699273c211f96aa7f32d47b8d5","unresolved":true,"context_lines":[{"line_number":22,"context_line":"CEILOMETER_REGISTRY \u003d CollectorRegistry()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"fb43cce9_35ffdf81","line":26,"range":{"start_line":25,"start_character":0,"end_line":26,"end_character":68},"in_reply_to":"754c5c85_21504337","updated":"2024-09-23 13:37:21.000000000","message":"Now I understand it better, however I might not be very used to how the openstack services handle these reload operations, so I still lack an idea of what you are suggesting I should do with the Thread object returned by the start_http_server function.\n\nIt would be awesome if you could maybe point me to an example of any other service that is currently handling multithreads and take care of the reload so I can apply the same pattern here.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"3796c6e3_38f3b168","line":26,"range":{"start_line":26,"start_character":4,"end_line":26,"end_character":68},"in_reply_to":"e371be37_b6e4278f","updated":"2024-09-23 11:20:37.000000000","message":"Thanks, see also my comment on the config options for some ideas.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def export(prometheus_port):"},{"line_number":26,"context_line":"    start_http_server(prometheus_port, registry\u003dCEILOMETER_REGISTRY)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def collect_metrics(samples):"}],"source_content_type":"text/x-python","patch_set":13,"id":"e371be37_b6e4278f","line":26,"range":{"start_line":26,"start_character":4,"end_line":26,"end_character":68},"in_reply_to":"fe37d967_0f67b246","updated":"2024-09-23 09:05:10.000000000","message":"That would be awesome, I will add the possibility for the user to specify an network interface address for Ceilometer to use instead of exposing in every interface. Patch will follow.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":131,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"},{"line_number":132,"context_line":"            labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027]"},{"line_number":133,"context_line":"                                    [\u0027display_name\u0027])"},{"line_number":134,"context_line":"            # index +\u003d 1"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        if (sample[\u0027resource_metadata\u0027].get(\u0027name\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":137,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"ce9f7ea4_9e3bed51","line":134,"updated":"2024-09-20 10:51:43.000000000","message":"if this is not required, please remove.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"4630a3003e2cf5b974a7b361cdea561bece840d3","unresolved":true,"context_lines":[{"line_number":131,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"},{"line_number":132,"context_line":"            labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027]"},{"line_number":133,"context_line":"                                    [\u0027display_name\u0027])"},{"line_number":134,"context_line":"            # index +\u003d 1"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        if (sample[\u0027resource_metadata\u0027].get(\u0027name\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":137,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"e831c29a_b0832149","line":134,"in_reply_to":"402a0bc9_22afb7a5","updated":"2024-09-23 09:38:23.000000000","message":"Almost everytime I read this line (either here or in sg-core), it took me a few seconds to understand what it means. Maybe it would be worth adding a comment with an explanation of why it\u0027s there.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"02bb9d385fe079699273c211f96aa7f32d47b8d5","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"},{"line_number":132,"context_line":"            labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027]"},{"line_number":133,"context_line":"                                    [\u0027display_name\u0027])"},{"line_number":134,"context_line":"            # index +\u003d 1"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        if (sample[\u0027resource_metadata\u0027].get(\u0027name\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":137,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"3edb71e5_e46e896c","line":134,"in_reply_to":"a3a3723e_90eca140","updated":"2024-09-23 13:37:21.000000000","message":"I see that this is considered confusing, and I believe that a comment might not help, so I will just remove it from here to make this more readable.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[{"line_number":131,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"},{"line_number":132,"context_line":"            labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027]"},{"line_number":133,"context_line":"                                    [\u0027display_name\u0027])"},{"line_number":134,"context_line":"            # index +\u003d 1"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        if (sample[\u0027resource_metadata\u0027].get(\u0027name\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":137,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"402a0bc9_22afb7a5","line":134,"in_reply_to":"ce9f7ea4_9e3bed51","updated":"2024-09-23 09:05:10.000000000","message":"This is an indication that we are not adding plus one to the index because we specifically dont want to, it is not that we have forgot about it.\n\nI could remove it, but I believe that adds to the code legibility.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"d31dc36c141330052a672079ef944bdbede5a891","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"},{"line_number":132,"context_line":"            labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027]"},{"line_number":133,"context_line":"                                    [\u0027display_name\u0027])"},{"line_number":134,"context_line":"            # index +\u003d 1"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        if (sample[\u0027resource_metadata\u0027].get(\u0027name\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":137,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"79b1ad13_f481e4be","line":134,"in_reply_to":"e831c29a_b0832149","updated":"2024-09-23 11:03:59.000000000","message":"Acknowledged","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":true,"context_lines":[{"line_number":131,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"},{"line_number":132,"context_line":"            labels[\u0027values\u0027].append(sample[\u0027resource_metadata\u0027]"},{"line_number":133,"context_line":"                                    [\u0027display_name\u0027])"},{"line_number":134,"context_line":"            # index +\u003d 1"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        if (sample[\u0027resource_metadata\u0027].get(\u0027name\u0027, \u0027\u0027) !\u003d \u0027\u0027):"},{"line_number":137,"context_line":"            labels[\u0027keys\u0027].append(\"resource_name\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"a3a3723e_90eca140","line":134,"in_reply_to":"e831c29a_b0832149","updated":"2024-09-23 11:20:37.000000000","message":"Yes, this looks like forgoten cleanup after debugging, please use descriptive comment instead explaining why we don\u0027t want to increment.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5241,"name":"Martin Magr","email":"mmagr@redhat.com","username":"mmagr"},"change_message_id":"95d07d5473dc02cf44a041a128d6cbd55a9113d3","unresolved":true,"context_lines":[{"line_number":28,"context_line":"def collect_metrics(samples):"},{"line_number":29,"context_line":"    for sample in samples:"},{"line_number":30,"context_line":"        name \u003d \"ceilometer_\" + sample[\u0027counter_name\u0027].replace(\u0027.\u0027, \u0027_\u0027)"},{"line_number":31,"context_line":"        type \u003d sample[\u0027counter_type\u0027]"},{"line_number":32,"context_line":"        value \u003d sample[\u0027counter_volume\u0027]"},{"line_number":33,"context_line":"        labels \u003d _gen_labels(sample)"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"221c5640_38aad6b9","line":31,"updated":"2024-09-26 12:46:43.000000000","message":"Nit: Sorry for not noticing this sooner, but you probably should not override `type` builtin here. I guess `stype` should work.","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"64b3d8ec28660898fafdf88690596b5769aeaf95","unresolved":true,"context_lines":[{"line_number":28,"context_line":"def collect_metrics(samples):"},{"line_number":29,"context_line":"    for sample in samples:"},{"line_number":30,"context_line":"        name \u003d \"ceilometer_\" + sample[\u0027counter_name\u0027].replace(\u0027.\u0027, \u0027_\u0027)"},{"line_number":31,"context_line":"        type \u003d sample[\u0027counter_type\u0027]"},{"line_number":32,"context_line":"        value \u003d sample[\u0027counter_volume\u0027]"},{"line_number":33,"context_line":"        labels \u003d _gen_labels(sample)"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"c5dffffb_f083bf66","line":31,"in_reply_to":"221c5640_38aad6b9","updated":"2024-09-27 07:03:27.000000000","message":"Thats a good catch, I will fix it in a subsequent patch if everything else is OK.","commit_id":"7289278dada9ac5dfd8679694effb9bd0ffb3b22"}],"ceilometer/tests/unit/test_prom_exporter.py":[{"author":{"_account_id":30893,"name":"Chris Sibbitt","email":"csibbitt@redhat.com","username":"csibbitt"},"change_message_id":"fa62ece2b1d8cde63969a46b04d04997d25f26a5","unresolved":true,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_gen_labels(self):"},{"line_number":186,"context_line":"        label1 \u003d prom_exporter._gen_labels(self.test_data[0])"},{"line_number":187,"context_line":"        self.assertEqual(\"disk\", label1[\u0027keys\u0027][0])"},{"line_number":188,"context_line":"        self.assertEqual(\"read\", label1[\u0027values\u0027][0])"},{"line_number":189,"context_line":"        label2 \u003d prom_exporter._gen_labels(self.test_data[2])"},{"line_number":190,"context_line":"        self.assertEqual(\"image\", label2[\u0027keys\u0027][0])"},{"line_number":191,"context_line":"        self.assertEqual(\"f9276c96-8a12-432b-96a1-559d70715f97\","}],"source_content_type":"text/x-python","patch_set":7,"id":"6de0e6e6_185f9b60","line":188,"range":{"start_line":187,"start_character":9,"end_line":188,"end_character":53},"updated":"2024-06-27 20:00:14.000000000","message":"\"disk\u003dread\" seems like a weird label to me. The prometheus style is to put \"read\" or \"write\" in the metric name, whereas the otel style is to have a label called \"direction\". AFAICT, \"read\" will be in the metric name here already - is there another case where","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"0292a84f2bd0177e546fb9e4802b1ee0cf477b00","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_gen_labels(self):"},{"line_number":186,"context_line":"        label1 \u003d prom_exporter._gen_labels(self.test_data[0])"},{"line_number":187,"context_line":"        self.assertEqual(\"disk\", label1[\u0027keys\u0027][0])"},{"line_number":188,"context_line":"        self.assertEqual(\"read\", label1[\u0027values\u0027][0])"},{"line_number":189,"context_line":"        label2 \u003d prom_exporter._gen_labels(self.test_data[2])"},{"line_number":190,"context_line":"        self.assertEqual(\"image\", label2[\u0027keys\u0027][0])"},{"line_number":191,"context_line":"        self.assertEqual(\"f9276c96-8a12-432b-96a1-559d70715f97\","}],"source_content_type":"text/x-python","patch_set":7,"id":"b2f20abd_62730322","line":188,"range":{"start_line":187,"start_character":9,"end_line":188,"end_character":53},"in_reply_to":"6de0e6e6_185f9b60","updated":"2024-06-28 10:05:54.000000000","message":"This was addressed on a previous comment, but to summarize, this is what sg-core is doing today, and I want this to be a drop-in replacement for that.","commit_id":"f3eaef585dc3d27ecf9dd45bfea0abaf936c3b67"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"0f2ffd2852ed92db27c736a9d60203b7c7614813","unresolved":true,"context_lines":[{"line_number":183,"context_line":"                                          sample_dict_2))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_gen_labels(self):"},{"line_number":186,"context_line":"        label1 \u003d prom_exporter._gen_labels(self.test_data[0])"},{"line_number":187,"context_line":"        self.assertEqual(\"disk\", label1[\u0027keys\u0027][0])"},{"line_number":188,"context_line":"        self.assertEqual(\"read\", label1[\u0027values\u0027][0])"},{"line_number":189,"context_line":"        label2 \u003d prom_exporter._gen_labels(self.test_data[2])"},{"line_number":190,"context_line":"        self.assertEqual(\"image\", label2[\u0027keys\u0027][0])"},{"line_number":191,"context_line":"        self.assertEqual(\"f9276c96-8a12-432b-96a1-559d70715f97\","},{"line_number":192,"context_line":"                         label2[\u0027values\u0027][0])"}],"source_content_type":"text/x-python","patch_set":13,"id":"fe3b0e17_f3ff4d6a","line":192,"range":{"start_line":186,"start_character":0,"end_line":192,"end_character":45},"updated":"2024-09-20 10:51:43.000000000","message":"if these are the only labels produced, we should also test that the length is 1.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"02bb9d385fe079699273c211f96aa7f32d47b8d5","unresolved":true,"context_lines":[{"line_number":183,"context_line":"                                          sample_dict_2))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_gen_labels(self):"},{"line_number":186,"context_line":"        label1 \u003d prom_exporter._gen_labels(self.test_data[0])"},{"line_number":187,"context_line":"        self.assertEqual(\"disk\", label1[\u0027keys\u0027][0])"},{"line_number":188,"context_line":"        self.assertEqual(\"read\", label1[\u0027values\u0027][0])"},{"line_number":189,"context_line":"        label2 \u003d prom_exporter._gen_labels(self.test_data[2])"},{"line_number":190,"context_line":"        self.assertEqual(\"image\", label2[\u0027keys\u0027][0])"},{"line_number":191,"context_line":"        self.assertEqual(\"f9276c96-8a12-432b-96a1-559d70715f97\","},{"line_number":192,"context_line":"                         label2[\u0027values\u0027][0])"}],"source_content_type":"text/x-python","patch_set":13,"id":"7b9abe0b_4ab343d1","line":192,"range":{"start_line":186,"start_character":0,"end_line":192,"end_character":45},"in_reply_to":"9b8ca076_8591f40a","updated":"2024-09-23 13:37:21.000000000","message":"Thank you for your reply, I will create a much more complete test here.","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4d44513dabd1fa40d264739add62465c191e8b4d","unresolved":true,"context_lines":[{"line_number":183,"context_line":"                                          sample_dict_2))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_gen_labels(self):"},{"line_number":186,"context_line":"        label1 \u003d prom_exporter._gen_labels(self.test_data[0])"},{"line_number":187,"context_line":"        self.assertEqual(\"disk\", label1[\u0027keys\u0027][0])"},{"line_number":188,"context_line":"        self.assertEqual(\"read\", label1[\u0027values\u0027][0])"},{"line_number":189,"context_line":"        label2 \u003d prom_exporter._gen_labels(self.test_data[2])"},{"line_number":190,"context_line":"        self.assertEqual(\"image\", label2[\u0027keys\u0027][0])"},{"line_number":191,"context_line":"        self.assertEqual(\"f9276c96-8a12-432b-96a1-559d70715f97\","},{"line_number":192,"context_line":"                         label2[\u0027values\u0027][0])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9b8ca076_8591f40a","line":192,"range":{"start_line":186,"start_character":0,"end_line":192,"end_character":45},"in_reply_to":"fcc088a4_01a0f040","updated":"2024-09-23 11:20:37.000000000","message":"You pointed out the logic failure there. If we would assert the length of 1, which would align with what the test actually tests, it would fail as the labels are not what we expect based on the test logic. On both examples this tests that the first label is correct, not that the _gen_labels() works as expected in general (it may now or in future produce only one label from any input and the test would indicate it working as intended). Perhaps write static expected_labels1 and expected_labels2 objects and compare that they are identical with what the _gen_labels() produces with the defined input. Even if we tested the last key/value pair of the list and the length, we would have much better confidence that the labels produced are what we expect, I doubt the ordering matters?","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"680d52495043165a08f7744ace3a2d2b24eaf064","unresolved":true,"context_lines":[{"line_number":183,"context_line":"                                          sample_dict_2))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_gen_labels(self):"},{"line_number":186,"context_line":"        label1 \u003d prom_exporter._gen_labels(self.test_data[0])"},{"line_number":187,"context_line":"        self.assertEqual(\"disk\", label1[\u0027keys\u0027][0])"},{"line_number":188,"context_line":"        self.assertEqual(\"read\", label1[\u0027values\u0027][0])"},{"line_number":189,"context_line":"        label2 \u003d prom_exporter._gen_labels(self.test_data[2])"},{"line_number":190,"context_line":"        self.assertEqual(\"image\", label2[\u0027keys\u0027][0])"},{"line_number":191,"context_line":"        self.assertEqual(\"f9276c96-8a12-432b-96a1-559d70715f97\","},{"line_number":192,"context_line":"                         label2[\u0027values\u0027][0])"}],"source_content_type":"text/x-python","patch_set":13,"id":"fcc088a4_01a0f040","line":192,"range":{"start_line":186,"start_character":0,"end_line":192,"end_character":45},"in_reply_to":"fe3b0e17_f3ff4d6a","updated":"2024-09-23 09:05:10.000000000","message":"There are many other labels produced, we are testing a subset of them here, not all of them. But I dont really get what length should be 1, care to elaborate a little bit more?","commit_id":"5f0b0d5b510dcc94cb9133328006c97de35c9062"}]}