Make telemetry files path configurable without enabling debug mode (#6)

* Make telemetry files path configurable without enabling debug mode * Add .gitignore * Update .gitignore * Update expected test results * Update expected test results * Update expected test results * Refactoring * Post merge fixes * Fix * Fix
percona · May 23, 2024 · 04043fb · 04043fb
1 parent 3fd139d
commit 04043fb
Show file tree

Hide file tree

Showing 11 changed files with 60 additions and 34 deletions.
diff --git a/.gitignore b/.gitignore
@@ -0,0 +1,16 @@
+# Object files
+*.o
+*.ko
+*.obj
+*.elf
+*.dylib
+
+## IDE
+.idea
+
+## DS_Store
+*.DS_Store
+
+## .vscode
+.vscode/
+.vscode/*
diff --git a/README.md b/README.md
@@ -20,7 +20,7 @@ You can find the configuration parameters of the `percona_pg_telemetry` extensio
   * Unit: boolean
   * When set to `false`, the leader process terminates when the configuration is read.
 
-2. `percona_pg_telemetry.pg_telemetry_folder`
+2. `percona_pg_telemetry.path`
 
   * Default: /usr/local/percona/telemetry/pg
 
@@ -39,7 +39,7 @@ You can find the configuration parameters of the `percona_pg_telemetry` extensio
   * It forces the extension to maintain upto this number of files. Older files will get
     removed and new files will be created.
 
-> **NOTE** GUCs 2 - 4 are only visible if environment variable PT_DEBUG is set. This is prevent users in production environment to change these variables which are only for testing purposes.
+> **NOTE** GUCs 3 - 4 are only visible if environment variable PT_DEBUG is set. This is prevent users in production environment to change these variables which are only for testing purposes.
 
 #### Functions
 

diff --git a/expected/basic.out b/expected/basic.out
@@ -12,11 +12,11 @@ SELECT  pg_sleep(3);
 SELECT  name
 FROM    pg_settings
 WHERE   name LIKE 'percona_pg_telemetry.%';
-                   name                   
-------------------------------------------
+                 name                 
+--------------------------------------
  percona_pg_telemetry.enabled
  percona_pg_telemetry.files_to_keep
- percona_pg_telemetry.pg_telemetry_folder
+ percona_pg_telemetry.path
  percona_pg_telemetry.scrape_interval
 (4 rows)
 

diff --git a/expected/basic_1.out b/expected/basic_1.out
@@ -15,7 +15,8 @@ WHERE   name LIKE 'percona_pg_telemetry.%';
              name             
 ------------------------------
  percona_pg_telemetry.enabled
-(1 row)
+ percona_pg_telemetry.path
+(2 rows)
 
 SELECT  percona_pg_telemetry_version();
  percona_pg_telemetry_version 

diff --git a/expected/debug_json.out b/expected/debug_json.out
@@ -63,7 +63,7 @@ HAVING  COUNT(*) = (
                                             )
                                     AS INTEGER
                                 )
-                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name like '%pg_telemetry_folder') AS INTEGER)
+                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name = 'percona_pg_telemetry.path') AS INTEGER)
                 );
  databases_count 
 -----------------
@@ -76,7 +76,7 @@ FROM    pg_database
 WHERE   datallowconn = true
 HAVING  COUNT(*) = (
                     SELECT  json_array_length(read_json_file()::JSON->'databases')
-                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name like '%pg_telemetry_folder') AS INTEGER)
+                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name = 'percona_pg_telemetry.path') AS INTEGER)
                 );
  databases_count_calc 
 ----------------------

diff --git a/expected/gucs.out b/expected/gucs.out
@@ -34,7 +34,7 @@ max_val    | 100
 boot_val   | 7
 reset_val  | 7
 -[ RECORD 3 ]----------------------------------------------------
-name       | percona_pg_telemetry.pg_telemetry_folder
+name       | percona_pg_telemetry.path
 setting    | .
 unit       | 
 short_desc | Directory path for writing database info file(s)

diff --git a/expected/gucs_1.out b/expected/gucs_1.out
@@ -24,6 +24,15 @@ min_val    |
 max_val    | 
 boot_val   | on
 reset_val  | on
+-[ RECORD 2 ]-------------------------------------------------
+name       | percona_pg_telemetry.path
+setting    | /usr/local/percona/telemetry/pg
+unit       | 
+short_desc | Directory path for writing database info file(s)
+min_val    | 
+max_val    | 
+boot_val   | /usr/local/percona/telemetry/pg
+reset_val  | /usr/local/percona/telemetry/pg
 
 \x
 ALTER SYSTEM SET percona_pg_telemetry.enabled = false;

diff --git a/percona_pg_telemetry.c b/percona_pg_telemetry.c
@@ -162,7 +162,7 @@ generate_filename(char *filename)
     time(&currentTime);
     pg_snprintf(f_name, MAXPGPATH, "%s-%lu-%ld.json", PT_FILENAME_BASE, system_id, currentTime);
 
-    join_path_components(filename, ptss->pg_telemetry_folder, f_name);
+    join_path_components(filename, ptss->telemetry_path, f_name);
 
     return filename;
 }
@@ -229,22 +229,22 @@ cleaup_telemetry_dir(void)
     char json_file_id[MAXPGPATH];
     int file_id_len;
 
-    validate_dir(ptss->pg_telemetry_folder);
+    validate_dir(ptss->telemetry_path);
 
-    d = AllocateDir(ptss->pg_telemetry_folder);
+    d = AllocateDir(ptss->telemetry_path);
 
     if (d == NULL)
     {
         ereport(ERROR,
                         (errcode_for_file_access(),
                             errmsg("could not open percona telemetry directory \"%s\": %m",
-                                        ptss->pg_telemetry_folder)));
+                                        ptss->telemetry_path)));
     }
 
     pg_snprintf(json_file_id, sizeof(json_file_id), "%s-%lu", PT_FILENAME_BASE, system_id);
     file_id_len = strlen(json_file_id);
 
-    while ((de = ReadDir(d, ptss->pg_telemetry_folder)) != NULL)
+    while ((de = ReadDir(d, ptss->telemetry_path)) != NULL)
     {
         if (strncmp(json_file_id, de->d_name, file_id_len) == 0)
         {
@@ -256,7 +256,7 @@ cleaup_telemetry_dir(void)
 }
 
 /*
- * pg_telemetry_folder
+ * telemetry_path
  */
 bool
 validate_dir(char *folder_path)
@@ -274,7 +274,7 @@ validate_dir(char *folder_path)
     {
         ereport(LOG,
                 (errcode_for_file_access(),
-                 errmsg("percona_pg_telemetry.pg_telemetry_folder \"%s\" is not set to a writeable folder or the folder does not exist.", folder_path)));
+                 errmsg("percona_pg_telemetry.path \"%s\" is not set to a writeable folder or the folder does not exist.", folder_path)));
 
         PT_WORKER_EXIT(PT_FILE_ERROR);
     }
