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.bonne...@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 <smithpb2...@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 https://postgrespro.ru/
