Skip to content

[trim]: Update log level severity to avoid errors during capabilities query#3916

Merged
dgsudharsan merged 3 commits intosonic-net:masterfrom
nazariig:master-trim-log-lvl
Oct 16, 2025
Merged

[trim]: Update log level severity to avoid errors during capabilities query#3916
dgsudharsan merged 3 commits intosonic-net:masterfrom
nazariig:master-trim-log-lvl

Conversation

@nazariig
Copy link
Collaborator

@nazariig nazariig commented Oct 6, 2025

Signed-off-by: Nazarii Hnydyn nazariig@nvidia.com

These changes help to avoid flooding log with errors during Packet Trimming attribute/enum capabilities check

HLD: sonic-net/SONiC#2033

What I did

  • Updated log level severity during capabilities check

Why I did it

  • To avoid flooding log with errors during capabilities check

How I verified it

  • Run VS UTs

Details if related

      .---.        .-----------
     /     \  __  /    ------
    / /     \(  )/    -----
   //////   ' \/ `   ---
  //// / // :    : ---
 // /   /  /`    '--
//          //..\\
       ====UU====UU====
           '//||\\`
             ''``

@nazariig nazariig requested a review from prsunny as a code owner October 6, 2025 12:39
@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 6, 2025

/azp run

@nazariig nazariig requested a review from kperumalbfn October 6, 2025 12:39
@azure-pipelines
Copy link

azure-pipelines bot commented Oct 6, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
Copy link
Collaborator Author

nazariig commented Oct 6, 2025

@dhanasekar-arista FYI

@dhanasekar-arista
Copy link
Contributor

dhanasekar-arista commented Oct 6, 2025

@dhanasekar-arista FYI

By logging these events at NOTICE level, we may miss some legitimate errors as well. While sonic-mgmt log analyzer may still see the NOTICE level entries and report them, this won't help customers who rely solely on monitoring the ERROR log level.

@nazariig
Copy link
Collaborator Author

nazariig commented Oct 6, 2025

@dhanasekar-arista FYI

By logging these events at NOTICE level, we may miss some legitimate errors as well. While sonic-mgmt log analyzer may still see the NOTICE level entries and report them, this won't help customers who rely solely on monitoring the ERROR log level.

@kperumalbfn please share your opinion

@prsunny prsunny requested a review from mramezani95 October 6, 2025 17:01
@prsunny
Copy link
Collaborator

prsunny commented Oct 6, 2025

@kperumalbfn , @developfast , please review

developfast
developfast requested changes Oct 6, 2025
View reviewed changes
Copy link
Contributor

@developfast developfast left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This change is a nice improvement - toStr(status) helper and the reduction in log flooding both make the logs much more actionable.

That said, I share some of the concern raised about blanket use of NOTICE. By lowering all non-success statuses, we risk missing genuinely unexpected errors, especially for customers who monitor only ERROR.

Would it make sense to distinguish cases here? For example:

Log SAI_STATUS_NOT_SUPPORTED (expected) at INFO/DEBUG.

Log other non-success statuses (e.g., INVALID_PARAMETER, FAILURE) at ERROR.

This way we cut down the noise while still preserving visibility into real errors that operators should act on.

@kperumalbfn
Copy link
Contributor

kperumalbfn commented Oct 6, 2025

@dhanasekar-arista FYI

By logging these events at NOTICE level, we may miss some legitimate errors as well. While sonic-mgmt log analyzer may still see the NOTICE level entries and report them, this won't help customers who rely solely on monitoring the ERROR log level.

@kperumalbfn please share your opinion

NOTICE is still be part of sonic syslog as the default log level is NOTICE for swss.

dhanasekar-arista
dhanasekar-arista reviewed Oct 7, 2025
View reviewed changes
orchagent/switch/trimming/capabilities.cpp
capList.resize(enumList.count);
enumList.list = capList.data();

return sai_query_attribute_enum_values_capability(gSwitchId, objType, attrId, &enumList);
Copy link
Contributor

@dhanasekar-arista dhanasekar-arista Oct 7, 2025

Choose a reason for hiding this comment

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

@kperumalbfn @nazariig IMO, We should suppress the unsupported error log here and proceed by returning a SUCCESS status. The same handling should also be implemented in queryAttrCapabilitiesSai.
This would not mask the ERROR logs.

Copy link
Collaborator Author

@nazariig nazariig Oct 7, 2025

Choose a reason for hiding this comment

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

@dhanasekar-arista by design queryEnumCapabilitiesSai is a generic API and it simply forwards the result without adding any extra business logic. It is a client code's responsibility to handle the status properly and do the logging using the relevant severity level

@dhanasekar-arista
Copy link
Contributor

dhanasekar-arista commented Oct 7, 2025

@dhanasekar-arista FYI

By logging these events at NOTICE level, we may miss some legitimate errors as well. While sonic-mgmt log analyzer may still see the NOTICE level entries and report them, this won't help customers who rely solely on monitoring the ERROR log level.

@kperumalbfn please share your opinion

NOTICE is still be part of sonic syslog as the default log level is NOTICE for swss.

Yes, but customer syslog monitoring tool may have different alarms raised for different levels. And there is a chance that a valid error log may be missed. Hence i'm not in favor of this change.

@nazariig nazariig force-pushed the master-trim-log-lvl branch from 08372cb to 495d00e Compare October 8, 2025 07:52
@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 8, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Oct 8, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
Copy link
Collaborator Author

nazariig commented Oct 8, 2025

/azpw run

@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 8, 2025

/AzurePipelines run

@azure-pipelines
Copy link

azure-pipelines bot commented Oct 8, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@developfast
Copy link
Contributor

developfast commented Oct 9, 2025

A bunch of random trimming and macsec vstests are failing not related to pr change @kperumalbfn - should we bypass?

developfast
developfast approved these changes Oct 9, 2025
View reviewed changes
Copy link
Contributor

@developfast developfast left a comment

Choose a reason for hiding this comment

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

soft approval

@nazariig nazariig force-pushed the master-trim-log-lvl branch from 495d00e to 0679665 Compare October 13, 2025 08:28
@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 13, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Oct 13, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@nazariig
[trim]: Update log level severity to avoid errors during attribute/en…
762682f 
…um capabilities query

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
@nazariig nazariig force-pushed the master-trim-log-lvl branch from 0679665 to 762682f Compare October 13, 2025 17:13
@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 13, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Oct 13, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 15, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Oct 15, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 16, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Oct 16, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@dgsudharsan dgsudharsan merged commit ab96400 into sonic-net:master Oct 16, 2025
15 checks passed
stepanblyschak pushed a commit to stepanblyschak/sonic-swss that referenced this pull request Oct 31, 2025
@nazariig @dgsudharsan @stepanblyschak
[trim]: Update log level severity to avoid errors during attribute/en…
cd65fdb 
…um capabilities query (sonic-net#3916)

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Co-authored-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
balanokia pushed a commit to balanokia/sonic-swss that referenced this pull request Nov 17, 2025
@nazariig @dgsudharsan @balanokia
[trim]: Update log level severity to avoid errors during attribute/en…
53343a9 
…um capabilities query (sonic-net#3916)

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Co-authored-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
theasianpianist pushed a commit to theasianpianist/sonic-swss that referenced this pull request Feb 4, 2026
@nazariig @dgsudharsan @theasianpianist
[trim]: Update log level severity to avoid errors during attribute/en…
2069683 
…um capabilities query (sonic-net#3916)

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Co-authored-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
baorliu pushed a commit to baorliu/sonic-swss that referenced this pull request Feb 23, 2026
@nazariig @dgsudharsan @baorliu
[trim]: Update log level severity to avoid errors during attribute/en…
6a09438 
…um capabilities query (sonic-net#3916)

Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Co-authored-by: Sudharsan Dhamal Gopalarathnam <sudharsand@nvidia.com>
Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dgsudharsan dgsudharsan dgsudharsan approved these changes

@volodymyrsamotiy volodymyrsamotiy volodymyrsamotiy approved these changes

@kperumalbfn kperumalbfn kperumalbfn approved these changes

@developfast developfast developfast approved these changes

@prsunny prsunny Awaiting requested review from prsunny prsunny is a code owner

@r12f r12f Awaiting requested review from r12f

@liat-grozovik liat-grozovik Awaiting requested review from liat-grozovik

@moshemos moshemos Awaiting requested review from moshemos

@keboliu keboliu Awaiting requested review from keboliu

@mramezani95 mramezani95 Awaiting requested review from mramezani95

+1 more reviewer

@dhanasekar-arista dhanasekar-arista dhanasekar-arista left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Enhancement ➕

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@nazariig @mssonicbld @dhanasekar-arista @prsunny @kperumalbfn @developfast @dgsudharsan @volodymyrsamotiy