@@ -359,8 +359,8 @@ pt_shmem_init(void)
         uint64 system_id = GetSystemIdentifier();
 
         /* Set paths */
-        strncpy(ptss->pg_telemetry_folder, t_folder, MAXPGPATH);
-        pg_snprintf(ptss->dbtemp_filepath, MAXPGPATH, "%s/%s-%lu.temp", ptss->pg_telemetry_folder, PT_FILENAME_BASE, system_id);
+        strncpy(ptss->telemetry_path, t_folder, MAXPGPATH);
+        pg_snprintf(ptss->dbtemp_filepath, MAXPGPATH, "%s/%s-%lu.temp", ptss->telemetry_path, PT_FILENAME_BASE, system_id);
 
         /* Let's be optimistic here. No error code and no file currently being written. */
         ptss->error_code = PT_SUCCESS;
@@ -396,21 +396,21 @@ init_guc(void)
                              NULL,
                              NULL);
 
+    /* telemetry files path */
+    DefineCustomStringVariable("percona_pg_telemetry.path",
+                            "Directory path for writing database info file(s)",
+                            NULL,
+                            &t_folder,
+                            PT_DEFAULT_FOLDER_PATH,
+                            PGC_SIGHUP,
+                            0,
+                            NULL,
+                            NULL,
+                            NULL);
+
     env = getenv("PT_DEBUG");
     if (env != NULL)
     {
-        /* file path */
-        DefineCustomStringVariable("percona_pg_telemetry.pg_telemetry_folder",
-                                "Directory path for writing database info file(s)",
-                                NULL,
-                                &t_folder,
-                                PT_DEFAULT_FOLDER_PATH,
-                                PGC_SIGHUP,
-                                0,
-                                NULL,
-                                NULL,
-                                NULL);
-
         /* scan time interval for the main launch process */
         DefineCustomIntVariable("percona_pg_telemetry.scrape_interval",
                                 "Data scrape interval",

diff --git a/percona_pg_telemetry.conf b/percona_pg_telemetry.conf
@@ -1,3 +1,3 @@
 shared_preload_libraries = 'percona_pg_telemetry'
-percona_pg_telemetry.pg_telemetry_folder = '.'
+percona_pg_telemetry.path = '.'
 percona_pg_telemetry.scrape_interval = 1
diff --git a/percona_pg_telemetry.h b/percona_pg_telemetry.h
@@ -61,7 +61,7 @@ typedef struct PTSharedState
     bool write_in_progress;
     TimestampTz last_file_processed;
     int curr_file_index;
-    char pg_telemetry_folder[MAXPGPATH];
+    char telemetry_path[MAXPGPATH];
     char dbtemp_filepath[MAXPGPATH];
     char telemetry_filenames[FLEXIBLE_ARRAY_MEMBER][MAXPGPATH];
 } PTSharedState;

diff --git a/sql/debug_json.sql b/sql/debug_json.sql
@@ -50,7 +50,7 @@ HAVING  COUNT(*) = (
                                             )
                                     AS INTEGER
                                 )
-                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name like '%pg_telemetry_folder') AS INTEGER)
+                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name = 'percona_pg_telemetry.path') AS INTEGER)
                 );
 -- Databases count will fail if you have any preexisting databases other than the standard template1 and postgres
 
@@ -59,7 +59,7 @@ FROM    pg_database
 WHERE   datallowconn = true
 HAVING  COUNT(*) = (
                     SELECT  json_array_length(read_json_file()::JSON->'databases')
-                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name like '%pg_telemetry_folder') AS INTEGER)
+                            + CAST(NOT EXISTS (SELECT * FROM pg_settings where name = 'percona_pg_telemetry.path') AS INTEGER)
                 );
 -- Databases count will fail if you have any preexisting databases other than the standard template1 and postgres