Skip to content

Change the pulsar_client_sending_buffers_count metric to client level#1408

Merged
RobertIndie merged 4 commits into
apache:masterfrom
BewareMyPower:bewaremypower/fix-metric-label
Aug 21, 2025
Merged

Change the pulsar_client_sending_buffers_count metric to client level#1408
RobertIndie merged 4 commits into
apache:masterfrom
BewareMyPower:bewaremypower/fix-metric-label

Conversation

@BewareMyPower

@BewareMyPower BewareMyPower commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

#1394 introduces the pulsar_client_sending_buffers_count metric to track how many buffers are allocated for send purpose and not put back to the pool. However, unlike other metrics, this metric is not client level, so it cannot be attached with CustomMetricsLabels in client options.

When a send buffer was not put back to the pool, it means the Release method is not called due to some reason. Changing this metric to client level could help locate which client has triggered this bug in an application that has many client instances from different businesses.

Here is an example metric when I configured CustomMetricsLabels: map[string]string{"key": "value"} after this change

pulsar_client_sending_buffers_count{client="go",key="value"} 1

@BewareMyPower BewareMyPower self-assigned this Aug 19, 2025
@BewareMyPower BewareMyPower requested review from RobertIndie and crossoverJie and removed request for RobertIndie August 19, 2025 12:07
@BewareMyPower BewareMyPower marked this pull request as draft August 19, 2025 12:12
@BewareMyPower BewareMyPower marked this pull request as ready for review August 19, 2025 12:44
@BewareMyPower BewareMyPower added this to the v0.17.0 milestone Aug 19, 2025
Comment thread pulsar/producer_partition.go Outdated
@crossoverJie crossoverJie requested a review from Copilot August 20, 2025 05:54

This comment was marked as outdated.

@crossoverJie

Copy link
Copy Markdown
Member

Can you explain the motivation for this change?

@BewareMyPower BewareMyPower marked this pull request as draft August 20, 2025 07:14
@BewareMyPower BewareMyPower force-pushed the bewaremypower/fix-metric-label branch from 0f7c9bc to ba298d3 Compare August 20, 2025 11:38
@BewareMyPower BewareMyPower changed the title Attach custom labels to the pulsar_client_sending_buffers_count metric Change the pulsar_client_sending_buffers_count metric to client level Aug 20, 2025
@BewareMyPower BewareMyPower marked this pull request as ready for review August 20, 2025 13:59
@BewareMyPower BewareMyPower requested a review from Copilot August 20, 2025 13:59
@BewareMyPower

Copy link
Copy Markdown
Contributor Author

@crossoverJie I have updated the PR description, PTAL again

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the pulsar_client_sending_buffers_count metric from a global level to client level, enabling it to be tagged with custom metrics labels. This change allows better tracking of buffer allocation issues across multiple client instances in applications.

Key changes include:

  • Refactored metric creation to use client-specific labels instead of global constants
  • Updated buffer allocation tracking to use client-level metrics
  • Modified batch builders and producers to accept metrics as a parameter

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pulsar/internal/metrics.go Moved SendingBuffersCount from global variable to client-level metric with custom labels support
pulsar/internal/buffer.go Added release callback mechanism to track buffer lifecycle and removed global metric tracking
pulsar/internal/batch_builder.go Updated to accept metrics parameter and track buffer allocation at client level
pulsar/internal/key_based_batch_builder.go Updated function signatures to pass metrics parameter through batch container creation
pulsar/internal/key_based_batch_builder_test.go Added metrics parameter to test setup
pulsar/producer_partition.go Updated to pass client metrics to batch builder and track buffer allocation in single send

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread pulsar/internal/buffer.go Outdated
Comment thread pulsar/internal/buffer.go Outdated

@crossoverJie crossoverJie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@RobertIndie RobertIndie merged commit d471a67 into apache:master Aug 21, 2025
7 checks passed
@BewareMyPower BewareMyPower deleted the bewaremypower/fix-metric-label branch August 21, 2025 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants