From: | Nikita Malakhov <hukutoc(at)gmail(dot)com> |
---|---|
To: | Anthonin Bonnefoy <anthonin(dot)bonnefoy(at)datadoghq(dot)com> |
Cc: | Peter Smith <smithpb2250(at)gmail(dot)com>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
Subject: | Re: POC: Extension for adding distributed tracing - pg_tracing |
Date: | 2024-01-26 12:42:50 |
Message-ID: | CAN-LCVOTwRmddkugJ7FwxPbWQ_s_jG_v54s3ifY3t+-basBceA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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(dot)bonnefoy(at)datadoghq(dot)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(at)gmail(dot)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
https://postgrespro.ru/
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2024-01-26 12:46:02 | Re: A new strategy for pull-up correlated ANY_SUBLINK |
Previous Message | Jeevan Chalke | 2024-01-26 12:37:15 | Re: More new SQL/JSON item methods |