Skip to content

Correct section name for services in fbtrace config #8500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 3, 2025
Merged

Correct section name for services in fbtrace config #8500

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be846ed
Correct section name for services in fbtrace config
Noremos Apr 2, 2025
645c574
Move section type paring to trace reader class
Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/common/config/config_file.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -979,14 +979,14 @@ ConfigFile::SectionType ConfigFile::Parameter::parseSectionKey() const
{
return SectionType::DATABASE_REGEX;
}
else if (name == "service")
Copy link
Member

@AlexPeshkoff AlexPeshkoff Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will break existing trace configs. I strongly disagree.

Copy link
Member

@hvlad hvlad Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When reviewing #8427 I missed that trace-specific routine was put into common class.
It is ConfigFile::Parameter::parseSectionKey() and related enum class SectionType.
I think both must be moved out of ConfigFile::Parameter into TraceCfgReader.

Copy link
Member

@AlexPeshkoff AlexPeshkoff Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both must be moved out of ConfigFile::Parameter into TraceCfgReader.

+1

Copy link
Contributor Author

@Noremos Noremos Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will break existing trace configs. I strongly disagree.

Existing configurations are using the key 'services'. It was my mistake in the original PR where I incorrectly set the key as 'service'."

Copy link
Contributor Author

@Noremos Noremos Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When reviewing #8427 I missed that trace-specific routine was put into common class. It is ConfigFile::Parameter::parseSectionKey() and related enum class SectionType. I think both must be moved out of ConfigFile::Parameter into TraceCfgReader.

I have moved the method

else if (name == "services")
{
return SectionType::SERVICE;
return SectionType::SERVICES;
}
else
{
fatal_exception::raiseFmt("error while parsing trace configuration\n\t"
"line %d: wrong section header, \"database\", \"databaseName\", \"databaseRegex\" or \"service\" is expected",
"line %d: wrong section header, \"database\", \"databaseName\", \"databaseRegex\" or \"services\" is expected",
line);

// Return something to calm down the compiler
Expand Down
2 changes: 1 addition & 1 deletion src/common/config/config_file.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class ConfigFile : public Firebird::AutoStorage, public Firebird::RefCounted
DATABASE,
DATABASE_NAME,
DATABASE_REGEX,
SERVICE
SERVICES
};

struct Parameter : public AutoStorage
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/ntrace/TraceConfiguration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void TraceCfgReader::readConfig()
const ConfigFile::Parameter* section = &params[n];

const ConfigFile::SectionType sectionType = section->parseSectionKey();
const bool isDatabase = (sectionType != ConfigFile::SectionType::SERVICE);
const bool isDatabase = (sectionType != ConfigFile::SectionType::SERVICES);

const ConfigFile::String pattern = section->value;
bool match = false;
Expand Down
Loading