)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"68cb3a979134b0f267c7505ebe5fb168d4093424","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"79f5b4b0_d8090ade","updated":"2025-06-30 15:11:50.000000000","message":"observabilityclient would be regarded as a generic client implementation, so IMO we should NOT add a cache mechanism withih the client. If we do that, then we should expose a tuning options so that users can intentionally disable the chaching.","commit_id":"9380ebdc3e2b5db0a753a371baf51b452185a24f"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"9e2bb56afe1621f14458ffee08b020e80a44bcf2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"faeb9be8_aa3a8b06","in_reply_to":"19be1652_a9e7aa83","updated":"2025-07-08 12:35:54.000000000","message":"Done","commit_id":"9380ebdc3e2b5db0a753a371baf51b452185a24f"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"8d900f91dde6023624af76e2439e95c0f42e86ec","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5cc0bd32_89844478","in_reply_to":"1c7a5ffc_760521de","updated":"2025-07-07 14:00:49.000000000","message":"The whole rbac file is a remnant from before aetos, when the rbac was implemented on the client-side, by modifying queries to add rbac labels directly in the client. This had a huge issue, as anyone using the client could just use curl and access all metrics anyway. This was recently deprecated and can\u0027t be used from the CLI anymore. But the file in a modified form stayed.\n\nIt\u0027s mostly standalone now and I guess it\u0027d make sense in something like \"telemetry-lib\", but do you think we need a separate repository just for a single class? I hoped that since it was already here, that it could stay here.\n\nRegarding your suggestion for an argument. It could be done like that. I could leave the current behavior (to not break anything that\u0027s already using the modify_query without the additional argument), but if the metric_names are supplied in the argument, they\u0027d be used instead of querying for them. In that case the query for the metric names would be moved into whatever is calling the modify_query and that would then be able to handle loops and so on correctly.\n\nI\u0027m fine with that solution. If you think that\u0027s acceptable, I can change this patch to implement it.","commit_id":"9380ebdc3e2b5db0a753a371baf51b452185a24f"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"cc514013212f9eb93ad876decfffaab44e8f1531","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"19be1652_a9e7aa83","in_reply_to":"3b2bd23f_d48739bd","updated":"2025-07-07 14:17:58.000000000","message":"Alright. Thanks a lot for the reviews! I\u0027ll get this ready during tomorrow.","commit_id":"9380ebdc3e2b5db0a753a371baf51b452185a24f"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"b4e4a7576795256cdbd13499119035add345b7e7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3b2bd23f_d48739bd","in_reply_to":"5cc0bd32_89844478","updated":"2025-07-07 14:09:13.000000000","message":"I\u0027m not really encouraging creation of telemetry-lib. It may resolve a wrong expectation caused by the \"client\" naming, but may not resolve the need for tuning according to access pattern or guarantee that the cache can work for a specific access pattern.\n\nI prefer the \"pass metric_names to avoid query\" instead of the vague caching mechanism and exposing complicated tuning, as we know that\u0027s what works best for the specific access pattern we currently need.","commit_id":"9380ebdc3e2b5db0a753a371baf51b452185a24f"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"09cf0b47e653a435e894f021e2aaa26ce03c3894","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a4e35237_d936b11e","in_reply_to":"79f5b4b0_d8090ade","updated":"2025-07-01 07:43:44.000000000","message":"The rbac module is being used by services like Aetos or Aodh in the future to modify PromQL queries to add the project labels. This code isn\u0027t executed when interacting with the client from the CLI or when using the client to communicate with Prometheus.\n\nThe parsing of the PromQL is implemented in a way, where it needs a list of all the metric names currently stored in Prometheus so that it\u0027s able to distinguish what\u0027s a metric name, what\u0027s a PromQL function and where to put the labels in the end.\n\nThe issue I\u0027m trying to solve here is, that for example in certain parts of Aetos, like here: https://opendev.org/openstack/aetos/src/branch/master/aetos/controllers/api/v1/series.py#L69 (and everywhere else we work with the match[] parameter), we\u0027re executing the modify_query in a loop. That means, that each iteration, there is a request being sent to Prometheus (which can be quite costly) to retrieve the metric name list, even though it\u0027s very unlikely that it changed during iterating through the loop.\n\nI think the 10 seconds caching here is quite fine, but if you want, I can add a parameter like `metric_name_cache_ttl` to the `__init__` method, which I\u0027d then use to set the `ttl` parametere of the cache. This would enable each service using the code to configure the cache duration according to its needs.","commit_id":"9380ebdc3e2b5db0a753a371baf51b452185a24f"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"3972018a684f75109be10feb2ea0abef537a13dc","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1c7a5ffc_760521de","in_reply_to":"a4e35237_d936b11e","updated":"2025-07-06 12:29:43.000000000","message":"As long as we keep the code in something named client we should not implement a feature based on specific usage in service. (If we soft-forbid external usage then it should be named like \"telemetry-lib\").\n\nI also wonder if adding the caching mechanism is ideal in this case. Can it be better if we add an argument to provide fixed list of metric_names, instead of querying it ?","commit_id":"9380ebdc3e2b5db0a753a371baf51b452185a24f"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f32061a2314ef89c3ad120a0d3620d82e2e23fa3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"85a4ed8e_c1eb5a8e","updated":"2025-07-08 13:58:15.000000000","message":"Thanks","commit_id":"fabd89f11dd7e60135a964645d7e664703abb2d5"}]}