Skip to content

[test] Build with ComponentStats-based SwssStats (PR sonic-swss#4516 + sonic-swss-common#1180)#26924

Closed
yutongzhang-microsoft wants to merge 5 commits into
sonic-net:masterfrom
yutongzhang-microsoft:swss-stats-implementation
Closed

[test] Build with ComponentStats-based SwssStats (PR sonic-swss#4516 + sonic-swss-common#1180)#26924
yutongzhang-microsoft wants to merge 5 commits into
sonic-net:masterfrom
yutongzhang-microsoft:swss-stats-implementation

Conversation

@yutongzhang-microsoft
Copy link
Copy Markdown
Contributor

@yutongzhang-microsoft yutongzhang-microsoft commented Apr 21, 2026

Do not merge — this PR is for CI/VS testing of the ComponentStats refactor while the two source PRs are still open.

Repoints both submodules to forks carrying the refactored library + thin-wrapper:

submodule was now source PR
src/sonic-swss-common c95e7f0 bc9b47c ( eature/component-stats) sonic-swss-common#1180
src/sonic-swss e7be681 f77ef29 (swss-stats-use-componentstats) sonic-swss#4516

.gitmodules URLs are also temporarily switched to https://github.com/yutongzhang-microsoft/... so CI can resolve the non-upstream SHAs.

Why

sonic-swss#4434 introduced SwssStats purely inside orchagent. Per review, the generic mechanism (atomic counters, version-based dirty tracking, deferred-Redis-connect writer thread, cv-based shutdown) has been lifted into sonic-swss-common as swss::ComponentStats so other containers (gnmi, bmp, telemetry…) can reuse the same plumbing by just picking their own metric names.

In sonic-swss, SwssStats is now a ~130-line facade that delegates everything to swss::ComponentStats::create(""SWSS"") and only owns the SET/DEL/COMPLETE/ERROR vocabulary. orch.cpp hooks and the global gSwssStatsRecord flag are unchanged.

How to verify

  1. Build:
    NOSTRETCH=1 NOBULLSEYE=1 make target/docker-orchagent.gz
    make target/sonic-vs.img.gz
    
  2. Boot sonic-vs, then:
    redis-cli -n 2 KEYS ""SWSS_STATS:*""
    redis-cli -n 2 HGETALL ""SWSS_STATS:PORT_TABLE""
    
    Should show SET / DEL / COMPLETE / ERROR counters incrementing as orchagent processes tasks.

Follow-up

Once both source PRs are merged into sonic-net/sonic-swss-common and sonic-net/sonic-swss master, this PR will be rewritten to:

  • revert .gitmodules URLs to sonic-net/...
  • bump submodule SHAs to the merged upstream commits

Update sonic-swss submodule to yutongzhang-microsoft/sonic-swss@bbe820f
(swss-stats-implementation branch) which adds SwssStats for orchagent
profiling (PR sonic-net/sonic-swss#4434).

Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Points src/sonic-swss to yutongzhang-microsoft/sonic-swss@e7be681
(swss-stats-implementation branch).

Changes since bbe820f:
- Add missing includes (<cstdint>, <tuple>, <unordered_map>, <utility>)
- Fix printf format specifier %d -> %u for uint32_t
- Only bump version counter when SET/DEL actually fires (not unknown ops)
- Fix lambda captures in concurrent unit tests
- Consumer::drain() calls recordError() on exception, recordComplete() on success only
- Update stats() helper comment in unit tests

Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Temporarily repoint src/sonic-swss and src/sonic-swss-common to the forks that carry the ComponentStats refactor so CI can build an end-to-end image and exercise the new SwssStats thin wrapper.

- sonic-swss-common -> yutongzhang-microsoft/sonic-swss-common@bc9b47c (feature/component-stats, PR sonic-net/sonic-swss-common#1180)

- sonic-swss -> yutongzhang-microsoft/sonic-swss@f77ef29 (swss-stats-use-componentstats, PR sonic-net/sonic-swss#4516)

This commit will be revised once both PRs merge into their respective upstream repos.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@yutongzhang-microsoft yutongzhang-microsoft changed the title chore: point src/sonic-swss to swss-stats-implementation branch [test] Build with ComponentStats-based SwssStats (PR sonic-swss#4516 + sonic-swss-common#1180) Apr 24, 2026
…nition

p4orch_tests link failed because gSwssStatsRecord was defined in both orch.cpp and swssstats.cpp. Updated sonic-swss to d63a28f which removes the duplicate from orch.cpp; the variable is now defined exactly once in swssstats.cpp.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

- sonic-swss-common: bc9b47c -> 5567352
  Adds writer-thread coverage tests (PR sonic-net#1180 review sonic-net#1) and applies
  review fixes: warn on conflicting create() params, drop unused
  unique_ptr<Counter> indirection, clarify header docs (lock-free scope,
  ASCII-only names, interval=0 clamp, first-call-wins, ref-stability
  invariant), document setEnabled retention behavior.

- sonic-swss: d63a28f -> 3710025
  Applies PR sonic-net#4516 review fixes: replace bare gSwssStatsRecord with
  SwssStats::setEnabled/isEnabled (with internal defense-in-depth checks),
  use SET_COMMAND/DEL_COMMAND for op comparison, document recordTask
  no-retry and recordError per-throw semantics, fix brace style,
  drop redundant size_before>0 check, replace the misleading
  DestructorExitsQuicklyWithLargeInterval test with a real
  DisabledFlagSilencesRecording test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: yutongzhang-microsoft <yutongzhang@microsoft.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@yutongzhang-microsoft
Copy link
Copy Markdown
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@yutongzhang-microsoft
Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

yutongzhang-microsoft added a commit to yutongzhang-microsoft/sonic-buildimage that referenced this pull request May 20, 2026
…+ sonic-swss-common#1180)

Recreated on top of current master to resolve conflicts in sonic-net#26924.

Repoints submodules to forks carrying the ComponentStats refactor:
- src/sonic-swss-common -> yutongzhang-microsoft @ feature/component-stats (55673525d58)
- src/sonic-swss        -> yutongzhang-microsoft @ swss-stats-use-componentstats (37100258229)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants