Integrate SHOW API probe into telemetry watchdog by lixiaoyuner · Pull Request #24053 · sonic-net/sonic-buildimage

lixiaoyuner · 2025-09-19T07:20:02Z

Why I did it

Integrate SHOW API probe into telemetry watchdog. We want the watchdog to catch the bad SHOW command implementation.

Work item tracking

Microsoft ADO (number only): 34793098

How I did it

Add gnmi_get into telemetry-watchdog, and call gnmi_get in the watchdog http service for each probe, as long as one xpath fails, the probe will return 500.

The watchdog will read xpath list from /cmd_list.json and environment variables.

How to verify it

Run the telemetry-watchdog container, call curl http://127.0.0.1:50080/ to check if the output is expected or not.

root@bjw-can-2700-2:/# curl -s http://127.0.0.1:50080/ |jq .
{
  "check_telemetry_port": "OK",
  "xpath_commands": [
    {
      "xpath": "clock",
      "success": true,
      "error": null,
      "duration_ms": 200
    }
  ]
}

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

mssonicbld · 2025-09-19T07:20:10Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-09-19T07:20:22Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-09-19T10:04:13Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-09-19T10:04:27Z

Azure Pipelines successfully started running 1 pipeline(s).

make1980 · 2025-09-19T19:24:05Z

dockers/docker-telemetry-watchdog/Dockerfile.j2

@@ -1,5 +1,5 @@
 {% from "dockers/dockerfile-macros.j2" import install_debian_packages, install_python_wheels, copy_files %}
-ARG BASE=docker-config-engine-bookworm-{{DOCKER_USERNAME}}:{{DOCKER_USERTAG}}
+ARG BASE=docker-sonic-gnmi-{{DOCKER_USERNAME}}:{{DOCKER_USERTAG}}


nit:

Integrate SHOW API probe into telemetry watchdog. We want the watch dog can catch the bad SHOW command implement.

->

Integrate SHOW API probe into telemetry watchdog. We want the watchdog to catch the bad SHOW command implementation.

nit:

Integrate SHOW API probe into telemetry watchdog. We want the watch dog can catch the bad SHOW command implement.

->

Integrate SHOW API probe into telemetry watchdog. We want the watchdog to catch the bad SHOW command implementation.

Thanks, fixed.

FengPan-Frank · 2025-09-19T23:34:45Z

dockers/docker-telemetry-watchdog/Dockerfile.j2

@@ -1,5 +1,5 @@
 {% from "dockers/dockerfile-macros.j2" import install_debian_packages, install_python_wheels, copy_files %}
-ARG BASE=docker-config-engine-bookworm-{{DOCKER_USERNAME}}:{{DOCKER_USERTAG}}
+ARG BASE=docker-sonic-gnmi-{{DOCKER_USERNAME}}:{{DOCKER_USERTAG}}


better to add DEPENDS in rule to make sure GNMI is built first for this.

better to add DEPENDS in rule to make sure GNMI is built first for this.

Thanks, fixed

FengPan-Frank · 2025-09-19T23:38:57Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+                Err(e) => eprintln!("Failed to parse {}: {}", CMD_LIST_JSON, e),
+            }
+        },
+    Err(e) => eprintln!("Could not read {}: {} (will continue with env var only)", CMD_LIST_JSON, e),


should this kind of error be visible to user as well? return in json resp

should this kind of error be visible to user as well? return in json resp

Put the error into JSON resp

make1980 · 2025-09-21T00:31:11Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+
+    // dedupe while preserving order
+    let mut seen = std::collections::HashSet::new();
+    list.retain(|x| seen.insert(x.clone()));


why don't insert to a hashset above directly? #Closed

why don't insert to a hashset above directly?

Yes, it's better to use a HashSet directly, updated the code.

make1980 · 2025-09-21T00:31:42Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+        if !blk.is_empty() {
+            list.retain(|x| !blk.contains(x));
+        }
+    }


do we really need this? #Closed

do we really need this?

I'd like to keep this, we may encounter a case that need to ignore some API, we can update the environment variables via k8s yaml file which will be very quick, no need to wait for code change.

