From 5a563162fc66f790b0ca2eba757177ccf52119de Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Tue, 19 Aug 2025 12:46:43 +0300 Subject: [PATCH 1/9] Load trace plugins directly via PluginManager and allow to set requeued plugins for trace session --- doc/README.services_extension | 5 +- doc/README.trace_services | 1 + src/common/IntlParametersBlock.cpp | 1 + src/common/classes/ClumpletReader.cpp | 1 + src/include/firebird/impl/consts_pub.h | 1 + src/include/firebird/impl/msg/fbtracemgr.h | 2 + src/include/gen/Firebird.pas | 1 + src/jrd/svc.cpp | 1 + src/jrd/trace/TraceCmdLine.cpp | 46 ++++++++++++-- src/jrd/trace/TraceConfigStorage.cpp | 32 +++++----- src/jrd/trace/TraceConfigStorage.h | 9 +++ src/jrd/trace/TraceManager.cpp | 74 ++++++---------------- src/jrd/trace/TraceManager.h | 47 ++++---------- src/jrd/trace/TraceService.cpp | 4 +- src/jrd/trace/TraceSession.h | 14 +++- src/jrd/trace/traceswi.h | 2 + src/utilities/fbsvcmgr/fbsvcmgr.cpp | 1 + src/utilities/fbtracemgr/traceMgrMain.cpp | 19 ++++-- 18 files changed, 136 insertions(+), 125 deletions(-) diff --git a/doc/README.services_extension b/doc/README.services_extension index 148b4621972..c849e869dae 100644 --- a/doc/README.services_extension +++ b/doc/README.services_extension @@ -70,8 +70,9 @@ Start user trace session : isc_action_svc_trace_start parameter(s) - isc_spb_trc_name : trace session name, string, optional - isc_spb_trc_cfg : trace session configuration, string, mandatory + isc_spb_trc_name : trace session name, string, optional + isc_spb_trc_cfg : trace session configuration, string, mandatory + isc_spb_trc_plugins : plugins to use for session output text message with status of operation : diff --git a/doc/README.trace_services b/doc/README.trace_services index d526aefc9e5..99b465bd21f 100644 --- a/doc/README.trace_services +++ b/doc/README.trace_services @@ -81,6 +81,7 @@ Action parameters switches : -N[AME] Session name -I[D] Session ID -C[ONFIG] Trace configuration file name + -P[LUGINS] Plugins list to use for trace session Connection parameters switches : -SE[RVICE] Service name diff --git a/src/common/IntlParametersBlock.cpp b/src/common/IntlParametersBlock.cpp index dadd6298a83..67027bde503 100644 --- a/src/common/IntlParametersBlock.cpp +++ b/src/common/IntlParametersBlock.cpp @@ -309,6 +309,7 @@ IntlParametersBlock::TagType IntlSpbStart::checkTag(UCHAR tag, const char** tagN { FB_IPB_TAG(isc_spb_trc_name); FB_IPB_TAG(isc_spb_trc_cfg); + FB_IPB_TAG(isc_spb_trc_plugins); return TAG_STRING; } break; diff --git a/src/common/classes/ClumpletReader.cpp b/src/common/classes/ClumpletReader.cpp index 71629427020..02839e9e6e8 100644 --- a/src/common/classes/ClumpletReader.cpp +++ b/src/common/classes/ClumpletReader.cpp @@ -476,6 +476,7 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const { case isc_spb_trc_cfg: case isc_spb_trc_name: + case isc_spb_trc_plugins: return StringSpb; case isc_spb_trc_id: return IntSpb; diff --git a/src/include/firebird/impl/consts_pub.h b/src/include/firebird/impl/consts_pub.h index c0b28457cce..b10250b0822 100644 --- a/src/include/firebird/impl/consts_pub.h +++ b/src/include/firebird/impl/consts_pub.h @@ -657,6 +657,7 @@ #define isc_spb_trc_id 1 #define isc_spb_trc_name 2 #define isc_spb_trc_cfg 3 +#define isc_spb_trc_plugins 4 /******************************************/ /* Array slice description language (SDL) */ diff --git a/src/include/firebird/impl/msg/fbtracemgr.h b/src/include/firebird/impl/msg/fbtracemgr.h index 17b66666289..53d9006e06e 100644 --- a/src/include/firebird/impl/msg/fbtracemgr.h +++ b/src/include/firebird/impl/msg/fbtracemgr.h @@ -38,3 +38,5 @@ FB_IMPL_MSG(FBTRACEMGR, 37, trace_switch_user_only, -901, "00", "000", "switch \ FB_IMPL_MSG(FBTRACEMGR, 38, trace_switch_param_miss, -901, "00", "000", "mandatory parameter \"@1\" for switch \"@2\" is missing") FB_IMPL_MSG(FBTRACEMGR, 39, trace_param_act_notcompat, -901, "00", "000", "parameter \"@1\" is incompatible with action \"@2\"") FB_IMPL_MSG(FBTRACEMGR, 40, trace_mandatory_switch_miss, -901, "00", "000", "mandatory switch \"@1\" is missing") +FB_IMPL_MSG_NO_SYMBOL(FBTRACEMGR, 41, " -P[LUGINS] Plugins list to use for trace session") +FB_IMPL_MSG_NO_SYMBOL(FBTRACEMGR, 42, " fbtracemgr -SE service_mgr -START -NAME my_trace -CONFIG my_cfg.txt -PLUGINS fbtrace,custom_plugin") diff --git a/src/include/gen/Firebird.pas b/src/include/gen/Firebird.pas index 7e6e69de77f..0bef5dea26a 100644 --- a/src/include/gen/Firebird.pas +++ b/src/include/gen/Firebird.pas @@ -4418,6 +4418,7 @@ IProfilerStatsImpl = class(IProfilerStats) isc_spb_trc_id = byte(1); isc_spb_trc_name = byte(2); isc_spb_trc_cfg = byte(3); + isc_spb_trc_plugins = byte(4); isc_sdl_version1 = byte(1); isc_sdl_eoc = byte(255); isc_sdl_relation = byte(2); diff --git a/src/jrd/svc.cpp b/src/jrd/svc.cpp index 1bde6674e94..b1c12d31007 100644 --- a/src/jrd/svc.cpp +++ b/src/jrd/svc.cpp @@ -3134,6 +3134,7 @@ bool Service::process_switches(ClumpletReader& spb, string& switches) { case isc_spb_trc_cfg: case isc_spb_trc_name: + case isc_spb_trc_plugins: get_action_svc_string(spb, switches); break; case isc_spb_trc_id: diff --git a/src/jrd/trace/TraceCmdLine.cpp b/src/jrd/trace/TraceCmdLine.cpp index 6a2c4cdf107..b5fee6282ee 100644 --- a/src/jrd/trace/TraceCmdLine.cpp +++ b/src/jrd/trace/TraceCmdLine.cpp @@ -63,6 +63,21 @@ namespace printMsg(number, dummy, newLine); } + struct MessageSet + { + const int range[2]; + const std::initializer_list list; + + void print() const + { + for (int i = range[0]; i <= range[1]; ++i) + printMsg(i); + + for (auto id = list.begin(); id != list.end(); ++id) + printMsg(*id); + } + }; + [[noreturn]] void usage(UtilSvc* uSvc, const ISC_STATUS code, const char* msg1 = NULL, const char* msg2 = NULL) { if (uSvc->isService()) @@ -94,16 +109,14 @@ namespace // If the items aren't contiguous, a scheme like in nbackup.cpp will have to be used. // ASF: This is message codes! - constexpr int MAIN_USAGE[] = {3, 21}; - constexpr int EXAMPLES[] = {22, 27}; + const MessageSet MAIN_USAGE{{3, 21}, {41}}; + const MessageSet EXAMPLES{{22, 27}, {42}}; constexpr int NOTES[] = {28, 29}; - for (int i = MAIN_USAGE[0]; i <= MAIN_USAGE[1]; ++i) - printMsg(i); + MAIN_USAGE.print(); printf("\n"); - for (int i = EXAMPLES[0]; i <= EXAMPLES[1]; ++i) - printMsg(i); + EXAMPLES.print(); printf("\n"); for (int i = NOTES[0]; i <= NOTES[1]; ++i) @@ -253,6 +266,27 @@ void fbtrace(UtilSvc* uSvc, TraceSvcIntf* traceSvc) usage(uSvc, isc_trace_param_val_miss, sw->in_sw_name); break; + case IN_SW_TRACE_PLUGINS: + switch (action_sw->in_sw) + { + case IN_SW_TRACE_STOP: + case IN_SW_TRACE_SUSPEND: + case IN_SW_TRACE_RESUME: + case IN_SW_TRACE_LIST: + usage(uSvc, isc_trace_param_act_notcompat, sw->in_sw_name, action_sw->in_sw_name); + break; + } + + if (!session.ses_plugins.empty()) + usage(uSvc, isc_trace_switch_once, sw->in_sw_name); + + itr++; + if (itr < argc && argv[itr]) + session.ses_plugins = argv[itr]; + else + usage(uSvc, isc_trace_param_val_miss, sw->in_sw_name); + break; + default: fb_assert(false); } diff --git a/src/jrd/trace/TraceConfigStorage.cpp b/src/jrd/trace/TraceConfigStorage.cpp index 7020cfc5690..ddad6c64322 100644 --- a/src/jrd/trace/TraceConfigStorage.cpp +++ b/src/jrd/trace/TraceConfigStorage.cpp @@ -676,6 +676,9 @@ ULONG ConfigStorage::getSessionSize(const TraceSession& session) noexcept if ((len = session.ses_logfile.length())) ret += sz + len; + if ((len = session.ses_plugins.length())) + ret += sz + len; + ret += sz + sizeof(session.ses_start); return ret; @@ -717,25 +720,16 @@ void ConfigStorage::addSession(TraceSession& session) char* p = reinterpret_cast (header) + slot->offset; Writer writer(p, slot->size); - if (!session.ses_name.empty()) { - writer.write(tagName, session.ses_name.length(), session.ses_name.c_str()); - } - if (session.ses_auth.hasData()) { + writer.writeStringIfExists(tagName, session.ses_name); + if (session.ses_auth.hasData()) writer.write(tagAuthBlock, session.ses_auth.getCount(), session.ses_auth.begin()); - } - if (!session.ses_user.empty()) { - writer.write(tagUserName, session.ses_user.length(), session.ses_user.c_str()); - } - if (session.ses_role.hasData()) { - writer.write(tagRole, session.ses_role.length(), session.ses_role.c_str()); - } - if (!session.ses_config.empty()) { - writer.write(tagConfig, session.ses_config.length(), session.ses_config.c_str()); - } + writer.writeStringIfExists(tagUserName, session.ses_user); + writer.writeStringIfExists(tagRole, session.ses_role); + writer.writeStringIfExists(tagConfig, session.ses_config); writer.write(tagStartTS, sizeof(session.ses_start), &session.ses_start); - if (!session.ses_logfile.empty()) { - writer.write(tagLogFile, session.ses_logfile.length(), session.ses_logfile.c_str()); - } + writer.writeStringIfExists(tagLogFile, session.ses_logfile); + writer.writeStringIfExists(tagPlugins, session.ses_plugins); + writer.write(tagEnd, 0, NULL); } @@ -841,6 +835,10 @@ bool ConfigStorage::readSession(const TraceCSHeader::Slot* slot, TraceSession& s p = session.ses_role.getBuffer(len); break; + case tagPlugins: + p = session.ses_plugins.getBuffer(len); + break; + default: fb_assert(false); return false; diff --git a/src/jrd/trace/TraceConfigStorage.h b/src/jrd/trace/TraceConfigStorage.h index 4c03a552ddf..2e1d5e208ac 100644 --- a/src/jrd/trace/TraceConfigStorage.h +++ b/src/jrd/trace/TraceConfigStorage.h @@ -193,6 +193,7 @@ class ConfigStorage final : public Firebird::GlobalStorage, public Firebird::Ipc tagStartTS, // date+time when started tagLogFile, // log file name, if any tagRole, // SQL role name, if any + tagPlugins, // trace plugins list, if any tagEnd }; @@ -240,6 +241,14 @@ class ConfigStorage final : public Firebird::GlobalStorage, public Firebird::Ipc void write(ITEM tag, ULONG len, const void* data); + inline void writeStringIfExists(const ITEM tag, const Firebird::AbstractString& data) + { + if (data.empty()) + return; + + write(tag, data.length(), data.c_str()); + } + private: char* m_mem; const char* const m_end; diff --git a/src/jrd/trace/TraceManager.cpp b/src/jrd/trace/TraceManager.cpp index 84edc6c4047..7335443b93f 100644 --- a/src/jrd/trace/TraceManager.cpp +++ b/src/jrd/trace/TraceManager.cpp @@ -51,10 +51,6 @@ namespace namespace Jrd { GlobalPtr TraceManager::storageInstance; -TraceManager::Factories* TraceManager::factories = NULL; -GlobalPtr TraceManager::init_factories_lock; -volatile bool TraceManager::init_factories; - bool TraceManager::check_result(ITracePlugin* plugin, const char* module, const char* function, bool result) @@ -126,51 +122,14 @@ void TraceManager::init() { // ensure storage is initialized getStorage(); - load_plugins(); - changeNumber = 0; -} -void TraceManager::load_plugins() -{ // Initialize all trace needs to false trace_needs = 0; - - if (init_factories) - return; - - WriteLockGuard guard(init_factories_lock, FB_FUNCTION); - if (init_factories) - return; - - factories = FB_NEW_POOL(*getDefaultMemoryPool()) TraceManager::Factories(*getDefaultMemoryPool()); - for (GetPlugins traceItr(IPluginManager::TYPE_TRACE); traceItr.hasData(); traceItr.next()) - { - FactoryInfo info; - info.factory = traceItr.plugin(); - info.factory->addRef(); - string name(traceItr.name()); - name.copyTo(info.name, sizeof(info.name)); - factories->add(info); - } - - init_factories = true; + changeNumber = 0; } - void TraceManager::shutdown() { - if (init_factories) - { - WriteLockGuard guard(init_factories_lock, FB_FUNCTION); - - if (init_factories) - { - init_factories = false; - delete factories; - factories = NULL; - } - } - getStorage()->shutdown(); } @@ -224,7 +183,7 @@ void TraceManager::update_sessions() } else { - trace_sessions[i].plugin->release(); + trace_sessions[i].release(); trace_sessions.remove(i); } } @@ -366,31 +325,36 @@ void TraceManager::update_session(const TraceSession& session) } } - ReadLockGuard guard(init_factories_lock, FB_FUNCTION); - if (!factories) - return; + TraceInitInfoImpl attachInfo(session, attachment, filename); - for (FactoryInfo* info = factories->begin(); info != factories->end(); ++info) + GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsString()); + for (; traceItr.hasData(); traceItr.next()) { - TraceInitInfoImpl attachInfo(session, attachment, filename); FbLocalStatus status; - ITracePlugin* plugin = info->factory->trace_create(&status, &attachInfo); + ITraceFactory* factory = traceItr.plugin(); + ITracePlugin* plugin = factory->trace_create(&status, &attachInfo); if (plugin) { - plugin->addRef(); SessionInfo sesInfo; sesInfo.plugin = plugin; - sesInfo.factory_info = info; + sesInfo.plugin->addRef(); + + sesInfo.factory = factory; + sesInfo.factory->addRef(); + + string name(traceItr.name()); + name.copyTo(sesInfo.pluginName, sizeof(sesInfo.pluginName)); + sesInfo.ses_id = session.ses_id; trace_sessions.add(sesInfo); - new_needs |= info->factory->trace_needs(); + new_needs |= sesInfo.factory->trace_needs(); } else if (status->getState() & IStatus::STATE_ERRORS) { string header; - header.printf("Trace plugin %s returned error on call trace_create.", info->name); + header.printf("Trace plugin %s returned error on call trace_create.", traceItr.name()); iscLogStatus(header.c_str(), &status); } } @@ -455,13 +419,13 @@ void TraceManager::event_dsql_restart(Attachment* att, jrd_tra* transaction, while (i < trace_sessions.getCount()) \ { \ SessionInfo* plug_info = &trace_sessions[i]; \ - if (check_result(plug_info->plugin, plug_info->factory_info->name, #METHOD, \ + if (check_result(plug_info->plugin, plug_info->pluginName, #METHOD, \ plug_info->plugin->METHOD PARAMS)) \ { \ i++; /* Move to next plugin */ \ } \ else { \ - plug_info->plugin->release(); \ + plug_info->release(); \ trace_sessions.remove(i); /* Remove broken plugin from the list */ \ } \ } diff --git a/src/jrd/trace/TraceManager.h b/src/jrd/trace/TraceManager.h index b0d389719e8..6a368f456f9 100644 --- a/src/jrd/trace/TraceManager.h +++ b/src/jrd/trace/TraceManager.h @@ -67,9 +67,6 @@ class TraceManager static ConfigStorage* getStorage() { return storageInstance->getStorage(); } - static size_t pluginsCount() - { return factories->getCount(); } - void event_attach(Firebird::ITraceDatabaseConnection* connection, bool create_db, ntrace_result_t att_result); @@ -137,7 +134,7 @@ class TraceManager inline bool needs(unsigned e) { - if (!active || !init_factories) + if (!active) return false; if (changeNumber != getStorage()->getChangeNumber()) @@ -192,43 +189,22 @@ class TraceManager NotificationNeeds trace_needs, new_needs; // This structure should be POD-like to be stored in Array - struct FactoryInfo + + struct SessionInfo { - FactoryInfo() : factory(NULL) - { - memset(name, 0, sizeof(name)); - } + char pluginName[MAXPATHLEN] = {}; Firebird::ITraceFactory* factory; - char name[MAXPATHLEN]; - }; - - class Factories : public Firebird::Array - { - public: - explicit Factories(Firebird::MemoryPool& p) - : Firebird::Array(p) - { } + Firebird::ITracePlugin* plugin; + ULONG ses_id; - ~Factories() + inline void release() { - Firebird::PluginManagerInterfacePtr pi; - - for (unsigned int i = 0; i < getCount(); ++i) - pi->releasePlugin(getElement(i).factory); + factory->release(); + plugin->release(); } - }; - - static Factories* factories; - static Firebird::GlobalPtr init_factories_lock; - static volatile bool init_factories; - - struct SessionInfo - { - FactoryInfo* factory_info; - Firebird::ITracePlugin* plugin; - ULONG ses_id; + // Used for SortedArray::find static ULONG generate(const SessionInfo& item) { return item.ses_id; } }; @@ -243,14 +219,13 @@ class TraceManager { for (unsigned int i = 0; i < getCount(); ++i) { - getElement(i).plugin->release(); + getElement(i).release(); } } }; Sessions trace_sessions; void init(); - void load_plugins(); void update_sessions(); void update_session(const Firebird::TraceSession& session); diff --git a/src/jrd/trace/TraceService.cpp b/src/jrd/trace/TraceService.cpp index ea0ece84834..6f9dd8387bb 100644 --- a/src/jrd/trace/TraceService.cpp +++ b/src/jrd/trace/TraceService.cpp @@ -34,6 +34,7 @@ #include "../../common/StatusArg.h" #include "../../common/ThreadStart.h" #include "../../common/db_alias.h" +#include "../../common/classes/GetPlugins.h" #include "../../jrd/svc.h" #include "../../common/os/guid.h" #include "../../jrd/trace/TraceLog.h" @@ -103,7 +104,8 @@ void TraceSvcJrd::setAttachInfo(const string& /*svc_name*/, const string& user, void TraceSvcJrd::startSession(TraceSession& session, bool interactive) { - if (!TraceManager::pluginsCount()) + GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsString()); + if (!traceItr.hasData()) { m_svc.printf(false, "Can not start trace session. There are no trace plugins loaded\n"); return; diff --git a/src/jrd/trace/TraceSession.h b/src/jrd/trace/TraceSession.h index b3d74e7ba1d..5c0a2cef150 100644 --- a/src/jrd/trace/TraceSession.h +++ b/src/jrd/trace/TraceSession.h @@ -53,7 +53,8 @@ class TraceSession ses_start(0), ses_flags(0), ses_logfile(pool), - ses_role(pool) + ses_role(pool), + ses_plugins(pool) {} TraceSession(MemoryPool& pool, TraceSession& other) : @@ -65,7 +66,8 @@ class TraceSession ses_start(other.ses_start), ses_flags(other.ses_flags), ses_logfile(pool, other.ses_logfile), - ses_role(pool, other.ses_role) + ses_role(pool, other.ses_role), + ses_plugins(pool, other.ses_plugins) {} ~TraceSession() {} @@ -81,6 +83,13 @@ class TraceSession ses_flags = 0; ses_logfile = ""; ses_role = ""; + ses_plugins = ""; + } + + // Return string or nullptr + inline const char* getPluginsString() const noexcept + { + return ses_plugins.hasData() ? ses_plugins.data() : nullptr; } ULONG ses_id; @@ -92,6 +101,7 @@ class TraceSession int ses_flags; PathName ses_logfile; string ses_role; + string ses_plugins; }; } // namespace Firebird diff --git a/src/jrd/trace/traceswi.h b/src/jrd/trace/traceswi.h index 900cfb0b9b6..194c9184060 100644 --- a/src/jrd/trace/traceswi.h +++ b/src/jrd/trace/traceswi.h @@ -41,6 +41,7 @@ inline constexpr int IN_SW_TRACE_FETCH_PWD = 12; inline constexpr int IN_SW_TRACE_TRUSTED_AUTH = 13; inline constexpr int IN_SW_TRACE_VERSION = 14; inline constexpr int IN_SW_TRACE_ROLE = 15; +inline constexpr int IN_SW_TRACE_PLUGINS = 16; // list of possible actions (services) for use with trace services @@ -62,6 +63,7 @@ static inline constexpr struct Switches::in_sw_tab_t trace_option_in_sw_table [] {IN_SW_TRACE_CONFIG, isc_spb_trc_cfg, "CONFIG", 0, 0, 0, false, false, 0, 1, NULL}, {IN_SW_TRACE_ID, isc_spb_trc_id, "ID", 0, 0, 0, false, false, 0, 1, NULL}, {IN_SW_TRACE_NAME, isc_spb_trc_name, "NAME", 0, 0, 0, false, false, 0, 1, NULL}, + {IN_SW_TRACE_PLUGINS, isc_spb_trc_plugins,"PLUGINS", 0, 0, 0, false, false, 0, 1, NULL}, {0, 0, NULL, 0, 0, 0, false, false, 0, 0, NULL} // End of List }; diff --git a/src/utilities/fbsvcmgr/fbsvcmgr.cpp b/src/utilities/fbsvcmgr/fbsvcmgr.cpp index faab0f2d708..ea014964f3f 100644 --- a/src/utilities/fbsvcmgr/fbsvcmgr.cpp +++ b/src/utilities/fbsvcmgr/fbsvcmgr.cpp @@ -591,6 +591,7 @@ constexpr SvcSwitches traceStartOptions[] = { {"trc_cfg", putFileFromArgument, 0, isc_spb_trc_cfg, 0}, {"trc_name", putStringArgument, 0, isc_spb_trc_name, 0}, + {"trc_plugin", putStringArgument, 0, isc_spb_trc_plugins, 0}, {0, 0, 0, 0, 0} }; diff --git a/src/utilities/fbtracemgr/traceMgrMain.cpp b/src/utilities/fbtracemgr/traceMgrMain.cpp index 9902097b890..ffe296a4639 100644 --- a/src/utilities/fbtracemgr/traceMgrMain.cpp +++ b/src/utilities/fbtracemgr/traceMgrMain.cpp @@ -109,6 +109,17 @@ void TraceSvcUtil::setAttachInfo(const string& service_name, const string& user, } } +template +static void insertStringIfExists(ClumpletWriter& spb, const string& str) +{ + if (str.isEmpty()) + return; + + spb.insertBytes(Tag, + reinterpret_cast (str.c_str()), + str.length()); +} + void TraceSvcUtil::startSession(TraceSession& session, bool /*interactive*/) { HalfStaticArray buff(*getDefaultMemoryPool()); @@ -158,12 +169,8 @@ void TraceSvcUtil::startSession(TraceSession& session, bool /*interactive*/) spb.insertTag(isc_action_svc_trace_start); spb.insertBytes(isc_spb_trc_cfg, p, len); - if (session.ses_name.hasData()) - { - spb.insertBytes(isc_spb_trc_name, - reinterpret_cast (session.ses_name.c_str()), - session.ses_name.length()); - } + insertStringIfExists(spb, session.ses_name); + insertStringIfExists(spb, session.ses_plugins); runService(spb.getBufferLength(), spb.getBuffer()); } From 0fc2b35a799fd2ccca649567fcb6a3003a1232bc Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Tue, 19 Aug 2025 14:14:55 +0300 Subject: [PATCH 2/9] Better for declaration split --- src/jrd/trace/TraceManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jrd/trace/TraceManager.cpp b/src/jrd/trace/TraceManager.cpp index 7335443b93f..ba944b22039 100644 --- a/src/jrd/trace/TraceManager.cpp +++ b/src/jrd/trace/TraceManager.cpp @@ -327,8 +327,8 @@ void TraceManager::update_session(const TraceSession& session) TraceInitInfoImpl attachInfo(session, attachment, filename); - GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsString()); - for (; traceItr.hasData(); traceItr.next()) + for (GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsString()); + traceItr.hasData(); traceItr.next()) { FbLocalStatus status; ITraceFactory* factory = traceItr.plugin(); From 624a23f535c066523c36633111e6c318bce7bd38 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 20 Aug 2025 10:15:21 +0300 Subject: [PATCH 3/9] Better names --- src/jrd/trace/TraceConfigStorage.cpp | 12 ++++++------ src/jrd/trace/TraceConfigStorage.h | 2 +- src/jrd/trace/TraceManager.cpp | 2 +- src/jrd/trace/TraceService.cpp | 2 +- src/jrd/trace/TraceSession.h | 2 +- src/utilities/fbsvcmgr/fbsvcmgr.cpp | 2 +- src/utilities/fbtracemgr/traceMgrMain.cpp | 9 ++++----- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/jrd/trace/TraceConfigStorage.cpp b/src/jrd/trace/TraceConfigStorage.cpp index ddad6c64322..67ea28fee70 100644 --- a/src/jrd/trace/TraceConfigStorage.cpp +++ b/src/jrd/trace/TraceConfigStorage.cpp @@ -720,15 +720,15 @@ void ConfigStorage::addSession(TraceSession& session) char* p = reinterpret_cast (header) + slot->offset; Writer writer(p, slot->size); - writer.writeStringIfExists(tagName, session.ses_name); + writer.writeData(tagName, session.ses_name); if (session.ses_auth.hasData()) writer.write(tagAuthBlock, session.ses_auth.getCount(), session.ses_auth.begin()); - writer.writeStringIfExists(tagUserName, session.ses_user); - writer.writeStringIfExists(tagRole, session.ses_role); - writer.writeStringIfExists(tagConfig, session.ses_config); + writer.writeData(tagUserName, session.ses_user); + writer.writeData(tagRole, session.ses_role); + writer.writeData(tagConfig, session.ses_config); writer.write(tagStartTS, sizeof(session.ses_start), &session.ses_start); - writer.writeStringIfExists(tagLogFile, session.ses_logfile); - writer.writeStringIfExists(tagPlugins, session.ses_plugins); + writer.writeData(tagLogFile, session.ses_logfile); + writer.writeData(tagPlugins, session.ses_plugins); writer.write(tagEnd, 0, NULL); } diff --git a/src/jrd/trace/TraceConfigStorage.h b/src/jrd/trace/TraceConfigStorage.h index 2e1d5e208ac..2b569389754 100644 --- a/src/jrd/trace/TraceConfigStorage.h +++ b/src/jrd/trace/TraceConfigStorage.h @@ -241,7 +241,7 @@ class ConfigStorage final : public Firebird::GlobalStorage, public Firebird::Ipc void write(ITEM tag, ULONG len, const void* data); - inline void writeStringIfExists(const ITEM tag, const Firebird::AbstractString& data) + inline void writeData(const ITEM tag, const Firebird::AbstractString& data) { if (data.empty()) return; diff --git a/src/jrd/trace/TraceManager.cpp b/src/jrd/trace/TraceManager.cpp index ba944b22039..c1ee9e8bd10 100644 --- a/src/jrd/trace/TraceManager.cpp +++ b/src/jrd/trace/TraceManager.cpp @@ -327,7 +327,7 @@ void TraceManager::update_session(const TraceSession& session) TraceInitInfoImpl attachInfo(session, attachment, filename); - for (GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsString()); + for (GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsList()); traceItr.hasData(); traceItr.next()) { FbLocalStatus status; diff --git a/src/jrd/trace/TraceService.cpp b/src/jrd/trace/TraceService.cpp index 6f9dd8387bb..f9bd41afe25 100644 --- a/src/jrd/trace/TraceService.cpp +++ b/src/jrd/trace/TraceService.cpp @@ -104,7 +104,7 @@ void TraceSvcJrd::setAttachInfo(const string& /*svc_name*/, const string& user, void TraceSvcJrd::startSession(TraceSession& session, bool interactive) { - GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsString()); + GetPlugins traceItr(IPluginManager::TYPE_TRACE, session.getPluginsList()); if (!traceItr.hasData()) { m_svc.printf(false, "Can not start trace session. There are no trace plugins loaded\n"); diff --git a/src/jrd/trace/TraceSession.h b/src/jrd/trace/TraceSession.h index 5c0a2cef150..8a168b6327f 100644 --- a/src/jrd/trace/TraceSession.h +++ b/src/jrd/trace/TraceSession.h @@ -87,7 +87,7 @@ class TraceSession } // Return string or nullptr - inline const char* getPluginsString() const noexcept + inline const char* getPluginsList() const noexcept { return ses_plugins.hasData() ? ses_plugins.data() : nullptr; } diff --git a/src/utilities/fbsvcmgr/fbsvcmgr.cpp b/src/utilities/fbsvcmgr/fbsvcmgr.cpp index ea014964f3f..00d17f99106 100644 --- a/src/utilities/fbsvcmgr/fbsvcmgr.cpp +++ b/src/utilities/fbsvcmgr/fbsvcmgr.cpp @@ -591,7 +591,7 @@ constexpr SvcSwitches traceStartOptions[] = { {"trc_cfg", putFileFromArgument, 0, isc_spb_trc_cfg, 0}, {"trc_name", putStringArgument, 0, isc_spb_trc_name, 0}, - {"trc_plugin", putStringArgument, 0, isc_spb_trc_plugins, 0}, + {"trc_plugins", putStringArgument, 0, isc_spb_trc_plugins, 0}, {0, 0, 0, 0, 0} }; diff --git a/src/utilities/fbtracemgr/traceMgrMain.cpp b/src/utilities/fbtracemgr/traceMgrMain.cpp index ffe296a4639..48ff9893557 100644 --- a/src/utilities/fbtracemgr/traceMgrMain.cpp +++ b/src/utilities/fbtracemgr/traceMgrMain.cpp @@ -109,13 +109,12 @@ void TraceSvcUtil::setAttachInfo(const string& service_name, const string& user, } } -template -static void insertStringIfExists(ClumpletWriter& spb, const string& str) +static void insertString(UCHAR tag, ClumpletWriter& spb, const string& str) { if (str.isEmpty()) return; - spb.insertBytes(Tag, + spb.insertBytes(tag, reinterpret_cast (str.c_str()), str.length()); } @@ -169,8 +168,8 @@ void TraceSvcUtil::startSession(TraceSession& session, bool /*interactive*/) spb.insertTag(isc_action_svc_trace_start); spb.insertBytes(isc_spb_trc_cfg, p, len); - insertStringIfExists(spb, session.ses_name); - insertStringIfExists(spb, session.ses_plugins); + insertString(isc_spb_trc_name, spb, session.ses_name); + insertString(isc_spb_trc_plugins, spb, session.ses_plugins); runService(spb.getBufferLength(), spb.getBuffer()); } From f85ab73395ee04b9dc6a058c788ebbcb194c5712 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 20 Aug 2025 10:23:24 +0300 Subject: [PATCH 4/9] Release trace plugin factory via IPluginManager --- src/jrd/trace/TraceManager.h | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/jrd/trace/TraceManager.h b/src/jrd/trace/TraceManager.h index 6a368f456f9..91c3b6b2dea 100644 --- a/src/jrd/trace/TraceManager.h +++ b/src/jrd/trace/TraceManager.h @@ -194,14 +194,20 @@ class TraceManager { char pluginName[MAXPATHLEN] = {}; - Firebird::ITraceFactory* factory; Firebird::ITracePlugin* plugin; + Firebird::ITraceFactory* factory; ULONG ses_id; - inline void release() + inline void release(Firebird::PluginManagerInterfacePtr& pi) { - factory->release(); plugin->release(); + pi->releasePlugin(factory); + } + + inline void release() + { + Firebird::PluginManagerInterfacePtr pi; + release(pi); } // Used for SortedArray::find @@ -217,9 +223,11 @@ class TraceManager ~Sessions() { + Firebird::PluginManagerInterfacePtr pi; + for (unsigned int i = 0; i < getCount(); ++i) { - getElement(i).release(); + getElement(i).release(pi); } } }; From f9ac3d34117ee7006be9cc0baca79b3aaf075111 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 20 Aug 2025 13:23:36 +0300 Subject: [PATCH 5/9] Print trace plugins in tracecmgr LIST output --- doc/README.services_extension | 9 +++++---- src/jrd/trace/TraceService.cpp | 15 +++++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/doc/README.services_extension b/doc/README.services_extension index c849e869dae..93d63013bb6 100644 --- a/doc/README.services_extension +++ b/doc/README.services_extension @@ -131,10 +131,11 @@ List of existing trace sessions output text messages with list and state of trace sessions - Session ID: - - name: - - user: - - date: YYYY-MM-DD HH:NN:SS - - flags: + - name: + - user: + - date: YYYY-MM-DD HH:NN:SS + - flags: + - plugins: "name" is trace session name and not printed if empty. "user" is creator user name diff --git a/src/jrd/trace/TraceService.cpp b/src/jrd/trace/TraceService.cpp index f9bd41afe25..6762f266394 100644 --- a/src/jrd/trace/TraceService.cpp +++ b/src/jrd/trace/TraceService.cpp @@ -219,6 +219,10 @@ bool TraceSvcJrd::changeFlags(ULONG id, int setFlags, int clearFlags) void TraceSvcJrd::listSessions() { + // Do not use value from Config::getDefaultConfig() because + // tracemgr will reread config in embedded mode + constexpr const char* defaultPlugins = ""; + m_svc.started(); // Writing into service when storage is locked could lead to deadlock, @@ -235,12 +239,12 @@ void TraceSvcJrd::listSessions() { m_svc.printf(false, "\nSession ID: %d\n", session.ses_id); if (!session.ses_name.empty()) { - m_svc.printf(false, " name: %s\n", session.ses_name.c_str()); + m_svc.printf(false, " name: %s\n", session.ses_name.c_str()); } - m_svc.printf(false, " user: %s\n", session.ses_user.c_str()); + m_svc.printf(false, " user: %s\n", session.ses_user.c_str()); const struct tm* t = localtime(&session.ses_start); - m_svc.printf(false, " date: %04d-%02d-%02d %02d:%02d:%02d\n", + m_svc.printf(false, " date: %04d-%02d-%02d %02d:%02d:%02d\n", t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, t->tm_hour, t->tm_min, t->tm_sec); @@ -266,7 +270,10 @@ void TraceSvcJrd::listSessions() if (session.ses_flags & trs_log_full) { flags += ", log full"; } - m_svc.printf(false, " flags: %s\n", flags.c_str()); + m_svc.printf(false, " flags: %s\n", flags.c_str()); + + const char* pluginsList = session.getPluginsList(); + m_svc.printf(false, " plugins: %s\n", pluginsList ? pluginsList : defaultPlugins); } } } From 5a7ffdfdec6224216de5413cb2b3816a2832bdb5 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 20 Aug 2025 14:14:50 +0300 Subject: [PATCH 6/9] Add 'plugins' description --- doc/README.services_extension | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/README.services_extension b/doc/README.services_extension index 93d63013bb6..740d60f0985 100644 --- a/doc/README.services_extension +++ b/doc/README.services_extension @@ -146,7 +146,9 @@ List of existing trace sessions if the session was created by the engine itself : "system" kind of session : "audit" or "trace" if user session log file is full : "log full" - + "plugins" is a set of plugins used to create trace session + Plugins can be specified via API/tracemgr. + By default, plugins form the firebird.conf (TracePlugin) are used Output of every service is obtained as usually using isc_service_query call with isc_info_svc_line or isc_info_svc_to_eof information items. From 0cc779a0b08acc717faa1c9bb119f0650ea811bc Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 20 Aug 2025 14:31:19 +0300 Subject: [PATCH 7/9] Correct english in README/description --- doc/README.services_extension | 2 +- doc/README.trace_services | 2 +- src/include/firebird/impl/msg/fbtracemgr.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/README.services_extension b/doc/README.services_extension index 740d60f0985..d0cec4c191a 100644 --- a/doc/README.services_extension +++ b/doc/README.services_extension @@ -72,7 +72,7 @@ Start user trace session : parameter(s) isc_spb_trc_name : trace session name, string, optional isc_spb_trc_cfg : trace session configuration, string, mandatory - isc_spb_trc_plugins : plugins to use for session + isc_spb_trc_plugins : plugins for use with trace session; valid separators: '\t', ',', ';' output text message with status of operation : diff --git a/doc/README.trace_services b/doc/README.trace_services index 99b465bd21f..a21aec216fc 100644 --- a/doc/README.trace_services +++ b/doc/README.trace_services @@ -81,7 +81,7 @@ Action parameters switches : -N[AME] Session name -I[D] Session ID -C[ONFIG] Trace configuration file name - -P[LUGINS] Plugins list to use for trace session + -P[LUGINS] Plugins list for use with trace session; valid list separators: '\t', ',', ';' Connection parameters switches : -SE[RVICE] Service name diff --git a/src/include/firebird/impl/msg/fbtracemgr.h b/src/include/firebird/impl/msg/fbtracemgr.h index 53d9006e06e..a2c90d4c4e3 100644 --- a/src/include/firebird/impl/msg/fbtracemgr.h +++ b/src/include/firebird/impl/msg/fbtracemgr.h @@ -38,5 +38,5 @@ FB_IMPL_MSG(FBTRACEMGR, 37, trace_switch_user_only, -901, "00", "000", "switch \ FB_IMPL_MSG(FBTRACEMGR, 38, trace_switch_param_miss, -901, "00", "000", "mandatory parameter \"@1\" for switch \"@2\" is missing") FB_IMPL_MSG(FBTRACEMGR, 39, trace_param_act_notcompat, -901, "00", "000", "parameter \"@1\" is incompatible with action \"@2\"") FB_IMPL_MSG(FBTRACEMGR, 40, trace_mandatory_switch_miss, -901, "00", "000", "mandatory switch \"@1\" is missing") -FB_IMPL_MSG_NO_SYMBOL(FBTRACEMGR, 41, " -P[LUGINS] Plugins list to use for trace session") +FB_IMPL_MSG_NO_SYMBOL(FBTRACEMGR, 41, " -P[LUGINS] Plugins list for use with trace session; valid list separators: , , ") FB_IMPL_MSG_NO_SYMBOL(FBTRACEMGR, 42, " fbtracemgr -SE service_mgr -START -NAME my_trace -CONFIG my_cfg.txt -PLUGINS fbtrace,custom_plugin") From 40d7ea6cd05bed414a53e08a4c5ecca583726ea2 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 20 Aug 2025 14:33:10 +0300 Subject: [PATCH 8/9] Add type description for isc_spb_trc_plugins --- doc/README.services_extension | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/README.services_extension b/doc/README.services_extension index d0cec4c191a..7596a53e43b 100644 --- a/doc/README.services_extension +++ b/doc/README.services_extension @@ -72,7 +72,7 @@ Start user trace session : parameter(s) isc_spb_trc_name : trace session name, string, optional isc_spb_trc_cfg : trace session configuration, string, mandatory - isc_spb_trc_plugins : plugins for use with trace session; valid separators: '\t', ',', ';' + isc_spb_trc_plugins : plugins for use with trace session, list of plugin names (valid separators: '\t', ',', ';'), optional output text message with status of operation : From 3e1a02599d502f3c5931c58ff4ea1478cb58c784 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 20 Aug 2025 14:34:51 +0300 Subject: [PATCH 9/9] Better plugins description --- doc/README.services_extension | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/README.services_extension b/doc/README.services_extension index 7596a53e43b..866d5646e5a 100644 --- a/doc/README.services_extension +++ b/doc/README.services_extension @@ -146,7 +146,7 @@ List of existing trace sessions if the session was created by the engine itself : "system" kind of session : "audit" or "trace" if user session log file is full : "log full" - "plugins" is a set of plugins used to create trace session + "plugins" is a set of plugins for use with trace session Plugins can be specified via API/tracemgr. By default, plugins form the firebird.conf (TracePlugin) are used