Add Timestamp Display for FEC Histogram Counters#4513
Open
bobby-nexthop wants to merge 2 commits into
Open
Conversation
Display the time each FEC histogram counter was last updated by reading the timestamp from the COUNTERS DB and formatting it as a human-readable datetime. Includes --relative-timestamp flag and corresponding test updates. Signed-off-by: Bobby McGonigle <bobby@nexthop.ai>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
prgeor
reviewed
May 11, 2026
prgeor
left a comment
prgeor
left a comment
Contributor
There was a problem hiding this comment.
@bobby-nexthop This is so useful enhancement. For completeness do you mind to update the doc
Contributor
|
@bobby-nexthop what about the case for gearbox? @arpit-nexthop @bobby-nexthop |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the show interfaces counters fec-histogram CLI in sonic-utilities by displaying per-bin “Last Updated” timestamps (sourced from COUNTERS_DB) and adding an optional --relative-timestamp view to help operators assess histogram freshness.
Changes:
- Extend FEC histogram output to include a per-bin “Last Updated” timestamp (from
*_TIMESTAMPCOUNTERS_DB fields). - Add
--relative-timestampoption to display human-readable relative age (“X minutes/days ago”). - Update unit tests and mock COUNTERS_DB fixtures to include timestamp fields and validate the new output.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
show/interfaces/__init__.py |
Adds timestamp/relative-time columns and a new CLI flag for FEC histogram display. |
utilities_common/general.py |
Introduces a shared helper to format relative time from epoch-ms timestamps. |
tests/portstat_test.py |
Updates expected fec-histogram output and adds a new test for --relative-timestamp. |
tests/portstat_db/counters_db.json |
Adds an example *_TIMESTAMP counter key for unit test data. |
tests/mock_tables/counters_db.json |
Mirrors the timestamp fixture change for mock table data. |
Comment on lines
+893
to
+897
| if timestamp_ms: | ||
| timestamp_sec = int(timestamp_ms) / 1000.0 | ||
| timestamp_str = datetime.fromtimestamp(timestamp_sec).strftime('%Y-%m-%d %H:%M:%S') | ||
| else: | ||
| timestamp_str = 'N/A' |
Contributor
Author
There was a problem hiding this comment.
The repo in other places always uses local time and not UTC, so will keep this as localtime. However the 0 validation is fair, adding that guard
Comment on lines
+80
to
+83
| str: Relative time string (e.g. "5 minutes ago", "2 days ago"), or "N/A" if in the future | ||
| ''' | ||
| now = datetime.now() | ||
| ts = datetime.fromtimestamp(int(timestamp_ms) / 1000.0) |
Comment on lines
+520
to
+537
| def test_show_intf_counters_fec_histogram_relative_timestamp(self): | ||
| runner = CliRunner() | ||
| result = runner.invoke( | ||
| show.cli.commands["interfaces"].commands["counters"].commands["fec-histogram"], | ||
| ["--relative-timestamp", "Ethernet0"]) | ||
| print(result.exit_code) | ||
| print(result.output) | ||
| assert result.exit_code == 0 | ||
| # Verify the extra column header is present | ||
| assert 'Relative Time' in result.output | ||
| # BIN0 has a timestamp so it should show a relative time (e.g. "days ago") | ||
| assert 'ago' in result.output | ||
| # BIN1 has no timestamp so it should show N/A in the relative column | ||
| lines = result.output.strip().split('\n') | ||
| # Header line, separator line, then 16 data lines | ||
| bin1_line = lines[3] # BIN1 is the second data row | ||
| # BIN1 should have two N/A entries (Last Updated and Relative Time) | ||
| assert bin1_line.count('N/A') == 2 |
Contributor
|
@bobby-nexthop please address copilot comment too |
Signed-off-by: Bobby McGonigle <bobby@nexthop.ai>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
prgeor
reviewed
May 18, 2026
| error_value = asic_db_kvp.get(counter_key, '0') | ||
| timestamp_ms = asic_db_kvp.get(timestamp_key, None) | ||
|
|
||
| if timestamp_ms and int(timestamp_ms) > 0: |
Contributor
There was a problem hiding this comment.
@bobby-nexthop @arpit-nexthop who is updating the timestamp when the BIN value changes? orchagent?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
FEC histogram counters (S0–S15) are polled periodically by FlexCounter, but users have no way to know when a given bin value last changed. Adding per-bin timestamps lets operator determine the freshness of each FEC codeword error bucket.
Display the time each FEC histogram counter was last updated by reading the timestamp from the COUNTERS DB and formatting it as a human-readable datetime. Includes --relative-timestamp flag and corresponding test updates.
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)