Skip to content

[Feature] Remove the legacy Monitor API and non-Prometheus metrics implementation #6923

Description

@warku123

Summary

java-tron maintains two parallel metrics stacks with independent switches: a legacy Dropwizard MetricRegistry stack (org.tron.core.metrics, gated by node.metricsEnable) exposed only through the gRPC MonitorApi.getStatsInfo endpoint and the HTTP GET /monitor/getstatsinfo servlet, and the Prometheus stack (org.tron.common.prometheus, node.metrics.prometheus.enable) that has become the standard monitoring path.

This proposal removes the legacy stack together with MonitorApi, leaving Prometheus as the single supported monitoring backend, in three steps:

  • Step 1 — migrate the one functional consumer: fetch-block peer selection stops reading a Dropwizard histogram and reuses the existing libp2p per-channel latency signal.
  • Step 2 — remove the legacy stack: MetricsUtil, the metric managers, the Monitor endpoints, the node.metricsEnable config chain, and the Dropwizard dependency.
  • Step 3 — close the observability gap: add a tron:node_info{version, chain_id} info metric so the node version survives and operators can tell mainnet from Nile/private networks at a glance.

This is the implementation issue for item 7 of the tracking issue #6921.

Problem

Motivation

Prometheus has become the standard monitoring solution for java-tron (see #6590 and the tron-docker metric_monitor reference stack). Maintaining a second, non-Prometheus implementation alongside it duplicates instrumentation effort and misleads operators: enabling node.metricsEnable without Prometheus produces no scrapeable output at all, and the two switches are independent.

Current State

  • MetricsUtil wraps a Dropwizard MetricRegistry; ~10 production classes write into it (HTTP interceptor, rate limiter, P2P stats, peer connection, block manager, etc.). Read-back goes through NodeMetricManager / BlockChainMetricManager / NetMetricManagerMetricsApiService → the two Monitor endpoints above.
  • Most legacy fields have Prometheus twins (tron:header_height, tron:block_fork, tron:peers, tron:tcp_bytes, tron:miner_latency_seconds, ...), but a few exist only in the legacy payload: node.version, node.ip, per-endpoint QPS/failQPS, and per-witness latency detail.
  • One hidden functional dependency: FetchBlockService.getPeerTop75() reads the per-peer histogram net.latency.fetch.block.<peerIP> to pick the fastest idle peer for block fetch:
// BlockMsgHandler: write side — one histogram per peer IP, never removed on disconnect,
// and histogramUpdateUnCheck records even when node.metricsEnable=false
MetricsUtil.histogramUpdateUnCheck(NET_LATENCY_FETCH_BLOCK + peer.getInetAddress(), now - time);

// FetchBlockService: read side — 75th percentile drives peer selection
double latency = MetricsUtil.getHistogram(NET_LATENCY_FETCH_BLOCK + peer.getInetAddress())
    .getSnapshot().get75thPercentile();

This makes the metric state part of block-sync behavior, not just observability, and the per-IP key family grows unboundedly over node lifetime.

Limitations or Risks

  • Removing the legacy registry without migrating the FetchBlockService read breaks fetch-block peer selection.
  • Old configs with node.metricsEnable=true silently lose monitoring after upgrade: the unknown key is ignored by config parsing (no error), while Prometheus stays disabled by default.

Proposed Solution

Proposed Design

Step 1 — migrate the functional consumer. FetchBlockService switches from the Dropwizard P75 histogram to Channel.getAvgLatency() — the libp2p per-connection RTT average already maintained for every peer and already used by PeerManager.sortPeers. No new state structure is introduced, and the unbounded per-IP histogram family disappears with it. The write side in BlockMsgHandler keeps only its Prometheus twin (tron:block_fetch_latency_seconds), so global fetch-latency observability is preserved.

Step 2 — remove the legacy stack. Delete MetricsUtil, MetricsKey, MetricsApiService, the MetricsInfo DTOs, the three metric managers, the gRPC MonitorApi registration, the /monitor/getstatsinfo servlet route, all legacy write sites, the node.metricsEnable config chain (reference.confNodeConfigCommonParameterArgs), and the io.dropwizard.metrics:metrics-core dependency. MetricsService.applyBlock keeps its class and method name but becomes Prometheus-only (duplicate-witness, miner-latency, transaction counters). /monitor/getnodeinfo is node info, not metrics, and is unaffected.

Step 3 — close the observability gap. Register an info metric at startup:

tron:node_info{version="4.8.x", chain_id="<genesis block hash>"} 1
  • version preserves the legacy node.version field.
  • chain_id is the genesis block hash (the canonical TRON chain identifier, already computed at Manager.initGenesis() and readable via Args.getInstance().getChainId() after Spring context refresh). Mainnet, Nile and private networks each have distinct values, so dashboards can distinguish networks directly.
  • Node IP is intentionally not exported — the scrape target's instance label already identifies the node.

Key Changes

  • Module: common (prometheus info metric), framework (removal + migration), build (drop the Dropwizard dependency).
  • Configuration: remove node.metricsEnable; node.metrics.prometheus.enable / .port keep their semantics.
  • API: gRPC Monitor.GetStatsInfo and HTTP GET /monitor/getstatsinfo stop being served; protobuf definitions are kept for source compatibility (see Compatibility). New Prometheus metric tron:node_info{version, chain_id}.

Impact

  • Developer Experience: one metrics stack to instrument and review; the Dropwizard registry, its key conventions, and its read-back managers disappear.
  • Stability: fetch-block peer selection moves from a per-tick Dropwizard snapshot (O(N log N) per peer, with allocation and reservoir locking) to an O(1) field read; the unbounded per-IP metric family is gone.
  • Security: the gRPC/HTTP Monitor attack surface is removed; the Prometheus exporter path is untouched.
  • Performance: metrics-disabled nodes no longer pay for the always-on histogramUpdateUnCheck writes.

Compatibility

  • Breaking Change: Yes — gRPC Monitor.GetStatsInfo and HTTP GET /monitor/getstatsinfo stop being served. The protobuf definitions (service Monitor in api.proto, message MetricsInfo in Tron.proto) are kept for source compatibility; clients calling the endpoint will receive UNIMPLEMENTED / 404. Per principle 2 of Tracking: code refactor and cleanup #6921, public APIs are normally deprecated for one release cycle first; this proposal removes the server-side implementation directly, as item 7 of Tracking: code refactor and cleanup #6921 describes ("remove ... together with MonitorApi") and because the endpoint has no known production consumers. If maintainers prefer a deprecation window, the endpoint can instead be kept registered and marked deprecated for one release.
  • Default Behavior Change: Yes — fetch-block peer selection uses the connection lifetime RTT average instead of the historical per-peer block-fetch P75. Two known differences: (1) avgLatency == 0 means "no RTT sample yet" — on the current peer it delays failover until the timeout, and on a candidate peer it is treated as "fastest" (both semantics are pinned by unit tests); (2) RTT does not measure block-serving throughput. Fetch-block is a head+1 fallback path, so impact is bounded; we will compare block-fetch tail latencies on Nile before/after.
  • Migration Required: Yes — operators who used the legacy stack must switch flags:
Before After
node.metricsEnable = true node.metrics.prometheus.enable = true
(legacy port n/a) node.metrics.prometheus.port = 9527 (default)

The removed key is ignored silently by config parsing; the release notes must call out this migration to avoid silently losing monitoring.

Test Plan

  • New unit tests: FetchBlockServiceTest (lowest-latency selection, timeout fast-switch, and both avgLatency=0 cold-start cases); PrometheusApiServiceTest extensions (tron:node_info{version, chain_id} sample, unknown-key guard, duplicate-witness and transaction counters).
  • Repo CI: build matrix, checkstyle, CodeQL, single-node integration, and the changed-line coverage gate (> 60%).
  • Nile observation of fetch-block behavior after the peer-selection migration.

Rollback

Revert the PR commits. The protobuf definitions are untouched, so restoring the server-side endpoints is a plain revert with no protocol or config migration needed.

References

Additional Notes

  • Do you have ideas regarding implementation? Yes — the implementation is ready (logical commits following the steps in "Proposed Solution", CI green including the changed-line coverage gate); the pull request will reference this issue.
  • Are you willing to implement this feature? Yes

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions