From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A5BEA64822 for ; Fri, 28 Jan 2022 11:19:50 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9711B2D284 for ; Fri, 28 Jan 2022 11:19:20 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id CF09B2D279 for ; Fri, 28 Jan 2022 11:19:19 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A590F46D4B for ; Fri, 28 Jan 2022 11:19:19 +0100 (CET) Message-ID: <0b6b466b-598a-7ec0-b83c-b7a3832a450e@proxmox.com> Date: Fri, 28 Jan 2022 11:19:18 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:97.0) Gecko/20100101 Thunderbird/97.0 Content-Language: en-US To: Proxmox VE development discussion , Markus Frank References: <20220128100339.21230-1-m.frank@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20220128100339.21230-1-m.frank@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.061 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [influxdata.com, influxdb.pm] Subject: [pve-devel] applied: [PATCH container v3] fix #3815: influxdb vmname should be string X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Jan 2022 10:19:50 -0000 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 > --- ----> 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!