tuhaihe commented on code in PR #1629:
URL: https://github.com/apache/cloudberry/pull/1629#discussion_r2980005009
##########
pom.xml:
##########
@@ -154,6 +154,12 @@ code or new licensing patterns.
<exclude>gpcontrib/gp_exttable_fdw/gp_exttable_fdw.control</exclude>
<exclude>gpcontrib/diskquota/**</exclude>
+
<exclude>gpcontrib/yagp_hooks_collector/yagp_hooks_collector.control</exclude>
+
<exclude>gpcontrib/yagp_hooks_collector/protos/yagpcc_set_service.proto</exclude>
+
<exclude>gpcontrib/yagp_hooks_collector/protos/yagpcc_plan.proto</exclude>
+
<exclude>gpcontrib/yagp_hooks_collector/protos/yagpcc_metrics.proto</exclude>
+ <exclude>gpcontrib/yagp_hooks_collector/.clang-format</exclude>
+ <exclude>gpcontrib/yagp_hooks_collector/Makefile</exclude>
Review Comment:
can move these new lines to the `Cloudberry` part, like:
https://github.com/apache/cloudberry/blob/6c37f8f6732bbe18387b5500838409153793ec13/pom.xml#L1263-L1272
##########
gpcontrib/yagp_hooks_collector/protos/yagpcc_metrics.proto:
##########
@@ -0,0 +1,185 @@
+syntax = "proto3";
+
+package yagpcc;
+option java_outer_classname = "SegmentYAGPCCM";
+option go_package =
"a.yandex-team.ru/cloud/mdb/yagpcc/api/proto/common;greenplum";
Review Comment:
Line 5: Is it private？ If so, do we need to make it public?
##########
gpcontrib/yagp_hooks_collector/src/stat_statements_parser/README.md:
##########
@@ -0,0 +1 @@
+This directory contains a slightly modified subset of pg_stat_statements for
PG v9.4 to be used in query and plan ID generation.
Review Comment:
Can add the standard ASF license header to the markdown file, like:
https://github.com/apache/cloudberry/blob/6c37f8f6732bbe18387b5500838409153793ec13/CODE_OF_CONDUCT.md?plain=1#L1-L18
##########
.github/workflows/build-cloudberry.yml:
##########
@@ -271,6 +271,10 @@ jobs:
},
"enable_core_check":false
},
+ {"test":"gpcontrib-yagp-hooks-collector",
+ "make_configs":["gpcontrib/yagp_hooks_collector:installcheck"],
+ "extension":"yagp_hooks_collector"
+ },
Review Comment:
Is there a need to add the new test to `build-cloudberry-rocky8.yml` or
`*.*-deb.yml` file? FYI.
##########
gpcontrib/yagp_hooks_collector/src/stat_statements_parser/README.md:
##########
@@ -0,0 +1 @@
+This directory contains a slightly modified subset of pg_stat_statements for
PG v9.4 to be used in query and plan ID generation.
Review Comment:
Can add an ASF license header. FYI.
##########
Makefile:
##########
@@ -3,14 +3,12 @@
# to build Postgres with a different make, we have this make file
# that, as a service, will look for a GNU make and invoke it, or show
# an error message if none could be found.
-
Review Comment:
If no actual changes to Makefile, we can restore it.
##########
gpcontrib/yagp_hooks_collector/README.md:
##########
@@ -0,0 +1,28 @@
+## YAGP Hooks Collector
+
+An extension for collecting greenplum query execution metrics and reporting
them to an external agent.
+
+### Collected Statistics
+
+#### 1. Query Lifecycle
+- **What:** Captures query text, normalized query text, timestamps (submit,
start, end, done), and user/database info.
+- **GUC:** `yagpcc.enable`.
+
+#### 2. `EXPLAIN` data
+- **What:** Triggers generation of the `EXPLAIN (TEXT, COSTS, VERBOSE)` and
captures it.
+- **GUC:** `yagpcc.enable`.
+
+#### 3. `EXPLAIN ANALYZE` data
+- **What:** Triggers generation of the `EXPLAIN (TEXT, ANALYZE, BUFFERS,
TIMING, VERBOSE)` and captures it.
+- **GUCs:** `yagpcc.enable`, `yagpcc.min_analyze_time`,
`yagpcc.enable_cdbstats`(ANALYZE), `yagpcc.enable_analyze`(BUFFERS, TIMING,
VERBOSE).
+
+#### 4. Other Metrics
+- **What:** Captures Instrument, Greenplum, System, Network, Interconnect,
Spill metrics.
Review Comment:
```suggestion
- **What:** Captures Instrument, Apache Cloudberry (Greenplum), System,
Network, Interconnect, Spill metrics.
```
##########
gpcontrib/yagp_hooks_collector/metric.md:
##########
@@ -0,0 +1,126 @@
+## YAGP Hooks Collector Metrics
Review Comment:
Can add an ASF licenser header too. FYI.
##########
gpcontrib/yagp_hooks_collector/README.md:
##########
@@ -0,0 +1,28 @@
+## YAGP Hooks Collector
+
+An extension for collecting greenplum query execution metrics and reporting
them to an external agent.
Review Comment:
```suggestion
An extension for collecting Apache Cloudberry/Greenplum query execution
metrics and reporting them to an external agent.
```
##########
configure.ac:
##########
@@ -1365,6 +1365,13 @@ PGAC_ARG_BOOL(with, zstd, yes, [do not build with
Zstandard],
AC_MSG_RESULT([$with_zstd])
AC_SUBST(with_zstd)
+#
+# yagp_hooks_collector
+#
+PGAC_ARG_BOOL(with, yagp_hooks_collector, no,
+ [build with YAGP hooks collector extension])
Review Comment:
```suggestion
[build with hooks collector extension])
```
?
##########
gpcontrib/yagp_hooks_collector/yagp_hooks_collector.control:
##########
@@ -0,0 +1,5 @@
+# yagp_hooks_collector extension
+comment = 'Intercept query and plan execution hooks and report them to Yandex
GPCC agents'
Review Comment:
```suggestion
comment = 'Intercept query and plan execution hooks and report them to
Cloudberry monitor agents'
```
?
##########
configure.ac:
##########
@@ -1365,6 +1365,13 @@ PGAC_ARG_BOOL(with, zstd, yes, [do not build with
Zstandard],
AC_MSG_RESULT([$with_zstd])
AC_SUBST(with_zstd)
+#
+# yagp_hooks_collector
Review Comment:
Does this new extension need other dependencies? Not sure about this. If so,
we can add the pre-required dependencies here. FYI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
