Skip to content

KAFKA-14659 source-record-write-[rate|total] metrics include filtered records#13193

Merged
C0urante merged 2 commits intoapache:trunkfrom
hgeraldino:sourcerecord-metrics-patch
Feb 28, 2023
Merged

KAFKA-14659 source-record-write-[rate|total] metrics include filtered records#13193
C0urante merged 2 commits intoapache:trunkfrom
hgeraldino:sourcerecord-metrics-patch

Conversation

@hgeraldino
Copy link
Copy Markdown
Contributor

@hgeraldino hgeraldino commented Feb 3, 2023

This PR aligns the description for source-record-write-rate and source-record-write-total metrics, and excludes the filtered records from the metrics being published.

Today, the source-record-poll-[rate|total] and source-record-write-[rate|total] are publishing similar values (except for cases where exceptions are thrown and retries are needed, which might throw off the rate a little bit). This means that it is not possible to find an accurate number of records written to Kafka by source connectors that use filter predicates.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@clolov
Copy link
Copy Markdown
Contributor

clolov commented Feb 3, 2023

Thank you for spotting this behaviour and contributing a fix! The proposed solution makes sense to me, I will just add @C0urante to put the appropriate tags for "connect" to this pull request and review it once it has been rebased.

@hgeraldino hgeraldino force-pushed the sourcerecord-metrics-patch branch from e85f781 to 3ba9053 Compare February 27, 2023 15:42
@hgeraldino hgeraldino force-pushed the sourcerecord-metrics-patch branch from 3ba9053 to 07d4f4d Compare February 27, 2023 15:44
@hgeraldino
Copy link
Copy Markdown
Contributor Author

Thanks @clolov,

Now that #13191 has been merged, this PR has been rebased and is ready for review.

CC @C0urante

Copy link
Copy Markdown
Contributor

@C0urante C0urante left a comment

Choose a reason for hiding this comment

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

Thanks @hgeraldino, just one small question around potentially-unnecessary docs updates.

Copy link
Copy Markdown
Contributor

@C0urante C0urante left a comment

Choose a reason for hiding this comment

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

LGTM, will merge and backport pending CI build. Thanks @hgeraldino!

@C0urante C0urante merged commit a1b8586 into apache:trunk Feb 28, 2023
C0urante pushed a commit that referenced this pull request Feb 28, 2023
…iltered records (#13193)

Reviewers: Christo Lolov <christololov@gmail.com>, Chris Egerton <chrise@aiven.io>
C0urante pushed a commit that referenced this pull request Feb 28, 2023
…iltered records (#13193)

Reviewers: Christo Lolov <christololov@gmail.com>, Chris Egerton <chrise@aiven.io>
rittikaadhikari added a commit to confluentinc/kafka that referenced this pull request Feb 28, 2023
…tream-trunk-27-Feb-2023

* commit 'dcc179995153c22c6248702976b60755b0b9fda8':
  MINOR: srcJar should depend on processMessages task (apache#13316)
  KAFKA-14659 source-record-write-[rate|total] metrics should exclude filtered records (apache#13193)
  MINOR: ExponentialBackoff Javadoc improvements (apache#13317)
  KAFKA-14742: Throttle connectors in ExactlyOnceSourceIntegrationTest to fix flakey OOMEs (apache#13291)
@hgeraldino hgeraldino deleted the sourcerecord-metrics-patch branch June 4, 2023 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants