[fix][broker] PIP-399: Fix Metric Name for Delayed Queue

[thetumbled]

PIP: #23789

Motivation

There is already one metric called pulsar_delayed_message_index_size_bytes for the total memory occupation used by delayed queue of one topic.

writeMetric(stream, "pulsar_delayed_message_index_size_bytes", stats.delayedTrackerMemoryUsage,
                cluster, namespace, topic, splitTopicAndPartitionIndexLabel);

Whereas, the metric for one sub also called pulsar_delayed_message_index_size_bytes, which do not comform the metric name norm and is confusing.

writeSubscriptionMetric(stream, "pulsar_delayed_message_index_size_bytes",
                    subsStats.delayedTrackerMemoryUsage, cluster, namespace, topic, sub, splitTopicAndPartitionIndexLabel);

Currently, it can export metric like:

# TYPE pulsar_delayed_message_index_size_bytes gauge
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0"} 0
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0",subscription="sub2"} 0

The metric of topic and subscription mix together. If we want to filter out the metric of sub to pick out the metric of topic, we need to use promsql like:
pulsar_delayed_message_index_size_bytes{subscription=""}
It is quite weird and i believe that if any user use it already, he/she will contribute a pr to fix this, as these label names can only be access by researching the code, which means he/she is an advanced user and is involved in the community probably.

Modifications

Rename the metric for one sub to pulsar_subscription_delayed_message_index_size_bytes.

Verifying this change

• Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

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:

[dao-jun]

LGTM. However, since it changes the metrics name, I'm not sure if it is OK to merge the PR without a discussion.

[thetumbled]

WDYT, @lhotari @coderzc

[lhotari]

Whereas, the metric for one sub also called pulsar_delayed_message_index_size_bytes, which do not comform the metric name norm and is confusing.

@thetumbled Does this cause any problem other than it's not consistent with other subscription level metric name?

According to our PIP guidelines, changes like this require a PIP since it would be a breaking change in metrics. However, if this would be a bug where the metric wouldn't be usable at all, then a PIP wouldn't be required.

[thetumbled]

Whereas, the metric for one sub also called pulsar_delayed_message_index_size_bytes, which do not comform the metric name norm and is confusing.

@thetumbled Does this cause any problem other than it's not consistent with other subscription level metric name?

According to our PIP guidelines, changes like this require a PIP since it would be a breaking change in metrics. However, if this would be a bug where the metric wouldn't be usable at all, then a PIP wouldn't be required.

Currently, it can export metric like:

# TYPE pulsar_delayed_message_index_size_bytes gauge
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0"} 0
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0",subscription="sub2"} 0

The metric of topic and subscription mix together. If we want to filter out the metric of sub to pick out the metric of topic, we need to use promsql like:
pulsar_delayed_message_index_size_bytes{subscription=""}
It is quite weird and i believe that if any user use it already, he/she will contribute a pr to fix this, as these label names can only be access by researching the code, which means he/she is an advanced user and is involved in the community probably.

[lhotari]

Currently, it can export metric like:

# TYPE pulsar_delayed_message_index_size_bytes gauge
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0"} 0
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0",subscription="sub2"} 0

The metric of topic and subscription mix together. If we want to filter out the metric of sub to pick out the metric of topic, we need to use promsql like: pulsar_delayed_message_index_size_bytes{subscription=""} It is quite weird and i believe that if any user use it already, he/she will contribute a pr to fix this, as these label names can only be access by researching the code, which means he/she is an advanced user and is involved in the community probably.

@thetumbled Please add this directly to the description of this PR since it clarifies the issue. I'd suggest to start a discussion on that mailing list with these details. It's simplest to create a minimal PIP in any case to get this change documented.

[thetumbled]

Currently, it can export metric like:

# TYPE pulsar_delayed_message_index_size_bytes gauge
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0"} 0
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0",subscription="sub2"} 0

The metric of topic and subscription mix together. If we want to filter out the metric of sub to pick out the metric of topic, we need to use promsql like: pulsar_delayed_message_index_size_bytes{subscription=""} It is quite weird and i believe that if any user use it already, he/she will contribute a pr to fix this, as these label names can only be access by researching the code, which means he/she is an advanced user and is involved in the community probably.

@thetumbled Please add this directly to the description of this PR since it clarifies the issue. I'd suggest to start a discussion on that mailing list with these details. It's simplest to create a minimal PIP in any case to get this change documented.

Sure, i will start one to discuss on this change.

[thetumbled]

Currently, it can export metric like:

# TYPE pulsar_delayed_message_index_size_bytes gauge
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0"} 0
pulsar_delayed_message_index_size_bytes{cluster="MyPulsar",namespace="public/default",topic="persistent://public/default/testNack-partition-0",subscription="sub2"} 0

The metric of topic and subscription mix together. If we want to filter out the metric of sub to pick out the metric of topic, we need to use promsql like: pulsar_delayed_message_index_size_bytes{subscription=""} It is quite weird and i believe that if any user use it already, he/she will contribute a pr to fix this, as these label names can only be access by researching the code, which means he/she is an advanced user and is involved in the community probably.

@thetumbled Please add this directly to the description of this PR since it clarifies the issue. I'd suggest to start a discussion on that mailing list with these details. It's simplest to create a minimal PIP in any case to get this change documented.

I propose a pip to discuss on this change: #23789.