[pve-devel] applied: [PATCH container v3] fix #3815: influxdb vmname should be string

[Thomas Lamprecht]

On 28.01.22 11:03, Markus Frank wrote:
> InfluxDB interprets the vmname 66601 as a number and the vmname vm42 as a String.

note, above line was still to long, fixed that on applying

> This leads to problematic metrics, that will be dropped by influxdb.
> Whichever comes first decides how the "schema" is defined.
> 
> To change that I added a $to_quote hashmap to define which value
> shouldn't get interpreted as number.
> In this case the value of name.
> 
> Change: Conversion happends in prepare_value.

FYI, changelogs for patch revisions should rather go --->

> 
> nodename and host are tags in InfluxDB so the only value they are able
> to contain are strings:
> https://docs.influxdata.com/influxdb/v2.1/reference/syntax/line-protocol/
> 
> Signed-off-by: Markus Frank <m.frank at proxmox.com>
> ---


----> here, as they are only relevant for review and do not matter for
the actual committed patch, albeit the reason for the final approach
may matter (but that can be seen as independent from the review history)

>  PVE/Status/InfluxDB.pm | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
>

applied, thanks! 

> @@ -331,9 +333,10 @@ sub get_recursive_values {
>  }
>  
>  sub prepare_value {
> -    my ($value) = @_;
> +    my ($value, $quote) = @_;

IMO $quote is not ideal here as it implies that we only quote if that param
is true, so I renamed it to $force_quote in a followup commit

>  
> -    if (looks_like_number($value)) {
> +    # don't treat value like a number if quote is 1
> +    if (looks_like_number($value) && !$quote) {

Ordering the !$force_quote early allows perl to skip the more expensive 
looks_like_number check if quoting is forced anyway (fixed in the same followup
I made for renaming the parameter name).

Anyhow, just rather small nits and mentioning it here only fyi/for the record.
Thx!