NJrslv commented on code in PR #1629: URL: https://github.com/apache/cloudberry/pull/1629#discussion_r3008822730
########## gpcontrib/gp_stats_collector/src/Config.cpp: ########## @@ -0,0 +1,177 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * Config.cpp + * + * IDENTIFICATION + * gpcontrib/gp_stats_collector/src/Config.cpp + * + *------------------------------------------------------------------------- + */ + +#include "Config.h" +#include "memory/gpdbwrappers.h" +#include <limits.h> +#include <memory> +#include <string> +#include <unordered_set> + +extern "C" { +#include "postgres.h" +#include "utils/guc.h" +} + +static char *guc_uds_path = nullptr; +static bool guc_enable_analyze = true; +static bool guc_enable_cdbstats = true; +static bool guc_enable_collector = true; +static bool guc_report_nested_queries = true; +static char *guc_ignored_users = nullptr; +static int guc_max_text_size = 1 << 20; // in bytes (1MB) +static int guc_max_plan_size = 1024; // in KB +static int guc_min_analyze_time = 10000; // in ms +static int guc_logging_mode = LOG_MODE_UDS; +static bool guc_enable_utility = false; + +static const struct config_enum_entry logging_mode_options[] = { + {"uds", LOG_MODE_UDS, false /* hidden */}, + {"tbl", LOG_MODE_TBL, false}, + {NULL, 0, false}}; + +static bool ignored_users_guc_dirty = false; + +static void assign_ignored_users_hook(const char *, void *) { + ignored_users_guc_dirty = true; +} + +void Config::init_gucs() { + DefineCustomStringVariable( + "gpsc.uds_path", "Sets filesystem path of the agent socket", 0LL, + &guc_uds_path, "/tmp/gpsc_agent.sock", PGC_SUSET, + GUC_NOT_IN_SAMPLE | GUC_GPDB_NEED_SYNC, 0LL, 0LL, 0LL); + + DefineCustomBoolVariable( + "gpsc.enable", "Enable metrics collector", 0LL, &guc_enable_collector, Review Comment: I think from the user's perspective, once `gp_stats_collector` is loaded via `shared_preload_libraries`, they want it enabled by default. We exclude noisy users (`gpadmin`, `repl`, `gpperfmon`, `monitor`) to avoid log overflow (not in terms of space, but for the user to easily see what is happening). But it is also possible that the user logged in as some ignored_user, for this reason, I'd clear by default ignored_users. What do I think about this, 2 different approaches: 1. If not built by default → enable via GUCs by default (users loading it expect it active) and clear ignored_users. 2. If built by default → disable all activating GUCs and keep ignored users as right now. What do you think about this? #### Update: Dianjin [explained](https://github.com/apache/cloudberry/pull/1629#discussion_r3008790709) to me that the second option is what we want. To sum it up: I want to build the extension by default, disable all the gucs, and clear the ignored_users by default (let the users pick what to ignore by themselves). -- 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]
