Skip to content

Conversation

@Shawyeok
Copy link
Contributor

@Shawyeok Shawyeok commented Dec 24, 2022

Motivation

Broker configuration exposeSubscriptionBacklogSizeInPrometheus was introduced in #9302, but it is true, broker just calculate the size of backlog and didn't expose it.

Modifications

  • if and only if exposeSubscriptionBacklogSizeInPrometheus=true, expose the metric as pulsar_subscription_back_log_size

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests and can be verified as follows:

  • if exposeSubscriptionBacklogSizeInPrometheus=false, it shouldn't expose the metric
  • if exposeSubscriptionBacklogSizeInPrometheus=true, it should expose the metric

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

Documentation

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

Matching PR in forked repository

PR in forked repository: Shawyeok#6

@Shawyeok
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@Shawyeok Shawyeok force-pushed the fix-exposeSubscriptionBacklogSizeInPrometheus branch from 0441fe8 to 09e5f39 Compare December 24, 2022 14:49
@Shawyeok Shawyeok changed the title [fix][broker] Fix exposeBundlesMetricsInPrometheus=true not working [fix][broker] Fix configuration exposeSubscriptionBacklogSizeInPrometheus not working Dec 24, 2022
@Shawyeok
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@Shawyeok
Copy link
Contributor Author

PTAL @codelipenghui

Copy link
Contributor

@gaoran10 gaoran10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@Shawyeok Shawyeok force-pushed the fix-exposeSubscriptionBacklogSizeInPrometheus branch from 09e5f39 to 4dfb6d4 Compare December 27, 2022 16:27
@codelipenghui
Copy link
Contributor

@Shawyeok A PIP is required If the change will introduce a new indicator to the metrics endpoint. Please see https://github.com/apache/pulsar/blob/master/wiki/proposals/PIP.md and the discussion under the mailing list https://lists.apache.org/thread/bmwtk6zcyt4ft4t95jmz1bnsxbysjw8m

I support the motivation.

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PIP is required.

@codelipenghui codelipenghui added this to the 2.12.0 milestone Dec 28, 2022
@Shawyeok
Copy link
Contributor Author

@Shawyeok A PIP is required If the change will introduce a new indicator to the metrics endpoint. Please see https://github.com/apache/pulsar/blob/master/wiki/proposals/PIP.md and the discussion under the mailing list https://lists.apache.org/thread/bmwtk6zcyt4ft4t95jmz1bnsxbysjw8m

I support the motivation.

Ok, I'll draft a PIP in the weekend.

@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Jan 29, 2023
@poorbarcode poorbarcode modified the milestones: 3.0.0, 3.1.0 Apr 10, 2023
@Technoboy- Technoboy- modified the milestones: 3.1.0, 3.2.0 Jul 31, 2023
@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023
@coderzc coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024
@lhotari lhotari modified the milestones: 4.0.0, 4.1.0 Oct 14, 2024
@coderzc coderzc modified the milestones: 4.1.0, 4.2.0 Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-required Your PR changes impact docs and you will update later. Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants