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

prometheus metrics for external stream sink #485

Merged

Conversation

qijun-niu-timeplus
Copy link
Collaborator

PR checklist:

  • Did you run ClangFormat ?
  • Did you separate headers to a different section in existing community code base ?
  • Did you surround proton: starts/ends for new code in existing community code base ?

Please write user-readable short description of the changes:

how to check:

  1. create a topic test on redpanda/kafka

  2. proton client

create external stream test(i int, j int) settings type='kafka', brokers='redpanda:9092', topic='test', data_format='JSONEachRow';
insert into test(i, j) values(1, 2)(3, 4)(5, 6);
select * from test settings seek_to='earliest'
  1. query prometheus metrics
curl http://localhost:9363/metrics

response:

...
# TYPE ProtonExternalStream_ReadBytes counter
ProtonExternalStream_ReadBytes{database="default", name="test"} 42
# TYPE ProtonExternalStream_ReadCounts counter
ProtonExternalStream_ReadCounts{database="default", name="test"} 3
# TYPE ProtonExternalStream_ReadFailed counter
ProtonExternalStream_ReadFailed{database="default", name="test"} 0
# TYPE ProtonExternalStream_WriteBytes counter
ProtonExternalStream_WriteBytes{database="default", name="test"} 42
# TYPE ProtonExternalStream_WriteCounts counter
ProtonExternalStream_WriteCounts{database="default", name="test"} 3
# TYPE ProtonExternalStream_WriteFailed counter
ProtonExternalStream_WriteFailed{database="default", name="test"} 0

Copy link
Collaborator

@yokofly yokofly left a comment

Choose a reason for hiding this comment

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

overall is good,

we can consider to use relaxed for this pure atomic counter. counter is not related to happen before. this is sufficient.

No ordering constraints, only atomic operations.

but we can create a new ticket for statistically the metrics.
https://stackoverflow.com/a/27716387
https://en.cppreference.com/w/cpp/atomic/memory_order#Relaxed_ordering

@yokofly
Copy link
Collaborator

yokofly commented Jan 15, 2024

create a new ticket here #488

@qijun-niu-timeplus qijun-niu-timeplus merged commit ab22690 into develop Jan 16, 2024
21 checks passed
@yokofly yokofly deleted the feature/issue-438-external-stream-sink-metrics branch February 5, 2024 11:40
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.

2 participants