Remove unwanted newline character at end of timestamp by arista-nwolfe · Pull Request #3111 · sonic-net/sonic-swss

arista-nwolfe · 2024-04-10T19:01:58Z

In #3052 2 new fields were added the PORT_TABLE in APPL_DB: last_down_time and last_up_time.
But their value was generated with std::ctime which includes a newline character at the end of the character array which causes issues for parsing the output.

Output today:

> sonic-db-cli APPL_DB hgetall PORT_TABLE:Ethernet8
{'admin_status': 'up', 'alias': 'Ethernet3/1', 'asic_port_name': 'Eth8', 'coreId': '0', 'corePortId': '3', 'description': 'Ethernet8-connected-to-@Ethernet5/4/1', 'fec': 'rs', 'index': '3', 'lanes': '4,5', 'mtu': '9100', 'numVoq': '8', 'pfc_asym': 'off', 'role': 'Ext', 'speed': '100000', 'tpid': '0x8100', 'oper_status': 'up', 'flap_count': '1', 'last_up_time': 'Wed Apr 10 00:18:44 2024
', 'system_oper_status': 'up', 'line_oper_status': 'up'}

> config interface shutdown Ethernet8
> sonic-db-cli APPL_DB hgetall PORT_TABLE:Ethernet8
{'admin_status': 'down', 'alias': 'Ethernet3/1', 'asic_port_name': 'Eth8', 'coreId': '0', 'corePortId': '3', 'description': 'Ethernet8-connected-to-@Ethernet5/4/1', 'fec': 'rs', 'index': '3', 'lanes': '4,5', 'mtu': '9100', 'numVoq': '8', 'pfc_asym': 'off', 'role': 'Ext', 'speed': '100000', 'tpid': '0x8100', 'oper_status': 'down', 'flap_count': '2', 'last_up_time': 'Wed Apr 10 00:18:44 2024
', 'system_oper_status': 'down', 'line_oper_status': 'down', 'last_down_time': 'Wed Apr 10 16:35:11 2024
'}

>  config interface startup Ethernet8
> sonic-db-cli APPL_DB hgetall PORT_TABLE:Ethernet8
{'admin_status': 'up', 'alias': 'Ethernet3/1', 'asic_port_name': 'Eth8', 'coreId': '0', 'corePortId': '3', 'description': 'Ethernet8-connected-to-@Ethernet5/4/1', 'fec': 'rs', 'index': '3', 'lanes': '4,5', 'mtu': '9100', 'numVoq': '8', 'pfc_asym': 'off', 'role': 'Ext', 'speed': '100000', 'tpid': '0x8100', 'oper_status': 'up', 'flap_count': '3', 'last_up_time': 'Wed Apr 10 16:35:34 2024
', 'system_oper_status': 'up', 'line_oper_status': 'up', 'last_down_time': 'Wed Apr 10 16:35:11 2024
'}

Output after this change:

> sonic-db-cli APPL_DB hgetall PORT_TABLE:Ethernet8
{'admin_status': 'up', 'alias': 'Ethernet3/1', 'asic_port_name': 'Eth8', 'coreId': '0', 'corePortId': '3', 'description': 'Ethernet8-connected-to-@Ethernet5/4/1', 'fec': 'rs', 'index': '3', 'lanes': '4,5', 'mtu': '9100', 'numVoq': '8', 'pfc_asym': 'off', 'role': 'Ext', 'speed': '100000', 'tpid': '0x8100', 'oper_status': 'up', 'flap_count': '1', 'last_up_time': 'Wed Apr 10 17:01:00 2024', 'system_oper_status': 'up', 'line_oper_status': 'up'}

> config interface shutdown Ethernet8
> sonic-db-cli APPL_DB hgetall PORT_TABLE:Ethernet8
{'admin_status': 'down', 'alias': 'Ethernet3/1', 'asic_port_name': 'Eth8', 'coreId': '0', 'corePortId': '3', 'description': 'Ethernet8-connected-to-@Ethernet5/4/1', 'fec': 'rs', 'index': '3', 'lanes': '4,5', 'mtu': '9100', 'numVoq': '8', 'pfc_asym': 'off', 'role': 'Ext', 'speed': '100000', 'tpid': '0x8100', 'oper_status': 'down', 'flap_count': '2', 'last_up_time': 'Wed Apr 10 17:01:00 2024', 'system_oper_status': 'down', 'line_oper_status': 'down', 'last_down_time': 'Wed Apr 10 18:37:03 2024'}

> config interface startup Ethernet8
> sonic-db-cli APPL_DB hgetall PORT_TABLE:Ethernet8
{'admin_status': 'up', 'alias': 'Ethernet3/1', 'asic_port_name': 'Eth8', 'coreId': '0', 'corePortId': '3', 'description': 'Ethernet8-connected-to-@Ethernet5/4/1', 'fec': 'rs', 'index': '3', 'lanes': '4,5', 'mtu': '9100', 'numVoq': '8', 'pfc_asym': 'off', 'role': 'Ext', 'speed': '100000', 'tpid': '0x8100', 'oper_status': 'up', 'flap_count': '3', 'last_up_time': 'Wed Apr 10 18:37:13 2024', 'system_oper_status': 'up', 'line_oper_status': 'up', 'last_down_time': 'Wed Apr 10 18:37:03 2024'}

prsunny · 2024-04-10T19:58:04Z

Can this be caught by VS test?

arista-nwolfe · 2024-04-10T23:21:49Z

/azpw run Azure.sonic-swss

mssonicbld · 2024-04-10T23:21:51Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2024-04-10T23:22:00Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2024-04-14T07:24:14Z

orchagent/portsorch.cpp

-        FieldValueTuple tuple("last_down_time", std::ctime(&now_c));
+        char buffer[32];
+        // Format: Www Mmm dd hh:mm:ss yyyy
+        std::strftime(buffer, sizeof(buffer), "%a %b %d %H:%M:%S %Y", std::localtime(&now_c));


@arista-nwolfe can we keep the time in UTC since the syslog event time is in UTC?

Will do, note that previously it was already in local time with ctime

arista-nwolfe · 2024-04-22T22:00:12Z

Can this be caught by VS test?

This PR is the fix for a regression in a feature introduced by someone else in #3052
I've opened #3120 to track adding a test for it.

arista-nwolfe · 2024-04-23T18:55:57Z

/azpw run Azure.sonic-swss

mssonicbld · 2024-04-23T18:56:00Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2024-04-23T18:56:10Z

Azure Pipelines successfully started running 1 pipeline(s).

arista-nwolfe · 2024-05-02T21:22:33Z

/azpw run Azure.sonic-swss

mssonicbld · 2024-05-02T21:22:36Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2024-05-02T21:22:46Z

Azure Pipelines successfully started running 1 pipeline(s).

arlakshm · 2024-05-03T21:02:00Z

@prsunny, can you help merge this?

prsunny · 2024-05-03T21:40:57Z

@prgeor , @arlakshm , could you folks signoff?

justin-wong-ce · 2024-08-06T22:36:13Z

@arista-nwolfe A backport to 202311 is needed to fix a regression caused by a backport of this PR: #3052

Can you add a Request for 202311 Branch label please?

arista-nwolfe · 2024-08-06T22:42:38Z

@arista-nwolfe A backport to 202311 is needed to fix a regression caused by a backport of this PR: #3052

Can you add a Request for 202311 Branch label please?

I don't have label permissions @wenyiz2021 @arlakshm @vmittal-msft would one of you be able to add it.

StormLiangMS · 2024-08-08T04:01:50Z

hi @arista-nwolfe could you run test with 202311 for the cherrypick?

StormLiangMS · 2024-08-08T04:56:05Z

@yxieca could you help to cherry pick?

arista-nwolfe · 2024-08-08T16:42:58Z

hi @arista-nwolfe could you run test with 202311 for the cherrypick?

Tested this on 202311

* Remove unwanted newline character at end of timestamp * Cleaner approach to generate timestamp string with no newline character * Use UTC rather than local time in timestamp

mssonicbld · 2024-08-08T22:29:20Z

Cherry-pick PR to 202311: #3256

* Remove unwanted newline character at end of timestamp * Cleaner approach to generate timestamp string with no newline character * Use UTC rather than local time in timestamp

arista-nwolfe requested a review from prsunny as a code owner April 10, 2024 19:01

prsunny requested a review from prgeor April 10, 2024 19:57

arista-nwolfe force-pushed the master-fix-newline-on-timestamp branch from 160c9a4 to 9b68530 Compare April 11, 2024 16:31

arlakshm previously approved these changes Apr 12, 2024

View reviewed changes

arista-nwolfe force-pushed the master-fix-newline-on-timestamp branch from 0608aba to 740340b Compare April 12, 2024 20:35

prgeor reviewed Apr 14, 2024

View reviewed changes

arista-nwolfe dismissed arlakshm’s stale review via 9127511 April 15, 2024 20:58

arista-nwolfe force-pushed the master-fix-newline-on-timestamp branch from 9127511 to 1bf7345 Compare April 16, 2024 19:09

rlhui assigned arista-nwolfe Apr 17, 2024

arista-nwolfe force-pushed the master-fix-newline-on-timestamp branch from 1bf7345 to f6c3383 Compare April 17, 2024 18:09

arista-nwolfe mentioned this pull request Apr 22, 2024

Add VS Test to catch newlines in REDIS values #3120

Open

arista-nwolfe added 3 commits May 2, 2024 14:28

Remove unwanted newline character at end of timestamp

414028a

Cleaner approach to generate timestamp string with no newline character

64c61e9

Use UTC rather than local time in timestamp

f2d6606

arista-nwolfe force-pushed the master-fix-newline-on-timestamp branch from ccb19ba to f2d6606 Compare May 2, 2024 21:28

arlakshm approved these changes May 6, 2024

View reviewed changes

prgeor approved these changes May 7, 2024

View reviewed changes

prsunny merged commit 837e467 into sonic-net:master May 7, 2024

justin-wong-ce mentioned this pull request Aug 6, 2024

Add port flap count and last flap timestamp to APPL_DB #3052

Merged

StormLiangMS added the Request for 202311 Branch label Aug 8, 2024

yxieca added the Approved for 202311 Branch label Aug 8, 2024

mssonicbld mentioned this pull request Aug 8, 2024

[action] [PR:3111] Remove unwanted newline character at end of timestamp (#3111) #3256

Merged

mssonicbld added the Created PR to 202311 Branch label Aug 8, 2024

mssonicbld added Included in 202311 Branch and removed Created PR to 202311 Branch labels Aug 8, 2024

Conversation

arista-nwolfe commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prsunny commented Apr 10, 2024

Uh oh!

arista-nwolfe commented Apr 10, 2024

Uh oh!

mssonicbld commented Apr 10, 2024

Uh oh!

azure-pipelines bot commented Apr 10, 2024

Uh oh!

prgeor Apr 14, 2024

Choose a reason for hiding this comment

Uh oh!

arista-nwolfe Apr 15, 2024

Choose a reason for hiding this comment

Uh oh!

arista-nwolfe commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arista-nwolfe commented Apr 23, 2024

Uh oh!

mssonicbld commented Apr 23, 2024

Uh oh!

azure-pipelines bot commented Apr 23, 2024

Uh oh!

arista-nwolfe commented May 2, 2024

Uh oh!

mssonicbld commented May 2, 2024

Uh oh!

azure-pipelines bot commented May 2, 2024

Uh oh!

arlakshm commented May 3, 2024

Uh oh!

prsunny commented May 3, 2024

Uh oh!

justin-wong-ce commented Aug 6, 2024

Uh oh!

arista-nwolfe commented Aug 6, 2024

Uh oh!

StormLiangMS commented Aug 8, 2024

Uh oh!

StormLiangMS commented Aug 8, 2024

Uh oh!

arista-nwolfe commented Aug 8, 2024

Uh oh!

mssonicbld commented Aug 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

arista-nwolfe commented Apr 10, 2024 •

edited

Loading

arista-nwolfe commented Apr 22, 2024 •

edited

Loading