make1980 · 2025-09-21T00:32:01Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+}
+
+fn run_gnmi_for_xpath(xpath: &str, port: u16, sec: &TelemetrySecurityConfig, target_name: &str, timeout: Duration) -> CommandResult {
+    // Build full command: gnmi_get -xpath_target SHOW -xpath <xpath> -target_addr 127.0.0.1:<port> -logtostderr -insecure true


longer than 120 characters #Closed

longer than 120 characters

Thanks, fixed all the lines which are too long.

make1980 · 2025-09-21T00:49:57Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+        Ok(c) => c,
+        Err(e) => {
+            eprintln!("Redis client error (security): {e}");
+            return TelemetrySecurityConfig { use_client_auth: false, ca_crt: DEFAULT_CA_CRT.to_string(), server_crt: DEFAULT_SERVER_CRT.to_string(), server_key: DEFAULT_SERVER_KEY.to_string() };


TelemetrySecurityConfig { use_client_auth: false, ca_crt: DEFAULT_CA_CRT.to_string(), server_crt: DEFAULT_SERVER_CRT.to_string(), server_key: DEFAULT_SERVER_KEY.to_string() };

nit: we should just construct this once and use it in all the failure cases #Closed

Thanks, fixed.

TelemetrySecurityConfig { use_client_auth: false, ca_crt: DEFAULT_CA_CRT.to_string(), server_crt: DEFAULT_SERVER_CRT.to_string(), server_key: DEFAULT_SERVER_KEY.to_string() };

nit: we should just construct this once and use it in all the failure cases

make1980 · 2025-09-21T00:52:52Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+    let mut get_field = |hash: &str, field: &str| -> Option<String> {
+        let r: redis::RedisResult<Option<String>> = conn.hget(hash, field);
+        match r { Ok(v) => v, Err(e) => { eprintln!("Redis HGET error {hash}.{field}: {e}"); None } }
+    };


nit: this pattern has appeared in this file multiple times - can we refactor it to a common function? #Closed

nit: this pattern has appeared in this file multiple times - can we refactor it to a common function?

Thanks, refactored.

make1980 · 2025-09-21T00:55:46Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+                                if !res.success { http_status = "HTTP/1.1 500 Internal Server Error"; }
+                                cmd_results.push(res);
+                            }
+                        }


could you introduce another env var to enable/disable this? #Closed

Added a SHOW API probe switch and enable probe as default.

make1980

🕐

mssonicbld · 2025-09-22T11:47:10Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-09-22T11:47:24Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-09-22T12:07:11Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-09-22T12:07:24Z

Azure Pipelines successfully started running 1 pipeline(s).

zbud-msft · 2025-09-22T17:52:14Z

dockers/docker-telemetry-watchdog/watchdog/src/cmd_list.json

@@ -0,0 +1,60 @@
+{
+    "xpaths": [


Issue with this is that

We arent testing options with the xpath in this case, user can pass in options for example --interface.

There are required arguments for some commands that need to be passed in, and these are usually like interface name. I dont know if the best way would to be hardcode an interface to check for all devices like Ethernet0, or if we can grab an oper up/admin up one from the host?

Issue with this is that

We arent testing options with the xpath in this case, user can pass in options for example --interface.

There are required arguments for some commands that need to be passed in, and these are usually like interface name. I dont know if the best way would to be hardcode an interface to check for all devices like Ethernet0, or if we can grab an oper up/admin up one from the host?

You are right, the commands with parameters are complicated, we don't plan to support for now, we support these common xpath at this stage. We can add more xpath with specific fixed parameters as you mentioned if we need later.

make1980 · 2025-09-22T21:43:09Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+        }
+    } else {
+        if let Err(e) = fs::read_to_string(CMD_LIST_JSON) {
+            let msg = format!(


why do we need to read the file again?

why do we need to read the file again?

That's indeed a bad implementation, fixed.

make1980 · 2025-09-22T21:46:34Z

dockers/docker-telemetry-watchdog/watchdog/src/main.rs

+                }
+            } else {
+                // Possibly large stderr; truncate if huge (optional threshold 16KB).
+                let mut truncated = if stderr_string.len() > 16 * 1024 {


16 * 1024

please define this as a constant

16 * 1024

please define this as a constant

Fixed.

make1980

mssonicbld · 2025-09-23T01:20:07Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-09-23T01:20:18Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-10-09T15:24:05Z

Cherry-pick PR to msft-202412: Azure/sonic-buildimage-msft#1709

…1708)  #### Why I did it Manual cherrypick for sonic-net/sonic-buildimage#24053 due to conflict. Integrate SHOW API probe into telemetry watchdog. We want the watchdog to catch the bad SHOW command implementation. ##### Work item tracking - Microsoft ADO **(number only)**:34793098 #### How I did it Add gnmi_get into telemetry-watchdog, and call gnmi_get in the watchdog http service for each probe, as long as one xpath fails, the probe will return 500. The watchdog will read xpath list from /cmd_list.json and environment variables. #### How to verify it Run the telemetry-watchdog container, call curl http://127.0.0.1:50080/ to check if the output is expected or not. root@bjw-can-2700-2:/# curl -s http://127.0.0.1:50080/ |jq . { "check_telemetry_port": "OK", "xpath_commands": [ { "xpath": "clock", "success": true, "error": null, "duration_ms": 200 } ] }  #### Which release branch to backport (provide reason below if selected)  - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 - [ ] 202111 - [ ] 202205 - [ ] 202211 #### Tested branch (Please provide the tested image version)  - [ ]  - [ ]  #### Description for the changelog   #### Link to config_db schema for YANG module changes  #### A picture of a cute animal (not mandatory but encouraged)

r12f · 2025-10-17T18:34:25Z

hi @lixiaoyuner , do you mind to help with a manual pick to 202412? the auto pick detected some conflicts on the code:

make1980 · 2025-10-17T19:16:43Z

@feng ***@***.***> - could you help with this? Thanks, Michael From: Riff ***@***.***> Sent: Friday, October 17, 2025 11:35 AM To: sonic-net/sonic-buildimage ***@***.***> Cc: Michael Ma ***@***.***>; Comment ***@***.***> Subject: Re: [sonic-net/sonic-buildimage] Integrate SHOW API probe into telemetry watchdog (PR #24053) [https://avatars.githubusercontent.com/u/1533278?s=20&v=4]r12f left a comment (sonic-net/sonic-buildimage#24053)<#24053 (comment)> hi @lixiaoyuner<https://github.com/lixiaoyuner> , do you mind to help with a manual pick to 202412? the auto pick detected some conflicts on the code: image.png (view on web)<https://github.com/user-attachments/assets/41e831ef-99bc-4ee5-b6c2-3a2136a0468a> - Reply to this email directly, view it on GitHub<#24053 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AZ62PEYR2PV3E6Q57GIBXBL3YEZETAVCNFSM6AAAAACG6B2DL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMJWGY3DGNBRG4>. You are receiving this because you commented.Message ID: ***@***.******@***.***>>

FengPan-Frank · 2025-10-18T03:37:04Z

@r12f @make1980 For this PR, I have already cherrypicked it into 202412 manually [202412] [PR:24053] Integrate SHOW API probe into telemetry watchdog by FengPan-Frank · Azure/sonic-buildimage-msft<Azure/sonic-buildimage-msft#1708> Thanks, Feng

…

________________________________ From: Michael Ma ***@***.***> Sent: Saturday, October 18, 2025 6:17 AM To: sonic-net/sonic-buildimage ***@***.***> Cc: Feng Pan ***@***.***>; Comment ***@***.***> Subject: Re: [sonic-net/sonic-buildimage] Integrate SHOW API probe into telemetry watchdog (PR #24053) [https://avatars.githubusercontent.com/u/108898195?s=20&v=4]make1980 left a comment (sonic-net/sonic-buildimage#24053)<#24053 (comment)> @feng ***@***.***> - could you help with this? Thanks, Michael From: Riff ***@***.***> Sent: Friday, October 17, 2025 11:35 AM To: sonic-net/sonic-buildimage ***@***.***> Cc: Michael Ma ***@***.***>; Comment ***@***.***> Subject: Re: [sonic-net/sonic-buildimage] Integrate SHOW API probe into telemetry watchdog (PR #24053) [https://avatars.githubusercontent.com/u/1533278?s=20&v=4]r12f left a comment (sonic-net/sonic-buildimage#24053)<#24053 (comment)> hi @lixiaoyuner<https://github.com/lixiaoyuner> , do you mind to help with a manual pick to 202412? the auto pick detected some conflicts on the code: image.png (view on web)<https://github.com/user-attachments/assets/41e831ef-99bc-4ee5-b6c2-3a2136a0468a> - Reply to this email directly, view it on GitHub<#24053 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AZ62PEYR2PV3E6Q57GIBXBL3YEZETAVCNFSM6AAAAACG6B2DL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMJWGY3DGNBRG4>. You are receiving this because you commented.Message ID: ***@***.******@***.***>> — Reply to this email directly, view it on GitHub<#24053 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AXEWCYEK7UJDZ2OKX3VM2NL3YE6DHAVCNFSM6AAAAACG6B2DL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMJWG44TCNZZHE>. You are receiving this because you commented.Message ID: ***@***.***>

r12f · 2025-10-19T04:43:26Z

thanks! updated tag.

Why I did it Integrate SHOW API probe into telemetry watchdog. We want the watchdog to catch the bad SHOW command implementation. Work item tracking Microsoft ADO (number only): 34793098 How I did it Add gnmi_get into telemetry-watchdog, and call gnmi_get in the watchdog http service for each probe, as long as one xpath fails, the probe will return 500. The watchdog will read xpath list from /cmd_list.json and environment variables. How to verify it Run the telemetry-watchdog container, call curl http://127.0.0.1:50080/ to check if the output is expected or not. root@bjw-can-2700-2:/# curl -s http://127.0.0.1:50080/ |jq . { "check_telemetry_port": "OK", "xpath_commands": [ { "xpath": "clock", "success": true, "error": null, "duration_ms": 200 } ] } Signed-off-by: Feng Pan <fenpan@microsoft.com>

Integrate SHOW API probe into telemetry watchdog

0646dd7

lixiaoyuner requested a review from lguohan as a code owner September 19, 2025 07:20

Build telemetry-watchdog after gnmi

6d9a9e2

lixiaoyuner requested review from qiluo-msft and xumia as code owners September 19, 2025 10:04

make1980 reviewed Sep 19, 2025

View reviewed changes

FengPan-Frank reviewed Sep 19, 2025

View reviewed changes

make1980 reviewed Sep 21, 2025

View reviewed changes

make1980 requested changes Sep 21, 2025

View reviewed changes

Fix comments

3430b63

Fix the line which is too long

c567150

zbud-msft reviewed Sep 22, 2025

View reviewed changes

make1980 reviewed Sep 22, 2025

View reviewed changes

make1980 approved these changes Sep 22, 2025

View reviewed changes

Fix the build error and two more comments

341bb60

FengPan-Frank approved these changes Sep 23, 2025

View reviewed changes

qiluo-msft approved these changes Sep 25, 2025

View reviewed changes

qiluo-msft merged commit cedc11d into sonic-net:master Sep 25, 2025
20 checks passed

FengPan-Frank added the Request for msft-202412 Branch label Oct 2, 2025

mssonicbld added the Cherry Pick Conflict_msft-202412 label Oct 2, 2025

FengPan-Frank mentioned this pull request Oct 9, 2025

[202412] [PR:24053] Integrate SHOW API probe into telemetry watchdog Azure/sonic-buildimage-msft#1708

Merged

10 tasks

r12f added the Approved for msft-202412 Branch label Oct 9, 2025

mssonicbld removed the Cherry Pick Conflict_msft-202412 label Oct 9, 2025

mssonicbld mentioned this pull request Oct 9, 2025

[action] [PR:24053] Integrate SHOW API probe into telemetry watchdog Azure/sonic-buildimage-msft#1709

Closed

9 tasks

mssonicbld added the Created PR to msft-202412 Branch label Oct 9, 2025

r12f added Included in msft-202412 Branch and removed Created PR to msft-202412 Branch labels Oct 19, 2025

Conversation

lixiaoyuner commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I did it

Work item tracking

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Uh oh!

mssonicbld commented Sep 19, 2025

Uh oh!

azure-pipelines bot commented Sep 19, 2025

Uh oh!

mssonicbld commented Sep 19, 2025

Uh oh!

azure-pipelines bot commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

make1980 Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

make1980 Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

make1980 Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

make1980 Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

make1980 Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

make1980 Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

make1980 left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Sep 22, 2025

Uh oh!

azure-pipelines bot commented Sep 22, 2025

Uh oh!

mssonicbld commented Sep 22, 2025

Uh oh!

azure-pipelines bot commented Sep 22, 2025

Uh oh!

lixiaoyuner commented Sep 19, 2025 •

edited

Loading

make1980 Sep 21, 2025 •

edited

Loading

make1980 Sep 21, 2025 •

edited

Loading

make1980 Sep 21, 2025 •

edited

Loading

make1980 Sep 21, 2025 •

edited

Loading

make1980 Sep 21, 2025 •

edited

Loading

make1980 Sep 21, 2025 •

edited

Loading

FengPan-Frank commented Oct 18, 2025 via email •

edited

Loading