Re: [dpdk-dev] [PATCH v2] doc: add more detail to telemetry guides

[Thomas Monjalon <thomas@monjalon.net>]

21/07/2020 18:09, Ciara Power:
> --- a/doc/guides/howto/telemetry.rst
> +++ b/doc/guides/howto/telemetry.rst
> @@ -1,6 +1,7 @@
>  ..  SPDX-License-Identifier: BSD-3-Clause
>      Copyright(c) 2020 Intel Corporation.
>  
> +.. _telemetry:

We don't need such anchor.
The beginning of a page can be linked via :doc: syntax.
Note: I would support a patch removing all such existing anchors
at beginning of files.

[...]
> +   * List all commands.
> +
> +      .. code-block:: console

I think you can achieve the same result as code-block
with adding double colons (::) at the end of the previous line:
	* List all commands::

[...]
> --- a/doc/guides/prog_guide/telemetry_lib.rst
> +++ b/doc/guides/prog_guide/telemetry_lib.rst
> +The parameters for a callback function are:
> +
> +* **cmd**
> +
> +  This is the command requested by the client, e.g. "/ethdev/list".
> +  For most callbacks this may be unused, however it will allow for handling
> +  multiple commands in one callback function. This can be seen in the example
> +  callback below.
> +
> +* **params**
> +
> +  This will contain any parameters required for the command. For example
> +  when calling "/ethdev/link_status,0", the port ID will be passed to the
> +  callback function in params. This can be seen in the example callback below.
> +
> +* **d**
> +
> +  The rte_tel_data pointer will be used by the callback function to format the
> +  requested data to be returned to Telemetry. The data APIs provided will
> +  enable adding to the struct, examples of this are shown later in this
> +  document.

It looks like a doxygen-level comment.
Why not linking doxygen from here and focus on example in this guide?
Note that we can also link example code from doxygen.

> +
> +**Example Callback**
> +
> +This callback is an example of handling multiple commands in one callback,
> +and also shows the use of params which holds a port ID. The params input needs
> +to be validated and converted to the required integer type for port ID. The cmd
> +parameter is then used in a comparison to decide which command was requested,
> +which will decide what port information should fill the rte_tel_data structure.
> +
> +.. code-block:: c
> +
> +   int
> +   handle_cmd_request(const char *cmd, const char *params,
> +         struct rte_tel_data *d)
> +   {
> +      int port_id, used = 0;
> +
> +      if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> +         return -1;
> +
> +      port_id = atoi(params);
> +      if (!rte_eth_dev_is_valid_port(port_id))
> +         return -1;
> +
> +      if (strcmp(cmd, "/cmd_1") == 0)
> +         /* Build up port data requested for command 1 */
> +      else
> +         /* Build up port data requested for command 2 */
> +
> +       return used;
> +   }
> +
> +
> +Formatting Data
> +~~~~~~~~~~~~~~~
> +
> +The callback function provided by the library must format its telemetry
> +information in the required data format. The Telemetry library provides a data
> +utilities API to build up the data structure with the required information.
> +The telemetry library is then responsible for formatting the data structure
> +into a JSON response before sending to the client.
> +
> +* **Array Data**

It looks like a sub-title.
Please prefer sub-titles because they appear in the menu
and can be referenced with a direct HTML link.


[...]
> +  .. code-block:: c
> +
> +     rte_tel_data_start_dict(d);
> +     rte_tel_data_add_dict_string(d, "version", rte_version());
> +     rte_tel_data_add_dict_int(d, "pid", getpid());
> +     rte_tel_data_add_dict_int(d, "max_output_len", MAX_OUTPUT_LEN);
> +
> +  The resulting response to the client shows the key/value data provided above
> +  by the handler function in telemetry, placed in a JSON reply by telemetry:
> +
> +  .. code-block:: console
> +
> +     {"/info": {"version": "DPDK 20.08.0-rc0", "pid": 3838, "max_output_len": 16384}}

In general I prefer avoiding real code because it will look
out-of-date quickly.
Why not demonstrating telemetry with a cooking recipe or NASA rocket?

[...]
> +For more information on the range of data functions available in the API,
> +please refer to the docs.

"the docs"? Can we add a link?