telemetry: correct json empty dictionaries

Message ID 20240118172602.898004-1-jonathan.erb@threater.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series telemetry: correct json empty dictionaries |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS

Commit Message

Jonathan Erb Jan. 18, 2024, 5:26 p.m. UTC
  Fix to allow telemetry to handle empty dictionaries correctly.

This patch resolves an issue where empty dictionaries are reported
by telemetry as '[]' rather than '{}'. Initializing the output
buffer based on the container type resolves the issue.

Signed-off-by: Jonathan Erb <jonathan.erb@threater.com>
---
 .mailmap                  | 2 +-
 lib/telemetry/telemetry.c | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)
  

Comments

Power, Ciara Jan. 19, 2024, 9:56 a.m. UTC | #1
> -----Original Message-----
> From: Jonathan Erb <jonathan.erb@threater.com>
> Sent: Thursday, January 18, 2024 5:26 PM
> To: Power, Ciara <ciara.power@intel.com>
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>;
> Jonathan Erb <jonathan.erb@threater.com>
> Subject: [PATCH] telemetry: correct json empty dictionaries
> 
> Fix to allow telemetry to handle empty dictionaries correctly.
> 
> This patch resolves an issue where empty dictionaries are reported by
> telemetry as '[]' rather than '{}'. Initializing the output buffer based on the
> container type resolves the issue.
> 
> Signed-off-by: Jonathan Erb <jonathan.erb@threater.com>
> ---
>  .mailmap                  | 2 +-
>  lib/telemetry/telemetry.c | 6 +++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index ab0742a382..a6b66ab3ad 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -675,7 +675,7 @@ John Ousterhout <ouster@cs.stanford.edu>  John
> Romein <romein@astron.nl>  John W. Linville <linville@tuxdriver.com>  Jonas
> Pfefferle <jpf@zurich.ibm.com> <pepperjo@japf.ch> -Jonathan Erb
> <jonathan.erb@threatblockr.com> <jonathan.erb@banduracyber.com>
> +Jonathan Erb <jonathan.erb@threater.com>
> +<jonathan.erb@threatblockr.com> <jonathan.erb@banduracyber.com>
>  Jonathan Tsai <jonathan1.tsai@intel.com>  Jon DeVree <nuxi@vault24.org>
> Jon Loeliger <jdl@netgate.com> diff --git a/lib/telemetry/telemetry.c
> b/lib/telemetry/telemetry.c index 92982842a8..eef4ac7bb7 100644
> --- a/lib/telemetry/telemetry.c
> +++ b/lib/telemetry/telemetry.c
> @@ -169,7 +169,11 @@ container_to_json(const struct rte_tel_data *d, char
> *out_buf, size_t buf_len)
>  		d->type != TEL_ARRAY_INT && d->type !=
> TEL_ARRAY_STRING)
>  		return snprintf(out_buf, buf_len, "null");
> 
> -	used = rte_tel_json_empty_array(out_buf, buf_len, 0);
> +	if (d->type == TEL_DICT)
> +		used = rte_tel_json_empty_obj(out_buf, buf_len, 0);
> +	else
> +		used = rte_tel_json_empty_array(out_buf, buf_len, 0);
> +
>  	if (d->type == TEL_ARRAY_UINT)
>  		for (i = 0; i < d->data_len; i++)
>  			used = rte_tel_json_add_array_uint(out_buf,
> --
> 2.34.1

Acked-by: Ciara Power <ciara.power@intel.com>
  
David Marchand Feb. 1, 2024, 11:57 a.m. UTC | #2
On Fri, Jan 19, 2024 at 8:48 AM Jonathan Erb <jonathan.erb@threater.com> wrote:
>
> Fix to allow telemetry to handle empty dictionaries correctly.
>
> This patch resolves an issue where empty dictionaries are reported
> by telemetry as '[]' rather than '{}'. Initializing the output
> buffer based on the container type resolves the issue.
>
> Signed-off-by: Jonathan Erb <jonathan.erb@threater.com>

Thanks for the fix.
Could you point at the change that introduced this issue?

I will add the Fixes: tag when applying.
  
Jonathan Erb Feb. 2, 2024, 2:04 p.m. UTC | #3
David,

It was just a minor oversight in following commit:

commit c933bb5177cac5fbf4533e357d466cff1f3e6c97
Author: Ciara Power <ciara.power@intel.com>
Date:   Wed Sep 23 12:12:26 2020 +0100

     telemetry: support array values in data object

     Arrays of type uint64_t/int/string can now be included within an array
     or dict. One level of embedded containers is supported. This is
     necessary to allow for instances such as the ethdev queue stats to be
     reported as a list of uint64_t values, rather than having multiple dict
     entries with one uint64_t value for each queue stat.

     The memory management APIs provided by telemetry simplify the memory
     allocation/free aspect of the embedded container. The 
rte_tel_data_alloc
     function is called in the library/app callback to return a pointer to a
     container that has been allocated memory. When adding this container
     to an array/dict, a parameter is passed to indicate if the memory
     should be freed by telemetry after use. This will allow reuse of the
     allocated memory if the library/app wishes to do so.

     Signed-off-by: Ciara Power <ciara.power@intel.com>
     Acked-by: Bruce Richardson <bruce.richardson@intel.com>

On 2/1/24 06:57, David Marchand wrote:
> On Fri, Jan 19, 2024 at 8:48 AM Jonathan Erb <jonathan.erb@threater.com> wrote:
>> Fix to allow telemetry to handle empty dictionaries correctly.
>>
>> This patch resolves an issue where empty dictionaries are reported
>> by telemetry as '[]' rather than '{}'. Initializing the output
>> buffer based on the container type resolves the issue.
>>
>> Signed-off-by: Jonathan Erb <jonathan.erb@threater.com>
> Thanks for the fix.
> Could you point at the change that introduced this issue?
>
> I will add the Fixes: tag when applying.
>
>
--
  
David Marchand Feb. 12, 2024, 10:15 a.m. UTC | #4
On Fri, Jan 19, 2024 at 8:48 AM Jonathan Erb <jonathan.erb@threater.com> wrote:
>
> Fix to allow telemetry to handle empty dictionaries correctly.
>
> This patch resolves an issue where empty dictionaries are reported
> by telemetry as '[]' rather than '{}'. Initializing the output
> buffer based on the container type resolves the issue.
>
Fixes: c933bb5177ca ("telemetry: support array values in data object")
Cc: stable@dpdk.org

> Signed-off-by: Jonathan Erb <jonathan.erb@threater.com>
Acked-by: Ciara Power <ciara.power@intel.com>

Applied thanks.
  

Patch

diff --git a/.mailmap b/.mailmap
index ab0742a382..a6b66ab3ad 100644
--- a/.mailmap
+++ b/.mailmap
@@ -675,7 +675,7 @@  John Ousterhout <ouster@cs.stanford.edu>
 John Romein <romein@astron.nl>
 John W. Linville <linville@tuxdriver.com>
 Jonas Pfefferle <jpf@zurich.ibm.com> <pepperjo@japf.ch>
-Jonathan Erb <jonathan.erb@threatblockr.com> <jonathan.erb@banduracyber.com>
+Jonathan Erb <jonathan.erb@threater.com> <jonathan.erb@threatblockr.com> <jonathan.erb@banduracyber.com>
 Jonathan Tsai <jonathan1.tsai@intel.com>
 Jon DeVree <nuxi@vault24.org>
 Jon Loeliger <jdl@netgate.com>
diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 92982842a8..eef4ac7bb7 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -169,7 +169,11 @@  container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len)
 		d->type != TEL_ARRAY_INT && d->type != TEL_ARRAY_STRING)
 		return snprintf(out_buf, buf_len, "null");
 
-	used = rte_tel_json_empty_array(out_buf, buf_len, 0);
+	if (d->type == TEL_DICT)
+		used = rte_tel_json_empty_obj(out_buf, buf_len, 0);
+	else
+		used = rte_tel_json_empty_array(out_buf, buf_len, 0);
+
 	if (d->type == TEL_ARRAY_UINT)
 		for (i = 0; i < d->data_len; i++)
 			used = rte_tel_json_add_array_uint(out_buf,