prometheus / prometheus/client_python

`InfoMetricFamily`’s `add_metric()` parameters are strangely named and should provide default values

未关闭
#1,049 1 条评论 1 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

主要语言
Python
星标
4.4k
派生
876
平均合并
8 天 4 小时
30 天内合并 PR
1

描述

Hey.

I was looking into InfoMetricFamily’s add_metric() and I either I just don't get it or the API is a bit strange.

With an Info object I'd have done something like this:

m = prometheus_client.Info("logical_drive", "foobar", ("controller_name",
                                                       "array_name",
                                                       "logical_drive_name",
                                                       "caching",
                                                       "device",
                                                       "raid_level",
                                                       "logical_drive_label",
                                                       "multidomain_status",
                                                       "parity_initialization_status",
                                                       "status"
                                                      )
                      )

and then set it like:

m.labels(controller_name=controller_name, array_name=array_name, logical_drive_name=logical_drive_name,
         **{n: logical_drive_properties.get(n, "")  for n in (
                                                              "device",
                                                              "raid_level",
                                                              "logical_drive_label",
                                                              "multidomain_status",
                                                              "parity_initialization_status",
                                                              "status"
                                                             )
           },
         caching=bool_or_none_to_label_value( logical_drive_properties.get("caching") )
        )

(here an example where I set some of the properties directly, and some via dict unpacking)

  • It would fail if I forgot a label.
  • I could use label names as attribute names, like controller_name= rather than "controller_name"=... which is however not super important

With InfoMetricFamily seem quite a bit different:

  • add_metric() now has the parameters labels and value.
  • As far as I understand the code, labels are actually not labels, but values for these, namely the ones set with labels in the constructor of InfoMetricFamily.
  • It's no longer possible to give labels as dict, one really needs to give them in the right order as sequence. Why does Info allow that but not this?
  • AFAICS, there is no check if exactly those labels are set, that were given in the constructor. Why over at Info but not here? What sense does it then even make to set the labels in the constructor?
  • value is misleading in so many ways. It's not the value of the metric (that is 1) and even if it relates to the labels being the “value”, then it should be values.
  • It might have perhaps even been better to swap the two names... or rater use some completely different names.

Anyway... guess this can't be changed now without breaking the ABI.

However, as far as I understand the code:
https://github.com/prometheus/client_python/blob/09a5ae30602a7a81f6174dae4ba08b93ee7feed2/prometheus_client/metrics_core.py#L372

the idea is one can give labels and/or value and both are merged in a final dict of label/value pairs.... but then it would be nice if labels and value were not required.

Why not simply give them default values () respectively {}?

I could provide a patch if nothing speaks against that particular change. But the above points are IMO still problematic. Especially also that the behaviour is considerable different from Info, which is not really obvious.

Cheers,
Chris.

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

从 prometheus_client/metrics_core.py 第 372 行附近链接的 InfoMetricFamily.add_metric() 实现开始,然后将其参数处理与 Info 进行比较。确认 labels 和 value 的预期默认值及兼容性影响;完成的标准是,在不破坏现有调用方的情况下实现已达成一致的 API 变更,并通过测试覆盖其行为。

由索引模型根据 Issue 内容生成。

评估

技术栈
python
领域
observability-sre
Issue 类型
功能
难度
4/5
预计耗时
3-5 天
活跃度
停滞
描述清晰度
基本清楚
新手友好度
35/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。