|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 functions from the metrics library that have no users. The changes are mostly clean, but there are a few issues to address.

---

## Errors

**1. Missing function implementation**

The release notes mention `rte_metrics_tel_get_global_stats` as a removed function, but this function is not present in the remaining code after the `#ifdef RTE_HAS_JANSSON` section is removed. The patch deletes the stub implementation at the end but never shows the actual implementation being removed. Either the function was already missing, or the patch is incomplete.

**Verification needed**: Confirm `rte_metrics_tel_get_global_stats` existed in the original code and was intentionally removed, or remove it from the release notes if it was already gone.

---

## Warnings

**1. Inconsistent header removal in meson.build**

```diff
-headers = files('rte_metrics.h', 'rte_metrics_telemetry.h')
+headers = files('rte_metrics.h')
```

The `rte_metrics_telemetry.h` header is removed from the `headers` list, which is correct. However, the comment about conditional compilation is added *before* the header removal. Consider reordering for clarity:

```python
sources = files('rte_metrics.c')
headers = files('rte_metrics.h')

# Legacy JSON telemetry commands are only available with jansson.
if dpdk_conf.has('RTE_HAS_JANSSON')
    sources += files('rte_metrics_telemetry.c')
    ext_deps += jansson_dep
endif
```

This is more logical since the header is now unconditional while the telemetry source is conditional.

**2. Release notes could be more concise**

The release notes entry is verbose. The list of removed function names could be formatted more cleanly:

```rst
* metrics: Removed the experimental telemetry helper API.

  Removed ``rte_metrics_telemetry.h`` and the following experimental functions:
  ``rte_metrics_tel_reg_all_ethdev``,
  ``rte_metrics_tel_encode_json_format``,
  ``rte_metrics_tel_get_global_stats``,
  ``rte_metrics_tel_get_port_stats_ids``,
  ``rte_metrics_tel_get_ports_stats_json``,
  and ``rte_metrics_tel_extract_data``.
  These had no users and exposed jansson types in the public API.
  The legacy telemetry commands remain unchanged.
```

---

## Info

**1. Good cleanup**

The removal of dead code and unexported experimental API is appropriate. The functions were experimental since 20.05 (6 releases ago at 26.11), had no users, and exposed third-party library types (jansson) in the public header, which is poor API design.

**2. Static function consolidation is correct**

Making the functions static and moving types into the `.c` file is the right approach since they're only used internally now.

**3. Build structure change is sound**

Moving `rte_metrics_telemetry.c` into the conditional jansson block makes sense since the file now only contains jansson-dependent code. The main `rte_metrics.c` and header remain unconditional.

---

## Final Recommendation

The patch is mostly correct. Address the missing `rte_metrics_tel_get_global_stats` implementation (Error #1) before merging. The warnings are minor style/clarity suggestions that can be addressed at the maintainer's discretion.