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.

Sign up for GitHub

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

Jump to bottom

[Feature][Core] Support record metrics in flink engine #6035

Merged
merged 1 commit into from
Dec 24, 2023
Merged

[Feature][Core] Support record metrics in flink engine #6035

merged 1 commit into from
Dec 24, 2023

Conversation

TyrantLucifer
Copy link
Member

Purpose of this pull request

Support record metrics in flink engine

Does this PR introduce any user-facing change?

No

How was this patch tested?

You can see job summary when job finished in logs.

Check list

@TyrantLucifer
Copy link
Member Author

@hailin0 @EricJoy2048 @Hisoka-X @ic4y PTAL.

@Hisoka-X
Copy link
Member

Please fix test case

@TyrantLucifer TyrantLucifer force-pushed the flink-support-record-metric branch from 61ff03d to e98d4cd Compare December 20, 2023 07:35
@TyrantLucifer
Copy link
Member Author

Please fix test case

Done.

@hailin0 hailin0 added core SeaTunnel core module feature New feature flink labels Dec 21, 2023
@hailin0
Copy link
Member

hailin0 commented Dec 21, 2023 •
edited
Loading

Please add testcase check metrics key & value

@TyrantLucifer
Copy link
Member Author

Please add testcase check metrics key & value

image image

@TyrantLucifer
Copy link
Member Author

This pr used the origin filnk metric system, so it does not has measure to write a test case for it. Currently to verify metrics can see flink web ui. I have offered the snapshot screen images in this pr. cc @hailin0 @Hisoka-X @EricJoy2048

@Hisoka-X
Copy link
Member

This pr used the origin filnk metric system, so it does not has measure to write a test case for it. Currently to verify metrics can see flink web ui. I have offered the snapshot screen images in this pr. cc @hailin0 @Hisoka-X @EricJoy2048

how about use flink web http server to check metrics? Query it in docker e2e environment. I'm worry about it will not worked in the future after other guys change the code.

@TyrantLucifer TyrantLucifer force-pushed the flink-support-record-metric branch from e98d4cd to 560b7c0 Compare December 21, 2023 14:32
@TyrantLucifer
Copy link
Member Author

This pr used the origin filnk metric system, so it does not has measure to write a test case for it. Currently to verify metrics can see flink web ui. I have offered the snapshot screen images in this pr. cc @hailin0 @Hisoka-X @EricJoy2048

how about use flink web http server to check metrics? Query it in docker e2e environment. I'm worry about it will not worked in the future after other guys change the code.

Got it, hey guys I have added the e2e test case in latest commit, please review again. cc @Hisoka-X @hailin0 @EricJoy2048

@TyrantLucifer TyrantLucifer force-pushed the flink-support-record-metric branch from 560b7c0 to 848e568 Compare December 22, 2023 03:09
@TyrantLucifer TyrantLucifer force-pushed the flink-support-record-metric branch from 848e568 to 373fd11 Compare December 22, 2023 05:30
@TyrantLucifer TyrantLucifer force-pushed the flink-support-record-metric branch 2 times, most recently from 5813505 to 9646664 Compare December 22, 2023 09:30
@TyrantLucifer TyrantLucifer force-pushed the flink-support-record-metric branch from 9646664 to c4a15d1 Compare December 22, 2023 13:01
@TyrantLucifer TyrantLucifer force-pushed the flink-support-record-metric branch from c4a15d1 to 93cbee2 Compare December 22, 2023 13:58
hailin0
hailin0 approved these changes Dec 23, 2023
View reviewed changes
Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

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

LGTM

@Hisoka-X Hisoka-X merged commit a9ec9d5 into apache:dev Dec 24, 2023
5 checks passed
alextinng pushed a commit to alextinng/seatunnel that referenced this pull request Dec 26, 2023
@TyrantLucifer
[Feature][Core] Support record metrics in flink engine (apache#6035)
1401a9b
Japson0 added a commit to Japson0/seatunnel that referenced this pull request Jan 4, 2024
@Japson0
Revert "[Feature][Core] Support record metrics in flink engine (apach…
c94a9d1 
…e#6035)"

This reverts commit a9ec9d5.
chaorongzhi pushed a commit to chaorongzhi/seatunnel that referenced this pull request Aug 21, 2024
@TyrantLucifer
[Feature][Core] Support record metrics in flink engine (apache#6035)
56375e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hisoka-X Hisoka-X Hisoka-X approved these changes

@hailin0 hailin0 hailin0 approved these changes

@ic4y ic4y Awaiting requested review from ic4y

@EricJoy2048 EricJoy2048 Awaiting requested review from EricJoy2048

Assignees
No one assigned
Labels
approved core SeaTunnel core module feature New feature flink reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TyrantLucifer @Hisoka-X @hailin0 @EricJoy2048