|WARNING| metrics: remove experimental telemetry helper API

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/170706

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-07

# DPDK Patch Review

## Summary
This patch removes experimental telemetry helper API from the metrics library. The functions were never used outside the library itself and are being internalized. Overall the patch is well-structured, but there are a few issues to address.

---

## Errors

### 1. Missing global stats function implementation
The `rte_metrics_tel_get_global_stats()` function is removed from the header and the `#else` stub branch, but no static implementation is provided in the main source file. This function was exported and the release notes mention it is being removed, but there's no corresponding static version in the `#ifdef RTE_HAS_JANSSON` section.

**Fix:** Either add the static implementation, or if it's truly unused and should be deleted entirely, verify that none of the legacy telemetry command handlers (`handle_global_stats`, `handle_ports_all_stats_values`, `handle_ports_stats_values_by_name`) depend on it. If they do, this is a functional break.

---

## Warnings

### 1. Release notes missing explicit date
The release notes update mentions removing experimental API but should explicitly note this is a breaking change for any out-of-tree code that may have used these functions, even though they had no in-tree users.

**Suggestion:** Consider adding a note like: "Out-of-tree code using these functions must be updated; no in-tree users existed."

### 2. Inconsistent jansson handling in header
The deleted header had this pattern:
```c
#ifdef RTE_HAS_JANSSON
#include <jansson.h>
#else
#define json_t void *
#endif
```

The source file now unconditionally includes `<jansson.h>` at the top, which is correct since the file is only built when jansson is present (per the meson change). However, verify that no build configurations could violate this assumption.

---

## Info

### 1. Structure visibility change
Moving `struct telemetry_metrics_data` from the header to the source file and making `tel_met_data` static is good encapsulation. The structure is now properly internal to the implementation.

### 2. Meson build logic is clear
The comment explains that the telemetry file is only built when jansson is available, which matches the new source structure. This is a clean improvement.

### 3. Release notes are thorough
The release notes clearly list all removed functions and note that the legacy commands remain functional. Good documentation of the change.

---

## Correctness Review

- **Resource management:** No resource leaks introduced; the patch only removes exports and deletes stubs
- **Error paths:** Existing error handling unchanged
- **API consistency:** Functions that remain are now static, consistent with their actual usage
- **Build system:** Meson changes correctly gate the telemetry source on jansson availability

---

## Before Merging

1. **Verify `rte_metrics_tel_get_global_stats()` status:** Confirm whether this function needs a static implementation or if all its call sites have been removed
2. **Test legacy telemetry commands:** Ensure `ports_all_stat_values`, `global_stat_values`, and `ports_stats_values_by_name` still work correctly after this change
3. **Verify no external users:** Confirm with the community that no out-of-tree code depends on these experimental APIs (though being experimental, this is not strictly required)