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

enhance: add streaming client metrics #36523

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

chyezh
Copy link
Contributor

@chyezh chyezh commented Sep 25, 2024

issue: #33285

@sre-ci-robot sre-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines. label Sep 25, 2024
@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Sep 25, 2024
@chyezh chyezh force-pushed the feat_streaming_node_metrics branch 2 times, most recently from 982ff08 to c57d20f Compare September 27, 2024 09:14
@chyezh chyezh force-pushed the feat_streaming_node_metrics branch 2 times, most recently from 4031a2c to e459ab5 Compare September 28, 2024 08:15
Signed-off-by: chyezh <[email protected]>
@chyezh chyezh force-pushed the feat_streaming_node_metrics branch from e459ab5 to 8955d01 Compare September 28, 2024 10:02
@mergify mergify bot added the ci-passed label Sep 28, 2024
Copy link

codecov bot commented Sep 28, 2024

Codecov Report

Attention: Patch coverage is 92.76650% with 57 lines in your changes missing coverage. Please review.

Project coverage is 82.44%. Comparing base (bd79106) to head (8955d01).
Report is 37 commits behind head on master.

Files with missing lines Patch % Lines
...l/streamingnode/server/wal/metricsutil/timetick.go 81.81% 6 Missing and 6 partials ⚠️
...l/interceptors/segment/manager/pchannel_manager.go 35.71% 9 Missing ⚠️
...distributed/streaming/internal/consumer/metrics.go 86.11% 2 Missing and 3 partials ⚠️
...distributed/streaming/internal/producer/metrics.go 89.13% 2 Missing and 3 partials ⚠️
...ternal/streamingnode/server/wal/metricsutil/txn.go 83.33% 2 Missing and 2 partials ⚠️
...l/streamingnode/server/wal/metricsutil/wal_scan.go 93.65% 4 Missing ⚠️
.../streamingnode/server/wal/metricsutil/wal_write.go 94.11% 3 Missing ⚠️
pkg/util/syncutil/closed_lock.go 75.00% 2 Missing and 1 partial ⚠️
...ver/wal/interceptors/segment/manager/seal_queue.go 93.10% 2 Missing ⚠️
...al/interceptors/segment/manager/segment_manager.go 85.71% 1 Missing and 1 partial ⚠️
... and 7 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #36523      +/-   ##
==========================================
+ Coverage   73.38%   82.44%   +9.06%     
==========================================
  Files        1281     1293      +12     
  Lines      151464   152207     +743     
==========================================
+ Hits       111153   125494   +14341     
+ Misses      35292    21684   -13608     
- Partials     5019     5029      +10     
Files with missing lines Coverage Δ
...buted/streaming/internal/consumer/consumer_impl.go 89.18% <100.00%> (+2.52%) ⬆️
...distributed/streaming/internal/consumer/handler.go 100.00% <100.00%> (ø)
...ted/streaming/internal/consumer/message_handler.go 100.00% <ø> (ø)
...istributed/streaming/internal/producer/producer.go 88.04% <100.00%> (+0.98%) ⬆️
.../streamingcoord/server/balancer/channel/manager.go 98.40% <100.00%> (+0.18%) ⬆️
.../streamingcoord/server/balancer/channel/metrics.go 100.00% <100.00%> (ø)
...streamingcoord/server/balancer/channel/pchannel.go 100.00% <100.00%> (ø)
...ternal/streamingcoord/server/service/assignment.go 100.00% <100.00%> (ø)
...streamingnode/server/resource/idalloc/allocator.go 100.00% <100.00%> (ø)
internal/streamingnode/server/service/handler.go 69.23% <ø> (-7.24%) ⬇️
... and 40 more

... and 272 files with indirect coverage changes

metrics.WALSegmentAllocTotal.DeletePartialMatch(m.constLabel)
metrics.WALSegmentFlushedTotal.DeletePartialMatch(m.constLabel)
metrics.WALPartitionTotal.Delete(m.constLabel)
metrics.WALCollectionTotal.Delete(m.constLabel)
Copy link
Contributor

Choose a reason for hiding this comment

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

The WALSegmentBytes metric also needs to be cleaned up.

@jaime0815
Copy link
Contributor

/lgtm

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: chyezh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit 2ec6e60 into milvus-io:master Oct 8, 2024
16 checks passed
@chyezh chyezh deleted the feat_streaming_node_metrics branch October 8, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved ci-passed dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement lgtm size/XXL Denotes a PR that changes 1000+ lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants