[TAM]: Add telemetry stats APIs and maximal polling interval for stream telemetry

[Pterosaur]

Description

Enhance stream telemetry (Phase 2) with the following changes:

1. TAM telemetry stats APIs (inc/saitam.h)

Add stats APIs for TAM telemetry objects for debugging purposes:

• sai_get_tam_telemetry_stats_fn — get telemetry stats (deprecated, for backward compatibility)
• sai_get_tam_telemetry_stats_ext_fn — get telemetry stats extended
• sai_clear_tam_telemetry_stats_fn — clear telemetry stats

New stat counters (sai_tam_telemetry_stat_t):

• SAI_TAM_TELEMETRY_STAT_INGESTED_RECORDS — total records ingested
• SAI_TAM_TELEMETRY_STAT_PENDING_READ_RECORDS — records pending processing (gauge)
• SAI_TAM_TELEMETRY_STAT_CONSUMED_RECORDS — total records consumed
• SAI_TAM_TELEMETRY_STAT_DROPPED_RECORDS — total records dropped

2. Maximal polling interval (inc/saitypes.h)

Add maximal_polling_interval field to sai_stat_st_capability_t. The collector handles single-cycle counter rollovers, but vendors must ensure data does not roll over twice between two collection intervals. This field allows vendors to specify the upper bound.

Note: Adding a new member to sai_stat_st_capability_t is an ABI-breaking change that causes metadata CI to fail. See PR comments for discussion and proposed workaround.

[kcudnik]

it will not build, you are breaking backward compatybility with structure item shift

[Pterosaur]

it will not build, you are breaking backward compatybility with structure item shift

Hi @kcudnik , long time no talk. Glad to have your eyes on this.

I’m aware that this PR breaks backward compatibility. The root cause is that the original design didn’t provide APIs for querying SAI statistics of TAM objects.

I attempted to add the new APIs at the end of _sai_tam_api_t to preserve backward compatibility. However, the PR checker rejects it because it requires functions of the same type to be placed consecutively.

Do you have any suggestion?

[kcudnik]

for apis you can add then at the end of api struct, but for extended struct you would need to break compatybility and add exception

[Pterosaur]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Pterosaur]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Pterosaur]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Pterosaur]

@tjchadaga This PR adds a new field maximal_polling_interval to sai_stat_st_capability_t, which changes the struct member count from 2 to 3. This causes the metadata CI checks (meta/structs.pl struct history validation and meta/test.pm size assertion) to fail.

We could work around this by adding sai_stat_st_capability_t to the extensible struct allowlist in both files. Here's the patch that would make CI pass:

meta/structs.pl

--- a/meta/structs.pl
+++ b/meta/structs.pl
@@ -164,18 +164,27 @@ sub BuildCommitHistory
         # of union may not increase by adding members, and actual union size
         # check is performed by sai sanity check
 
+        # Structs that are allowed to append new members at the end (ABI extension).
+        # api_t structs are always allowed. Add other structs here as needed.
+        my @extensible_structs = (
+            "sai_switch_health_data_t",
+            "sai_port_oper_status_notification_t",
+            "sai_stat_st_capability_t",
+        );
+
+        my %extensible = map { $_ => 1 } @extensible_structs;
+
         if ($currCount != $histCount and not $structTypeName =~ /^sai_\w+_api_t$/
-                and $structTypeName ne "sai_switch_health_data_t"
-                and $structTypeName ne "sai_port_oper_status_notification_t")
+                and not $extensible{$structTypeName})
         {
             LogError "FATAL: struct $structTypeName member count differs, was $histCount but is $currCount on commit $commit" if $type eq "struct";
         }
 
         if ($histCount > $currCount)
         {
-            if ($structTypeName eq "sai_port_oper_status_notification_t")
+            if ($extensible{$structTypeName})
             {
-                # we allow this to change back backward compatibility
+                # we allow extensible structs to change for backward compatibility
             }
             else
             {

meta/test.pm

--- a/meta/test.pm
+++ b/meta/test.pm
@@ -646,7 +646,14 @@ sub CreateStructUnionSizeCheckTest
             $STRUCTS{$name} = $name;
 
             next if $name =~ /^sai_\w+_api_t$/; # skip api structs
-            next if $name eq "sai_switch_health_data_t";
+
+            # Skip extensible structs that are allowed to grow (see also structs.pl)
+            my %extensible_structs = map { $_ => 1 } (
+                "sai_switch_health_data_t",
+                "sai_port_oper_status_notification_t",
+                "sai_stat_st_capability_t",
+            );
+            next if $extensible_structs{$name};
 
             my $upname = uc($name);

However, I don't think sai_stat_st_capability_t warrants a special exception in the allowlist — it's a straightforward data struct, not something that needs an extensibility mechanism. A force merge with the ABI break seems more appropriate. Could you advise on the preferred approach?

[Pterosaur]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Pterosaur]

@tjchadaga I've also created a draft PR #2260 that includes the same functional changes plus the meta/structs.pl and meta/test.pm modifications to make CI pass. If you think adding sai_stat_st_capability_t to the extensible struct allowlist is acceptable, we can apply those changes from the draft PR. Otherwise, a force merge on this PR would be the alternative.

[kcudnik]

just to inform, that breaking compatibility can cause some errors or read uninitialized memory, potentially a crash, if in notification struct is differetnt between what syncd expects and what library support, especially if there is a array of structs the alignement will fail, fields will shift, and garbage data will be read