Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][broker] Add metrics related interfaces for BrokerInterceptor #20772

Open
2 tasks done
crossoverJie opened this issue Jul 10, 2023 · 4 comments · May be fixed by #21102
Open
2 tasks done

[improve][broker] Add metrics related interfaces for BrokerInterceptor #20772

crossoverJie opened this issue Jul 10, 2023 · 4 comments · May be fixed by #21102
Assignees
Labels
Stale type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Comments

@crossoverJie
Copy link
Member

crossoverJie commented Jul 10, 2023

Search before asking

  • I searched in the issues and found nothing similar.

Motivation

When I want to monitor the relevant data in the BrokerInterceptor, currently the BrokerInterceptor does not provide relevant interfaces.

I think providing such extensions is necessary.

Solution

https://github.com/apache/pulsar/blob/e96b3398912163eb6e0528c10aed3507c95952fd/pulsar-broker/src/main/java/org/apache/pulsar/broker/intercept/BrokerInterceptor.java

Add a interface, like this:

default void addCustomizeMetrics(PrometheusMetricStreams streams, PulsarService pulsar){
}

PrometheusMetricStreams metricStreams = new PrometheusMetricStreams();
try {
SimpleTextOutputStream stream = new SimpleTextOutputStream(buf);

Then callback the addCustomizeMetrics(stream, pulsar) method here.

This is just a draft.

Alternatives

No response

Anything else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@crossoverJie crossoverJie added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Jul 10, 2023
@crossoverJie
Copy link
Member Author

@codelipenghui Please take a look.

@asafm
Copy link
Contributor

asafm commented Jul 18, 2023

I think it's ok.
I presume you need high granularity metrics - topic level, which is why you want to use PrometheusMetricStreams, right?

This only emphasize the need to solve this properly via OpenTelemetry API and let plugins register their own metrics easily, as part of PIP-264: #20197
Once approved and we'll get to the plugin section, we'll add a proper method for registering metrics, which will consistent across all plugin interface and not bespoke as it is today.

@crossoverJie
Copy link
Member Author

I read the PIP-264 and discussion threads, it sounds great, but it looks like it will take a lot of time investment.

Therefore, I think we can add this feature within the current context.

@github-actions
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants