Skip to content

[improve][broker] Port the scalable topic metrics to the Prometheus Java client 1.x - #27482

Draft
namest504 wants to merge 7 commits into
apache:masterfrom
namest504:improve/broker-scalable-prometheus-1x
Draft

namest504 wants to merge 7 commits into
apache:masterfrom
namest504:improve/broker-scalable-prometheus-1x

Conversation

@namest504

Copy link
Copy Markdown
Contributor

Main Issue: #27045

Draft until #27350 is merged. That PR makes the broker /metrics read the Prometheus Java client 1.x default registry, and the parity test in this PR needs it. The branch carries the #27350 commits until then. This PR does not depend on the other pulsar-broker port PRs; they can merge in any order.

Motivation

Part of the pulsar-broker item in #27045: the scalable topic metrics in ScalableTopicMetrics, ScalableTopicControllerMetrics, ScalableTopicConsumerMetrics and SegmentOwnerMetrics.

Modifications

These four classes were custom simpleclient collectors. Each broker has its own instance, but brokers in one JVM that share a registry share one collector, which sums their values per cluster and keeps the totals of brokers that already closed. That design is kept; the collectors are now 1.x MultiCollectors on a PrometheusRegistry. The first instance on a registry registers the collector, later instances join it, and close() unregisters it when the last instance leaves.

  • ScalableTopicMetrics keeps an internal, unregistered 1.x histogram for request durations and rebuilds its snapshot with the fixed bucket bounds. That histogram is built from an isolated configuration, so runtime bucket overrides from prometheus.properties or system properties cannot reach it; with the default builder such an override would make every scrape fail.
  • The other three classes keep their own counters and gauges; only the snapshot code changed.
  • Tests: the scalable test classes read the registry through a small PrometheusSamples helper that gives the samples the same shape the exposition has. The "unregistered after the last close" checks now look at what is registered, not at samples, so they fail if the unregister call is removed.
  • pulsar-broker adds prometheus-metrics-core (already shipped).

Metric and exposition format changes (for the 5.1 release notes)

Names, labels and values are unchanged. The text format differs the same way as in #27350: counter # TYPE lines carry _total, _created series are gone, summary and histogram counts are integers, labels are sorted. Histogram buckets are still cumulative with a +Inf bucket.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • PrometheusMetricsTest.testScalableTopicMetricsAreScraped: one lookup request, then the broker /metrics shows the request duration histogram with the 12 fixed buckets and the gauges and counters of the four classes, all with cluster.
  • ScalableTopicMetricsTest.testRuntimeBucketOverridesDoNotReachTheRequestHistogram: with a bucket override for the histogram name, the scrape still shows the 12 fixed buckets. It fails with IllegalArgumentException when the histogram uses the default builder.
  • The parity test only looks at cluster="test" series and checks the request count as a delta, because other test classes in the same JVM leave collectors for other cluster names on the shared default registry.
  • All 542 tests under org.apache.pulsar.broker.service.scalable pass, plus ScalableTopicRequestMetricsTest, ScalableTopicRequestWriteMetricsTest, PulsarServiceTest and V5SegmentLoadReporterTest.
  • Personal CI with [improve][monitor] Expose the Prometheus Java client 1.x default registry in broker and proxy system metrics #27350 underneath: https://github.com/namest504/pulsar/actions/runs/42578218386 (the one failed job could not download the Gradle distribution)

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Dependencies: pulsar-broker adds io.prometheus:prometheus-metrics-core 1.8.0 (already shipped). Metrics: format changes listed above.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

…mbedded bookie

Main.buildBookieServer() created a second provider from the same configuration, and that one still landed on the 1.x default registry.
…n Prometheus registry

BKCluster and LocalBookkeeperEnsemble let EmbeddedServer build the provider, which lands
on the default registry the broker's /metrics reads. They now create and own the provider
the same way PulsarBrokerStarter does, through a shared BookieStatsProviders helper.
…ava client 1.x

The four per-broker collectors become MultiCollectors on PrometheusRegistry, keeping
the per-registry sharing, per-cluster aggregation and retirement of counters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants