DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Bruce Richardson <bruce.richardson@intel.com>, dev@dpdk.org
Subject: Re: [PATCH 2/2] doc/contributing: guidelines for logging, tracing and telemetry
Date: Wed, 14 Jun 2023 12:35:51 +0100	[thread overview]
Message-ID: <5f70df87-72c6-7504-5972-0ac042a4142a@amd.com> (raw)
In-Reply-To: <20230613143355.77914-3-bruce.richardson@intel.com>

On 6/13/2023 3:33 PM, Bruce Richardson wrote:
> As discussed by DPDK technical board [1], out contributor guide should
> include some details as to when to use logging vs tracing vs telemetry
> to provide the end user with information about the running process and
> the DPDK libraries it uses.
> 
> [1] https://mails.dpdk.org/archives/dev/2023-March/265204.html
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  doc/guides/contributing/coding_style.rst |  2 ++
>  doc/guides/contributing/design.rst       | 34 ++++++++++++++++++++++++
>  doc/guides/prog_guide/telemetry_lib.rst  |  2 ++
>  doc/guides/prog_guide/trace_lib.rst      |  2 ++
>  4 files changed, 40 insertions(+)
> 
> diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
> index f50a78a346..13b2390d9e 100644
> --- a/doc/guides/contributing/coding_style.rst
> +++ b/doc/guides/contributing/coding_style.rst
> @@ -794,6 +794,8 @@ Control Statements
>                   /* NOTREACHED */
>           }
>  
> +.. _dynamic_logging:
> +
>  Dynamic Logging
>  ---------------
>  
> diff --git a/doc/guides/contributing/design.rst b/doc/guides/contributing/design.rst
> index d24a7ff6a0..9edf3ea43b 100644
> --- a/doc/guides/contributing/design.rst
> +++ b/doc/guides/contributing/design.rst
> @@ -4,6 +4,40 @@
>  Design
>  ======
>  
> +Runtime Information - Logging, Tracing and Telemetry
> +-------------------------------------------------------
> +
> +It is often desirable to provide information to the end-user as to what is happening to the application at runtime.
>

I think target audience matters, who is consumer of the runtime
information, possible candidates I can think of:
- dev-ops engineer deploys and maintains a dpdk application
- DPDK based application developer
- DPDK developer

Should we distinguish them? Like I guess telemetry output can be more
useful to dev-ops engineer, but debug output can be more useful to
application developer.

To roughly set target consumer for each method may help developer to
decide which one to use, but not sure if we can narrow it like this.

> +DPDK provides a number of built-in mechanisms to provide this introspection:
> +
> +* :ref:`Logging<dynamic_logging>`
> +* :ref:`Tracing<trace_library>`
> +* :ref:`Telemetry<telemetry_library>`
> +
> +Each of these has it's own strengths and suitabilities for use within DPDK components.
> +
> +Below are some guidelines for when each should be used:
> +
> +* For reporting error conditions, or other abnormal runtime issues, *logging* should be used.
> +  Depending on the severity of the issue, the appropriate log level, for example,
> +  ``ERROR``, ``WARNING`` or ``NOTICE``, should be used.

+1

> +* In general, it is not recommended that the DPDK logging functions should be used for debugging.
> +  Although the ``DEBUG`` log level may be used in components, it should only be used sparingly,
> +  and the *tracing* functionality used instead.

I think DEBUG logging is still useful for control path, 'tracing' is
missing the custom message, so I found it more useful for to analyze
order and frequency of a function call, which suits more to datapath.

> +  More specifically:
> +
> +  * for cases where a path through the code is only likely to be taken once,
> +    for example, initialization code, either *logging* at ``DEBUG`` level or *tracing* may be used -
> +    though tracing is preferred.>

I am for preferring 'logging' for this case, because there is extra
effort required to analyze the trace output, but logs are simple and
just there.

> +  * where a path is to be taken multiple times within a short timeframe, for example,
> +    datapath or regular control plane code, *tracing* should be used.
> +
>

+1 to use tracing mainly for datapath

Also there is logging support for datapath, it would be nice to clarify
that usage too. Datapath logging used:
a) RTE_LOG_DP macros where code compiled out based on selected log level
b) 'RTE_ETHDEV_DEBUG_RX' & 'RTE_ETHDEV_DEBUG_TX' macros

ethdev layer use b for logging,
drivers use combination of a & b

Tracing helps on datapath to get profiling etc, but there are cases
driver needs to log some variables, like descriptor details to ensure
*functionality* is correct.


I suggest to remove/deprecate RTE_LOG_DP [1], and use
'RTE_ETHDEV_DEBUG_x' macros for developer logging, and use tracing for
profiling. What do you think?


[1]
Intention is good with RTE_LOG_DP macro to set the desired log level and
compile out not used logs so that they won't take extra cycle.
But every time log level changed, it requires compiling again anyway,
and in practice there is no multiple level logging need for datapath,
most of the time single macro to enable/disable logging is sufficient.
That why I believe simpler to use an explicit macro to enable/disable
datapath logging, like 'RTE_ETHDEV_DEBUG_RX' & 'RTE_ETHDEV_DEBUG_TX'.



> +* For numerical or statistical data generated by a component, for example, per-packet statistics,
> +  *telemetry* should be used.
>

+1


> +* For any data where the data may need to be gathered at any point in the execution to help assess the state of the application component,
> +  for example, core configuration, device information, *telemetry* should be used.
> +

+1

Should we explicitly state that telemetry only used as readonly?
I think it implicitly states, but since there was a patch in the past, I
wonder if this needs to be highlighted?


  parent reply	other threads:[~2023-06-14 11:36 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13 14:33 [PATCH 0/2] Improve docs on getting info on running process Bruce Richardson
2023-06-13 14:33 ` [PATCH 1/2] doc/contributing: provide coding details for dynamic logging Bruce Richardson
2023-06-13 14:33 ` [PATCH 2/2] doc/contributing: guidelines for logging, tracing and telemetry Bruce Richardson
2023-06-13 15:21   ` Stephen Hemminger
2023-06-13 19:38   ` Morten Brørup
2023-06-14  8:36     ` Bruce Richardson
2023-06-14  9:39       ` Morten Brørup
2023-06-14 11:35   ` Ferruh Yigit [this message]
2023-06-15 11:51   ` Jerin Jacob
2023-06-20 17:07 ` [PATCH v2 0/2] Improve docs on getting info on running process Bruce Richardson
2023-06-20 17:07   ` [PATCH v2 1/2] doc/contributing: provide coding details for dynamic logging Bruce Richardson
2023-07-04  6:20     ` fengchengwen
2023-07-04  7:46     ` David Marchand
2023-06-20 17:07   ` [PATCH v2 2/2] doc/contributing: guidelines for logging, tracing and telemetry Bruce Richardson
2023-07-04  7:54     ` David Marchand
2023-07-18 16:48 ` [PATCH v3 0/2] Improve docs on getting info on running process Bruce Richardson
2023-07-18 16:48   ` [PATCH v3 1/2] doc/contributing: provide coding details for dynamic logging Bruce Richardson
2023-07-18 20:23     ` Ferruh Yigit
2023-07-18 16:48   ` [PATCH v3 2/2] doc/contributing: guidelines for logging, tracing and telemetry Bruce Richardson
2023-07-18 20:23     ` Ferruh Yigit
2023-07-19  1:07     ` lihuisong (C)
2023-07-20 10:57       ` Jerin Jacob
2023-07-18 17:40   ` [PATCH v3 0/2] Improve docs on getting info on running process Stephen Hemminger
2023-07-19  8:32     ` Bruce Richardson
2023-07-19 14:01     ` Bruce Richardson
2023-07-20 11:05 ` [PATCH v4 " Bruce Richardson
2023-07-20 11:05   ` [PATCH v4 1/2] doc/contributing: provide coding details for dynamic logging Bruce Richardson
2023-07-20 11:05   ` [PATCH v4 2/2] doc/contributing: guidelines for logging, tracing and telemetry Bruce Richardson
2023-07-25  9:51     ` Thomas Monjalon
2023-07-20 12:38   ` [PATCH v4 0/2] Improve docs on getting info on running process Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5f70df87-72c6-7504-5972-0ac042a4142a@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).