prometheus / prometheus/client_python

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

Abierto
#1,049 1 comentario 1 reacción 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
Python
Estrellas
4.4k
Forks
876
Merge medio
8 d 4 h
PR fusionados (30 d)
1

Descripción

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.

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza con la implementación enlazada de InfoMetricFamily.add_metric() en prometheus_client/metrics_core.py alrededor de la línea 372 y compara después su gestión de parámetros con Info. Confirma los valores predeterminados previstos y las implicaciones de compatibilidad para labels y value; se considera terminado cuando se haya implementado un cambio de API acordado sin romper los llamadores existentes y su comportamiento esté cubierto por pruebas.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
python
Área
observability-sre
Tipo de issue
Nueva funcionalidad
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
35/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.