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] [pip] PIP-363: Add callback parameters to the method: org.apache.pulsar.client.impl.SendCallback.sendComplete. #22940

Merged
merged 8 commits into from
Aug 13, 2024

Conversation

crossoverJie
Copy link
Member

Motivation

As introduced in PIP-264, Pulsar has been fully integrated into the OpenTelemetry system, which defines some metric specifications for messaging systems.

In the current Pulsar client code, it is not possible to obtain the number of messages sent in batches(as well as some other sending data), making it impossible to implement messaging.publish.messages metric.

In the opentelemetry-java-instrumentation code, the org.apache.pulsar.client.impl.SendCallback interface is used to instrument data points. For specific implementation details, we can refer to this.

In the current situation, org.apache.pulsar.client.impl.ProducerImpl does not provide a public method to obtain the numMessagesInBatch.

So, we can add some of org.apache.pulsar.client.impl.ProducerImpl.OpSendMsg's key data into the org.apache.pulsar.client.impl.SendCallback.sendComplete method.

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:

@github-actions github-actions bot added PIP doc-not-needed Your PR changes do not impact docs labels Jun 19, 2024
pip/pip-363.md Outdated Show resolved Hide resolved
@nodece
Copy link
Member

nodece commented Jun 21, 2024

+1

pip/pip-363.md Outdated Show resolved Hide resolved
pip/pip-363.md Outdated Show resolved Hide resolved
@crossoverJie
Copy link
Member Author

@lhotari Do you have any suggestions?

@crossoverJie
Copy link
Member Author

@lhotari @Technoboy- Could you please vote here? Thank you.

@crossoverJie
Copy link
Member Author

The vote has passed with 3 binding +1s. https://lists.apache.org/thread/t0olt3722j17gjtdxqqsl3cpy104ogpr

@crossoverJie
Copy link
Member Author

@RobertIndie @BewareMyPower Could you please approve and merge this PR?

@BewareMyPower BewareMyPower merged commit fe21441 into apache:master Aug 13, 2024
20 checks passed
grssam pushed a commit to grssam/pulsar that referenced this pull request Sep 4, 2024
…pache.pulsar.client.impl.SendCallback.sendComplete. (apache#22940)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs PIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants