feat_: support enable and disable log #6347

qfrank · 2025-02-17T10:20:10Z

this PR added a new endpoint SetLogEnabled so that frontend can enable log when log is disabled by default for release build.

relate mobile PR
relate comment

NOTE: I haven't review the way we store settings and all this mess with NodeConfig yet.

status-im-auto · 2025-02-17T10:22:36Z

Jenkins Builds

Click to see older builds (24)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✖️	`726441b`	#1	2025-02-17 10:22:35	~2 min	`tests-rpc`	📄`log`
✔️	`726441b`	#1	2025-02-17 10:24:24	~3 min	`ios`	📦`zip`
✔️	`726441b`	#1	2025-02-17 10:24:36	~4 min	`macos`	📦`zip`
✔️	`726441b`	#1	2025-02-17 10:25:02	~4 min	`windows`	📦`zip`
✔️	`726441b`	#1	2025-02-17 10:25:42	~5 min	`linux`	📦`zip`
✔️	`726441b`	#1	2025-02-17 10:26:27	~5 min	`macos`	📦`zip`
✔️	`726441b`	#1	2025-02-17 10:26:38	~6 min	`android`	📦`aar`
✔️	`726441b`	#1	2025-02-17 10:51:26	~30 min	`tests`	📄`log`

✖️	`cca7ce8`	#2	2025-02-17 10:38:42	~1 min	`tests-rpc`	📄`log`
✔️	`cca7ce8`	#2	2025-02-17 10:40:19	~3 min	`windows`	📦`zip`
✔️	`cca7ce8`	#2	2025-02-17 10:41:43	~4 min	`ios`	📦`zip`
✔️	`cca7ce8`	#2	2025-02-17 10:41:53	~4 min	`macos`	📦`zip`
✔️	`cca7ce8`	#2	2025-02-17 10:42:43	~5 min	`linux`	📦`zip`
✔️	`cca7ce8`	#2	2025-02-17 10:43:05	~5 min	`macos`	📦`zip`
✔️	`cca7ce8`	#2	2025-02-17 10:43:21	~6 min	`android`	📦`aar`
✔️	`cca7ce8`	#2	2025-02-17 11:22:17	~30 min	`tests`	📄`log`

✔️	`8556857`	#3	2025-02-18 05:51:01	~3 min	`android`	📦`aar`
✔️	`8556857`	#3	2025-02-18 05:51:42	~3 min	`ios`	📦`zip`
✔️	`8556857`	#3	2025-02-18 05:51:49	~3 min	`macos`	📦`zip`
✔️	`8556857`	#3	2025-02-18 05:52:08	~4 min	`windows`	📦`zip`
✔️	`8556857`	#3	2025-02-18 05:53:50	~5 min	`linux`	📦`zip`
✔️	`8556857`	#3	2025-02-18 05:54:01	~6 min	`macos`	📦`zip`
✔️	`8556857`	#3	2025-02-18 06:00:04	~12 min	`tests-rpc`	📄`log`
✔️	`8556857`	#3	2025-02-18 06:19:29	~31 min	`tests`	📄`log`

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`388fce1`	#4	2025-02-21 10:46:47	~2 min	`ios`	📦`zip`
✔️	`388fce1`	#4	2025-02-21 10:47:10	~3 min	`android`	📦`aar`
✔️	`388fce1`	#4	2025-02-21 10:47:50	~3 min	`windows`	📦`zip`
✔️	`388fce1`	#4	2025-02-21 10:49:29	~5 min	`linux`	📦`zip`
✔️	`388fce1`	#4	2025-02-21 10:49:30	~5 min	`macos`	📦`zip`
✔️	`388fce1`	#4	2025-02-21 10:50:51	~6 min	`macos`	📦`zip`
✔️	`388fce1`	#4	2025-02-21 10:56:37	~12 min	`tests-rpc`	📄`log`
✖️	`388fce1`	#4	2025-02-21 11:15:49	~31 min	`tests`	📄`log`
✖️	`388fce1`	#5	2025-02-21 14:08:48	~31 min	`tests`	📄`log`

✔️	`3fea0ef`	#5	2025-02-25 09:02:20	~2 min	`ios`	📦`zip`
✔️	`3fea0ef`	#5	2025-02-25 09:02:49	~3 min	`android`	📦`aar`
✔️	`3fea0ef`	#5	2025-02-25 09:03:06	~3 min	`windows`	📦`zip`
✔️	`3fea0ef`	#5	2025-02-25 09:03:46	~4 min	`macos`	📦`zip`
✔️	`3fea0ef`	#5	2025-02-25 09:05:50	~6 min	`linux`	📦`zip`
✔️	`3fea0ef`	#5	2025-02-25 09:07:26	~7 min	`macos`	📦`zip`
✔️	`3fea0ef`	#5	2025-02-25 09:12:31	~12 min	`tests-rpc`	📄`log`
✖️	`3fea0ef`	#6	2025-02-25 09:31:27	~31 min	`tests`	📄`log`
✖️	`3fea0ef`	#7	2025-02-25 14:30:10	~28 min	`tests`	📄`log`
✔️	`3fea0ef`	#8	2025-02-25 15:29:47	~29 min	`tests`	📄`log`

codecov · 2025-02-17T11:23:19Z

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 59.42%. Comparing base (8f30436) to head (3fea0ef).
Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
protocol/messenger_settings.go	0.00%	2 Missing ⚠️
services/ext/api.go	0.00%	2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (42.85%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #6347       +/-   ##
============================================
+ Coverage     0.45%   59.42%   +58.96%     
============================================
  Files          824      866       +42     
  Lines       109152   112634     +3482     
============================================
+ Hits           498    66932    +66434     
+ Misses      108597    37837    -70760     
- Partials        57     7865     +7808

Flag	Coverage Δ
functional	`0.45% <0.00%> (-0.01%)`	⬇️
unit	`59.41% <42.85%> (?)`

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

Files with missing lines	Coverage Δ
nodecfg/node_config.go	`70.24% <100.00%> (+70.24%)`	⬆️
protocol/messenger_settings.go	`25.00% <0.00%> (+25.00%)`	⬆️
services/ext/api.go	`6.30% <0.00%> (+6.30%)`	⬆️

... and 769 files with indirect coverage changes

ilmotta · 2025-02-19T04:04:26Z

this PR added a new endpoint SetLogEnabled so that frontend can enable log when log is disabled by default for release build.

@qfrank won't from now on all release builds in mobile have the error level set by default?

qfrank · 2025-02-19T05:13:00Z

this PR added a new endpoint SetLogEnabled so that frontend can enable log when log is disabled by default for release build.

@qfrank won't from now on all release builds in mobile have the error level set by default?

now we have pre_login.log and geth.log , and have the error level set for pre_login.log, and disabled log for geth.log for release builds. Maybe we can set error level for geth.log by default after moving android log from Downloads to app's protected directory?

this new endpoint is mainly used to solve the issue, mobile support disable/enable log operation, however, it only updated settings.LogLevel before and won't update table log_config.enabled which means it won't work actually. @ilmotta

ilmotta · 2025-02-20T05:33:52Z

Maybe we can set error level for geth.log by default after moving android log from Downloads to app's protected directory?

That seems reasonable to me @qfrank 💯

igor-sirotin

Looks ok to me, just ensure with @osmaczko that this is the approach we want now with the logs

osmaczko

Looks fine. It would also be possible to make it take effect at runtime. Please see: #6210, which deprecates service endpoints related to logging.

qfrank · 2025-02-21T10:31:34Z

Looks fine. It would also be possible to make it take effect at runtime. Please see: #6210, which deprecates service endpoints related to logging.

I'll take a look on how mobile can reuse take effect at runtime later, thank you!

Samyoul · 2025-02-25T13:51:00Z

I'm just sharing my findings here also.

Full `/api` test suite:

Running the tests locally I get 49 out of 53 tests passed. 0 tests failing, 4 tests terminating prematurely

Weirdly the tests "PASS"

TestRestoreAccountAndLogin

TestRestoreAccountAndLoginWithoutDisplayName

TestAcceptTerms

TestRestoreKeycardAccountAndLogin

TestConvertAccount works every time on my machine

Samyoul · 2025-02-25T14:26:06Z

CI https://ci.status.im/blue/organizations/jenkins/status-go%2Fprs%2Ftests/detail/PR-6347/6/tests/

`TestMessengerCommunitiesSuite/TestRetrieveBigCommunity`

Locally PASS

`TestMessengerCommunitiesSuite`

Locally PASS for entire Suite

`TestSyncDeviceSuite/TestPairAcceptContactRequest`

Locally PASS

`TestSyncDeviceSuite/TestPairDeclineContactRequest`

Locally PASS

Samyoul · 2025-02-25T14:48:28Z

PR Test History

qfrank · 2025-02-26T03:05:49Z

However, locally passing is based on the situation where the test may only run once; running it several times will yield different results, for example, TestRetrieveBigCommunity.

    communities_messenger_helpers_test.go:538: 
        	Error Trace:	/go/src/github.com/status-im/status-go/protocol/communities_messenger_helpers_test.go:538
        	            				/go/src/github.com/status-im/status-go/protocol/communities_messenger_test_suite_base_test.go:101
        	            				/go/src/github.com/status-im/status-go/protocol/communities_messenger_test.go:3941
        	Error:      	Received unexpected error:
        	            	user not accepted
        	Test:       	TestMessengerCommunitiesSuiteTestRetrieveBigCommunity

* chore_: remove private information for error log (#6334) * chore_: remove private information for error log * fix_: flaky test TestPairDeclineContactRequest * feat_: support enable and disable log (#6347)

qfrank requested review from ilmotta and igor-sirotin February 17, 2025 10:20

qfrank self-assigned this Feb 17, 2025

qfrank force-pushed the fix/no_log_file branch from 726441b to cca7ce8 Compare February 17, 2025 10:36

qfrank mentioned this pull request Feb 17, 2025

fix_: no log file status-im/status-mobile#22122

Merged

qfrank marked this pull request as ready for review February 17, 2025 10:59

qfrank force-pushed the fix/no_log_file branch from cca7ce8 to 8556857 Compare February 18, 2025 05:47

pavloburykh mentioned this pull request Feb 20, 2025

feat_: add pre login log status-im/status-mobile#21987

Merged

qfrank requested review from seanstrom and osmaczko February 20, 2025 09:40

ilmotta approved these changes Feb 20, 2025

View reviewed changes

igor-sirotin approved these changes Feb 21, 2025

View reviewed changes

osmaczko reviewed Feb 21, 2025

View reviewed changes

qfrank force-pushed the fix/no_log_file branch from 8556857 to 388fce1 Compare February 21, 2025 10:43

feat_: support enable and disable log

3fea0ef

qfrank force-pushed the fix/no_log_file branch from 388fce1 to 3fea0ef Compare February 25, 2025 08:59

Samyoul approved these changes Feb 26, 2025

View reviewed changes

igor-sirotin merged commit a3412cc into develop Feb 26, 2025
18 of 19 checks passed

igor-sirotin deleted the fix/no_log_file branch February 26, 2025 15:06

qfrank added a commit that referenced this pull request Feb 27, 2025

feat_: support enable and disable log (#6347)

074719d

qfrank mentioned this pull request Feb 27, 2025

chore_: cherry pick log feat #6371

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat_: support enable and disable log #6347

feat_: support enable and disable log #6347

qfrank commented Feb 17, 2025 •

edited

Loading

status-im-auto commented Feb 17, 2025 •

edited

Loading

codecov bot commented Feb 17, 2025 •

edited

Loading

ilmotta commented Feb 19, 2025

qfrank commented Feb 19, 2025 •

edited

Loading

ilmotta commented Feb 20, 2025

igor-sirotin left a comment

osmaczko left a comment

qfrank commented Feb 21, 2025

Samyoul commented Feb 25, 2025 •

edited

Loading

Samyoul commented Feb 25, 2025

Samyoul commented Feb 25, 2025

qfrank commented Feb 26, 2025

feat_: support enable and disable log #6347

feat_: support enable and disable log #6347

Conversation

qfrank commented Feb 17, 2025 • edited Loading

status-im-auto commented Feb 17, 2025 • edited Loading

Jenkins Builds

codecov bot commented Feb 17, 2025 • edited Loading

Codecov Report

ilmotta commented Feb 19, 2025

qfrank commented Feb 19, 2025 • edited Loading

ilmotta commented Feb 20, 2025

igor-sirotin left a comment

Choose a reason for hiding this comment

osmaczko left a comment

Choose a reason for hiding this comment

qfrank commented Feb 21, 2025

Samyoul commented Feb 25, 2025 • edited Loading

Full /api test suite:

Samyoul commented Feb 25, 2025

TestMessengerCommunitiesSuite/TestRetrieveBigCommunity

TestMessengerCommunitiesSuite

TestSyncDeviceSuite/TestPairAcceptContactRequest

TestSyncDeviceSuite/TestPairDeclineContactRequest

Samyoul commented Feb 25, 2025

PR Test History

qfrank commented Feb 26, 2025

qfrank commented Feb 17, 2025 •

edited

Loading

status-im-auto commented Feb 17, 2025 •

edited

Loading

codecov bot commented Feb 17, 2025 •

edited

Loading

qfrank commented Feb 19, 2025 •

edited

Loading

Samyoul commented Feb 25, 2025 •

edited

Loading

Full `/api` test suite:

`TestMessengerCommunitiesSuite/TestRetrieveBigCommunity`

`TestMessengerCommunitiesSuite`

`TestSyncDeviceSuite/TestPairAcceptContactRequest`

`TestSyncDeviceSuite/TestPairDeclineContactRequest`