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.

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

Conversation

osmaczko
Copy link
Contributor

Add endpoints for log level and namespaces configuration to status.go and deprecate equivalents from service.

Endpoints are defined in status.go, as it has access to statusBackend, the only entity capable of manipulating node configuration without requiring a restart.

@osmaczko osmaczko self-assigned this Dec 12, 2024
@status-im-auto
Copy link
Member

status-im-auto commented Dec 12, 2024

Jenkins Builds

Click to see older builds (24)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 7c70372 #1 2024-12-12 20:07:31 ~4 min linux 📦zip
✔️ 7c70372 #1 2024-12-12 20:07:33 ~4 min macos 📦zip
✔️ 7c70372 #1 2024-12-12 20:07:41 ~4 min ios 📦zip
✔️ 7c70372 #1 2024-12-12 20:08:35 ~5 min android 📦aar
✔️ 7c70372 #1 2024-12-12 20:08:41 ~5 min windows 📦zip
✔️ 7c70372 #1 2024-12-12 20:08:46 ~5 min macos 📦zip
✔️ 7c70372 #1 2024-12-12 20:09:43 ~6 min tests-rpc 📄log
✔️ 7c70372 #1 2024-12-12 20:34:56 ~31 min tests 📄log
✔️ 3b47ede #2 2024-12-18 18:32:39 ~4 min macos 📦zip
✔️ 3b47ede #2 2024-12-18 18:32:57 ~4 min ios 📦zip
✔️ 3b47ede #2 2024-12-18 18:33:24 ~5 min macos 📦zip
✔️ 3b47ede #2 2024-12-18 18:33:32 ~5 min android 📦aar
✔️ 3b47ede #2 2024-12-18 18:33:35 ~5 min linux 📦zip
✔️ 3b47ede #2 2024-12-18 18:33:50 ~5 min windows 📦zip
✔️ 3b47ede #2 2024-12-18 18:34:31 ~6 min tests-rpc 📄log
✔️ 3b47ede #2 2024-12-18 18:58:58 ~30 min tests 📄log
✔️ ee0feaa #3 2024-12-19 14:25:05 ~3 min windows 📦zip
✔️ ee0feaa #3 2024-12-19 14:25:25 ~4 min macos 📦zip
✔️ ee0feaa #3 2024-12-19 14:25:50 ~4 min linux 📦zip
✔️ ee0feaa #3 2024-12-19 14:26:05 ~5 min ios 📦zip
✔️ ee0feaa #3 2024-12-19 14:26:17 ~5 min macos 📦zip
✖️ ee0feaa #3 2024-12-19 14:26:58 ~5 min tests-rpc 📄log
✔️ ee0feaa #3 2024-12-19 14:27:42 ~6 min android 📦aar
✔️ ee0feaa #3 2024-12-19 14:53:16 ~32 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ a46fe30 #4 2024-12-19 14:50:22 ~4 min windows 📦zip
✔️ a46fe30 #4 2024-12-19 14:50:39 ~4 min macos 📦zip
✔️ a46fe30 #4 2024-12-19 14:50:58 ~4 min ios 📦zip
✔️ a46fe30 #4 2024-12-19 14:51:16 ~5 min linux 📦zip
✔️ a46fe30 #4 2024-12-19 14:51:28 ~5 min android 📦aar
✔️ a46fe30 #4 2024-12-19 14:51:38 ~5 min macos 📦zip
✖️ a46fe30 #4 2024-12-19 14:52:27 ~6 min tests-rpc 📄log
✔️ a46fe30 #4 2024-12-19 15:22:08 ~28 min tests 📄log
✔️ f8ad1da #5 2024-12-19 19:33:04 ~3 min windows 📦zip
✔️ f8ad1da #5 2024-12-19 19:33:18 ~4 min ios 📦zip
✔️ f8ad1da #5 2024-12-19 19:33:26 ~4 min macos 📦zip
✔️ f8ad1da #5 2024-12-19 19:34:14 ~5 min linux 📦zip
✔️ f8ad1da #5 2024-12-19 19:34:26 ~5 min android 📦aar
✔️ f8ad1da #5 2024-12-19 19:34:34 ~5 min macos 📦zip
✔️ f8ad1da #5 2024-12-19 19:35:18 ~6 min tests-rpc 📄log
✔️ f8ad1da #5 2024-12-19 19:58:17 ~29 min tests 📄log

@osmaczko
Copy link
Contributor Author

Functional test results:

Running pytest with args: ['-p', 'vscode_pytest', '--status_backend_urls=http://localhost:42995', '--rootdir=/path/to/status-go/tests-functional', '/path/to/status-go/tests-functional/tests/test_logging.py::TestLogging::test_logging']
============================= test session starts ==============================
platform linux -- Python 3.11.9, pytest-6.2.4, py-1.11.0, pluggy-0.13.1 -- /bin/python
cachedir: .pytest_cache
rootdir: /path/to/status-go/tests-functional, configfile: pytest.ini
plugins: dependency-0.6.0
collecting ... collected 1 item

tests/test_logging.py::TestLogging::test_logging 
-------------------------------- live log call ---------------------------------
INFO     root:status_backend.py:39 Sending POST request to url http://localhost:42995/statusgo/InitializeApplication with data: {
    "apiLogging": true,
    "dataDir": "/tmp/pytest-of-myself/pytest-85/test_logging0",
    "logEnabled": true,
    "logLevel": "DEBUG"
}
INFO     root:signals.py:121 Connection opened
WARNING  websocket:_logging.py:66 websocket connected
INFO     root:status_backend.py:41 Got response: b'{"accounts":null,"centralizedMetricsInfo":{"enabled":false,"userConfirmed":false}}'
INFO     root:status_backend.py:47 Got response: b'{"accounts":null,"centralizedMetricsInfo":{"enabled":false,"userConfirmed":false}}'
INFO     root:status_backend.py:39 Sending POST request to url http://localhost:42995/statusgo/CreateAccountAndLogin with data: {
    "customizationColor": "primary",
    "displayName": "Mr_Meeseeks",
    "kdfIterations": 256000,
    "logEnabled": true,
    "logLevel": "DEBUG",
    "password": "Strong12345",
    "rootDataDir": "/tmp/pytest-of-myself/pytest-85/test_logging0"
}
INFO     root:status_backend.py:41 Got response: b'{"error":""}'
INFO     root:status_backend.py:47 Got response: b'{"error":""}'
INFO     root:status_backend.py:39 Sending POST request to url http://localhost:42995/statusgo/SetLogLevel with data: {
    "logLevel": "ERROR"
}
INFO     root:status_backend.py:41 Got response: b'{"error":""}'
INFO     root:status_backend.py:47 Got response: b'{"error":""}'
INFO     root:status_backend.py:39 Sending POST request to url http://localhost:42995/statusgo/SetLogNamespaces with data: {
    "logNamespaces": "test1.test2:debug,test1.test2.test3:info"
}
INFO     root:status_backend.py:41 Got response: b'{"error":""}'
INFO     root:status_backend.py:47 Got response: b'{"error":""}'
INFO     root:rpc.py:54 Sending POST request to url http://localhost:42995/statusgo/CallRPC with data: {
    "id": null,
    "jsonrpc": "2.0",
    "method": "wakuext_logTest"
}
INFO     root:rpc.py:58 Got response: {
    "id": null,
    "jsonrpc": "2.0",
    "result": null
}
INFO     root:status_backend.py:39 Sending POST request to url http://localhost:42995/statusgo/Logout with data: {}
INFO     root:status_backend.py:41 Got response: b'{"error":""}'
INFO     root:status_backend.py:47 Got response: b'{"error":""}'
INFO     root:status_backend.py:39 Sending POST request to url http://localhost:42995/statusgo/LoginAccount with data: {
    "kdfIterations": 256000,
    "keyUid": "0x42d81d7344e88629b9bf94536a732680683a8a0f42d1422cf8f24a0b5699bdb7",
    "password": "Strong12345"
}
INFO     root:status_backend.py:41 Got response: b'{"error":""}'
INFO     root:status_backend.py:47 Got response: b'{"error":""}'
INFO     root:rpc.py:54 Sending POST request to url http://localhost:42995/statusgo/CallRPC with data: {
    "id": null,
    "jsonrpc": "2.0",
    "method": "wakuext_logTest"
}
INFO     root:rpc.py:58 Got response: {
    "id": null,
    "jsonrpc": "2.0",
    "result": null
}
PASSED

============================== 1 passed in 12.55s ==============================
Finished running tests!

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 19 lines in your changes missing coverage. Please review.

Project coverage is 53.19%. Comparing base (0cf556b) to head (f8ad1da).

Files with missing lines Patch % Lines
mobile/status.go 51.85% 9 Missing and 4 partials ⚠️
api/geth_backend.go 72.72% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6210      +/-   ##
===========================================
- Coverage    53.25%   53.19%   -0.07%     
===========================================
  Files          834      834              
  Lines       134104   134153      +49     
===========================================
- Hits         71413    71358      -55     
- Misses       54816    54901      +85     
- Partials      7875     7894      +19     
Flag Coverage Δ
functional 20.07% <64.91%> (-0.50%) ⬇️
unit 60.00% <15.21%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
logutils/core.go 80.59% <100.00%> (+3.32%) ⬆️
params/config.go 59.15% <100.00%> (+0.54%) ⬆️
protocol/messenger_settings.go 20.51% <ø> (ø)
api/geth_backend.go 46.97% <72.72%> (+0.85%) ⬆️
mobile/status.go 11.04% <51.85%> (+1.54%) ⬆️

... and 86 files with indirect coverage changes

Copy link
Collaborator

@igor-sirotin igor-sirotin left a comment

Choose a reason for hiding this comment

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

👏

params/config.go Outdated Show resolved Hide resolved
Copy link
Contributor

@qfrank qfrank left a comment

Choose a reason for hiding this comment

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

👏

@osmaczko osmaczko force-pushed the chore/setup-logging-at-runtime branch 3 times, most recently from ee0feaa to a46fe30 Compare December 19, 2024 14:45
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.

Add endpoints for log level and namespaces configuration to
`status.go` and deprecate equivalents from service.

Endpoints are defined in `status.go`, as it has access to
`statusBackend`, the only entity capable of manipulating node
configuration without requiring a restart.
@osmaczko osmaczko force-pushed the chore/setup-logging-at-runtime branch from a46fe30 to f8ad1da Compare December 19, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Code Review
Development

Successfully merging this pull request may close these issues.

5 participants