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

[feat][broker] PIP-264: Add OpenTelemetry consumer metrics #22693

Merged
merged 22 commits into from
May 10, 2024

Conversation

dragosvictor
Copy link
Contributor

@dragosvictor dragosvictor commented May 10, 2024

PIP-264

Motivation

Using OpenTelemetry, exposes the current consumer-level metrics emitted by the broker and described in the doc.

Modifications

Similar to topic metrics PR #22467, navigate all the current consumers connected to the broker and expose their respective metrics in OpenTelemetry format.

The metric details can be consulted in the doc PR: apache/pulsar-site#896. Prometheus metric pulsar_consumer_blocked_on_unacked_messages has been dropped in favor of OpenTelemetry boolean attribute pulsar.consumer.blocked on metric pulsar.broker.consumer.message.unack.count, as these values are tightly related.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added test org.apache.pulsar.broker.stats.OpenTelemetryConsumerStatsTest#testMessagingMetrics, verifying all the new metrics.

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

  • 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: As described above
  • Anything that affects deployment

Documentation

Matching PR in forked repository

PR in forked repository: dragosvictor#24

@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label May 10, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@merlimat merlimat merged commit e558cfe into apache:master May 10, 2024
52 of 54 checks passed
@merlimat merlimat added this to the 3.4.0 milestone May 10, 2024
@dragosvictor dragosvictor deleted the dmisca-pip-264-consumer-metrics branch May 11, 2024 02:18
var builder = Attributes.builder()
.put(OpenTelemetryAttributes.PULSAR_CONSUMER_NAME, consumer.consumerName())
.put(OpenTelemetryAttributes.PULSAR_CONSUMER_ID, consumer.consumerId())
.put(OpenTelemetryAttributes.PULSAR_CONSUMER_CONNECTED_SINCE,
Copy link
Contributor

Choose a reason for hiding this comment

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

@merlimat @dragosvictor This and other attributes here are super high cardinality - why are we recording with them? This will "kill" any TSDB and be very costly. Even when compared with pulsar_consumer_msg_rate_out - it didn't have all those attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-3.3 doc-required Your PR changes impact docs and you will update later. ready-to-test release/3.3.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants