Re: POC: Extension for adding distributed tracing - pg_tracing - Mailing list pgsql-hackers

From Nikita Malakhov
Subject Re: POC: Extension for adding distributed tracing - pg_tracing
Date
Msg-id CAN-LCVOTwRmddkugJ7FwxPbWQ_s_jG_v54s3ifY3t+-basBceA@mail.gmail.com
Whole thread Raw
In response to Re: POC: Extension for adding distributed tracing - pg_tracing  (Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>)
List pgsql-hackers
Hi!

It's a good idea to split a big patch into several smaller ones.
But you've already implemented these features - you could provide them
as sequential small patches (i.e. v13-0002-guc-context-propagation.patch and so on)

Great job! I'm both hands for committing your patch set.

On Fri, Jan 26, 2024 at 1:17 PM Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> wrote:
Hi,

The last CFbot failure was on pg_upgrade/002_pg_upgrade and doesn't
seem related to the patch.

#   Failed test 'regression tests pass'
#   at C:/cirrus/src/bin/pg_upgrade/t/002_pg_upgrade.pl line 212.
#          got: '256'
#     expected: '0'
# Looks like you failed 1 test of 18.

Given that the patch grew a bit in complexity and features, I've tried
to reduce it to a minimum to make reviewing easier. The goal is to
keep the scope focused for a first version while additional features
and changes could be handled afterwards.

For this version:
- Trace context can be propagated through SQLCommenter
- Sampling decision uses either a global sampling rate or on
SQLCommenter's sampled flag
- We generate spans for Planner, ExecutorRun and ExecutorFinish

What was put on hold for now:
- Generate spans from planstate. This means we don't need the change
in instrument.h to track the start of a node for the first version.
- Generate spans for parallel processes
- Using a GUC variable to propagate trace context
- Filtering sampling on queryId

With this, the current patch provides the core to:
- Generate, store and export spans with basic buffer management (drop
old spans when full or drop new spans when full)
- Keep variable strings in a separate file (a la pg_stat_statements)
- Track tracing state and top spans across nested execution levels

Thoughts on this approach?

On Mon, Jan 22, 2024 at 7:45 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> 2024-01 Commitfest.
>
> Hi, This patch has a CF status of "Needs Review" [1], but it seems
> there were CFbot test failures last time it was run [2]. Please have a
> look and post an updated version if necessary.
>
> ======
> [1] https://commitfest.postgresql.org/46/4456/
> [2] https://cirrus-ci.com/task/5581154296791040
>
> Kind Regards,
> Peter Smith.


--
Regards,
Nikita Malakhov
Postgres Professional
The Russian Postgres Company

pgsql-hackers by date:

Previous
From: Jeevan Chalke
Date:
Subject: Re: More new SQL/JSON item methods
Next
From: vignesh C
Date:
Subject: Re: A new strategy for pull-up correlated ANY_SUBLINK