Skip to content
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

feat(logging)_: enable runtime logs configuration #6210

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+104 −36
Open

feat(logging)_: enable runtime logs configuration #6210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 26 additions & 6 deletions api/geth_backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2079,12 +2079,6 @@ func (b *GethStatusBackend) loadNodeConfig(inputNodeCfg *params.NodeConfig) erro
}
}

if len(conf.LogDir) == 0 {
conf.LogFile = filepath.Join(b.rootDataDir, conf.LogFile)
} else {
conf.LogFile = filepath.Join(conf.LogDir, conf.LogFile)
}

b.config = conf

if inputNodeCfg != nil && inputNodeCfg.RuntimeLogLevel != "" {
Expand Down Expand Up @@ -2859,3 +2853,29 @@ func (b *GethStatusBackend) TogglePanicReporting(enabled bool) error {
}
return b.DisablePanicReporting()
}

func (b *GethStatusBackend) SetLogLevel(level string) error {
b.mu.Lock()
defer b.mu.Unlock()

err := nodecfg.SetLogLevel(b.appDB, level)
if err != nil {
return err
}
b.config.LogLevel = level

return logutils.OverrideRootLoggerWithConfig(b.config.LogSettings())
}

func (b *GethStatusBackend) SetLogNamespaces(namespaces string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

@osmaczko is there a reason to not expose a data structure based API to clients instead of the string based approach like in test1.test2:debug,test1.test2.test3:info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great question. I don't believe having a data structure-based API in this case is feasible, as there are numerous namespaces, sub-namespaces, sub-sub-namespaces, and so on. Moreover, there is no single entity that can know all the registered namespaces. I attempted to address this but eventually stopped, as it would have required significant effort. I want developers (this utility is intended for developers) to have the freedom to configure namespaces however they see fit. I can imagine a developer adjusting namespace settings ad hoc to suit their needs for a specific task or during bug investigations.

As for the Waku debug toggle, I understand it is not ideal, as both clients would need to know the exact namespace. Maybe for that case, SetWakuLogLevel could be introduced as a helper, so the knowledge of the namespace is hidden in status-go?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay @osmaczko, you considered this option already 👍🏼

In any case, I was imagining something simple and easy to serialize, example payload:

{
  "logNamespaces": [
    {"name": "test1.test2",       "level": "debug"},
    {"name": "test1.test2.test3", "level": "info"}
  ]
}

Maybe the convenience of using a string vs a data structure is related to the programming language in question? The string approach requires more knowledge about how namespaces are used internally. For example, the dev should know the namespaces are splittable by a comma and that the log level should be separated by a colon. The log level type is a bit more opaque and the dev may need to manually find the regex that validates it deeper down in logutils.

Nothing too important, thanks for considering it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @ilmotta, that's a nice proposition.

Maybe the convenience of using a string vs a data structure is related to the programming language in question?

Not at all. The whole idea of namespaces was inspired by: go-log. The idea behind using a string is to potentially enable configuration through environment variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea behind using a string is to potentially enable configuration through environment variable.

👍🏼 If namespaces are meant to be also configurable by env vars then that is indeed a good argument in favor of using a string as input.

b.mu.Lock()
defer b.mu.Unlock()

err := nodecfg.SetLogNamespaces(b.appDB, namespaces)
if err != nil {
return err
}
b.config.LogNamespaces = namespaces

return logutils.OverrideRootLoggerWithConfig(b.config.LogSettings())
}
3 changes: 2 additions & 1 deletion logutils/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ func (core *Core) Sync() error {
}

func (core *Core) UpdateSyncer(newSyncer zapcore.WriteSyncer) {
core.syncer.Store(writeSyncerWrapper{WriteSyncer: newSyncer})
oldSyncer := core.syncer.Swap(writeSyncerWrapper{WriteSyncer: newSyncer})
_ = oldSyncer.(zapcore.WriteSyncer).Sync() // may fail but doesn't impact syncer update
}

func (core *Core) UpdateEncoder(newEncoder zapcore.Encoder) {
Expand Down
40 changes: 39 additions & 1 deletion mobile/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ func exportNodeLogs() string {
if config == nil {
return makeJSONResponse(errors.New("config and log file are not available"))
}
data, err := json.Marshal(exportlogs.ExportFromBaseFile(config.LogFile))
data, err := json.Marshal(exportlogs.ExportFromBaseFile(config.LogFilePath()))
if err != nil {
return makeJSONResponse(fmt.Errorf("error marshalling to json: %v", err))
}
Expand Down Expand Up @@ -2306,6 +2306,44 @@ func addCentralizedMetric(requestJSON string) string {
return metric.ID
}

func SetLogLevel(requestJSON string) string {
return callWithResponse(setLogLevel, requestJSON)
}

func setLogLevel(requestJSON string) string {
var request requests.SetLogLevel
err := json.Unmarshal([]byte(requestJSON), &request)
if err != nil {
return makeJSONResponse(err)
}

err = request.Validate()
if err != nil {
return makeJSONResponse(err)
}

return makeJSONResponse(statusBackend.SetLogLevel(request.LogLevel))
}

func SetLogNamespaces(requestJSON string) string {
return callWithResponse(setLogNamespaces, requestJSON)
}

func setLogNamespaces(requestJSON string) string {
var request requests.SetLogNamespaces
err := json.Unmarshal([]byte(requestJSON), &request)
if err != nil {
return makeJSONResponse(err)
}

err = request.Validate()
if err != nil {
return makeJSONResponse(err)
}

return makeJSONResponse(statusBackend.SetLogNamespaces(request.LogNamespaces))
}

func IntendedPanic(message string) string {
type intendedPanic struct {
error
Expand Down
9 changes: 8 additions & 1 deletion params/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1196,12 +1196,19 @@ func LesTopic(netid int) string {
}
}

func (c *NodeConfig) LogFilePath() string {
if c.LogDir == "" {
return filepath.Join(c.RootDataDir, c.LogFile)
}
return filepath.Join(c.LogDir, c.LogFile)
}

func (c *NodeConfig) LogSettings() logutils.LogSettings {
return logutils.LogSettings{
Enabled: c.LogEnabled,
Level: c.LogLevel,
Namespaces: c.LogNamespaces,
File: c.LogFile,
File: c.LogFilePath(),
MaxSize: c.LogMaxSize,
MaxBackups: c.LogMaxBackups,
CompressRotated: c.LogCompressRotated,
Expand Down
2 changes: 2 additions & 0 deletions protocol/messenger_settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func (m *Messenger) SetSyncingOnMobileNetwork(request *requests.SetSyncingOnMobi
return nil
}

// Deprecated: Use SetLogLevel from status.go instead.
func (m *Messenger) SetLogLevel(request *requests.SetLogLevel) error {
if err := request.Validate(); err != nil {
return err
Expand All @@ -38,6 +39,7 @@ func (m *Messenger) SetLogLevel(request *requests.SetLogLevel) error {
return nodecfg.SetLogLevel(m.database, request.LogLevel)
}

// Deprecated: Use SetLogNamespaces from status.go instead.
func (m *Messenger) SetLogNamespaces(request *requests.SetLogNamespaces) error {
if err := request.Validate(); err != nil {
return err
Expand Down
54 changes: 27 additions & 27 deletions tests-functional/tests/test_logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from resources.constants import USER_DIR
import re
from test_cases import StatusBackend
import pytest
import os


@pytest.mark.rpc
@pytest.mark.skip("waiting for status-backend to be executed on the same host/container")
class TestLogging:

@pytest.mark.init
Expand All @@ -20,46 +21,45 @@ def test_logging(self, tmp_path):
assert backend_client is not None

# Init and login
backend_client.init_status_backend(data_dir=str(tmp_path))
backend_client.create_account_and_login(data_dir=str(tmp_path))
backend_client.init_status_backend()
backend_client.create_account_and_login()
key_uid = self.ensure_logged_in(backend_client)

# Configure logging
backend_client.rpc_valid_request("wakuext_setLogLevel", [{"logLevel": "ERROR"}])
backend_client.rpc_valid_request(
"wakuext_setLogNamespaces",
[{"logNamespaces": "test1.test2:debug,test1.test2.test3:info"}],
)
backend_client.api_valid_request("SetLogLevel", {"logLevel": "ERROR"})
backend_client.api_valid_request("SetLogNamespaces", {"logNamespaces": "test1.test2:debug,test1.test2.test3:info"})

# Re-login (logging settings take effect after re-login)
log_pattern = [
r"DEBUG\s+test1\.test2\s+",
r"INFO\s+test1\.test2\s+",
r"INFO\s+test1\.test2\.test3\s+",
r"WARN\s+test1\.test2\s+",
r"WARN\s+test1\.test2\.test3\s+",
r"ERROR\s+test1\s+",
r"ERROR\s+test1\.test2\s+",
r"ERROR\s+test1\.test2\.test3\s+",
]

# Ensure changes take effect at runtime
backend_client.rpc_valid_request("wakuext_logTest")
geth_log = backend_client.extract_data(os.path.join(USER_DIR, "geth.log"))
self.expect_logs(geth_log, "test message", log_pattern, count=1)

# Ensure changes are persisted after re-login
backend_client.logout()
backend_client.login(str(key_uid))
self.ensure_logged_in(backend_client)

# Test logging
backend_client.rpc_valid_request("wakuext_logTest")
self.expect_logs(
tmp_path / "geth.log",
"test message",
[
r"DEBUG\s+test1\.test2",
r"INFO\s+test1\.test2",
r"INFO\s+test1\.test2\.test3",
r"WARN\s+test1\.test2",
r"WARN\s+test1\.test2\.test3",
r"ERROR\s+test1",
r"ERROR\s+test1\.test2",
r"ERROR\s+test1\.test2\.test3",
],
)
geth_log = backend_client.extract_data(os.path.join(USER_DIR, "geth.log"))
self.expect_logs(geth_log, "test message", log_pattern, count=2)

def expect_logs(self, log_file, filter_keyword, expected_logs):
def expect_logs(self, log_file, filter_keyword, expected_logs, count):
with open(log_file, "r") as f:
log_content = f.read()

filtered_logs = [line for line in log_content.splitlines() if filter_keyword in line]
for expected_log in expected_logs:
assert any(re.search(expected_log, log) for log in filtered_logs), f"Log entry not found: {expected_log}"
assert sum(1 for log in filtered_logs if re.search(expected_log, log)) == count, f"Log entry not found or count mismatch: {expected_log}"

def ensure_logged_in(self, backend_client):
login_response = backend_client.wait_for_signal("node.login")
Expand Down
Loading