Skip to content

[receiver/Flinkmetricsreceiver] Apache Flink Metric Receiver #10121

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
Merged
merged 36 commits into from
Jun 1, 2022
Merged

[receiver/Flinkmetricsreceiver] Apache Flink Metric Receiver #10121

merged 36 commits into from
Jun 1, 2022

Conversation

JonathanWamsley
Copy link
Contributor

Description: Add New Component: Apache Flink Metric Receiver

Link to tracking Issue: New Component: Apache Flink Metric Receiver

Testing: A series of testing was added including:

  • config_test.go
  • client_test.go
  • scraper_test.go
  • factory_test.go
  • integration_test.go

Documentation: README.md documentation.md

@JonathanWamsley JonathanWamsley force-pushed the flinkmetricsreceiver branch 2 times, most recently from d086817 to 5744c6e Compare May 18, 2022 09:57
antonblock
antonblock reviewed May 18, 2022
View reviewed changes
TylerHelmuth
TylerHelmuth reviewed May 18, 2022
View reviewed changes
@JonathanWamsley JonathanWamsley force-pushed the flinkmetricsreceiver branch 2 times, most recently from 32ced72 to f8eed3a Compare May 18, 2022 17:54
@JonathanWamsley JonathanWamsley marked this pull request as ready for review May 18, 2022 19:40
@JonathanWamsley JonathanWamsley requested review from a team and djaglowski May 18, 2022 19:40
djaglowski
djaglowski reviewed May 19, 2022
View reviewed changes
@JonathanWamsley JonathanWamsley force-pushed the flinkmetricsreceiver branch 5 times, most recently from 553ec66 to 1743f8f Compare May 23, 2022 13:28
djaglowski
djaglowski reviewed May 23, 2022
View reviewed changes
@JonathanWamsley
Copy link
Contributor Author

I updated pr feedback @djaglowski and it is ready to be looked at again please.

djaglowski
djaglowski reviewed May 24, 2022
View reviewed changes
@JonathanWamsley JonathanWamsley force-pushed the flinkmetricsreceiver branch 2 times, most recently from 7f104f5 to 7465bfc Compare May 25, 2022 13:11
@JonathanWamsley
Copy link
Contributor Author

PR is ready for review again please @djaglowski

djaglowski
djaglowski reviewed May 25, 2022
View reviewed changes
@JonathanWamsley JonathanWamsley force-pushed the flinkmetricsreceiver branch 3 times, most recently from 9e336d0 to 07cce9f Compare May 26, 2022 14:56
@JonathanWamsley
Copy link
Contributor Author

JonathanWamsley commented May 26, 2022 •
edited
Loading

Ready to be looked at again @djaglowski please

djaglowski
djaglowski reviewed May 26, 2022
View reviewed changes
receiver/flinkmetricsreceiver/metadata.yaml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

%?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Copy link
Member

Choose a reason for hiding this comment

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

I'm confused. I thought you said this metric is measured in %. That's my interpretation from the article you linked.

Copy link
Member

Choose a reason for hiding this comment

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

@JonathanWamsley PTAL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I could not find a unit though and searching through the metadata.yaml mongo atlas had that convention. so I put it in. I thought you were saying with "%?" to remove that convention or something.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, should the units be % or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes they should be. The unit can be 1 but not % because that will break in the metadata generation. Maybe "{percentage}" instead?

Copy link
Member

Choose a reason for hiding this comment

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

"%" should work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated unit

djaglowski
djaglowski reviewed May 26, 2022
View reviewed changes
@JonathanWamsley
unexport process methods
3b96bbe
@JonathanWamsley
go tidy up
059b852
@JonathanWamsley
add partial metrics collecting on taskmanager, jobs, and subtasks
2556c3a
@JonathanWamsley
go tidy update
e987ade
@JonathanWamsley
fix shadow declaration of err
0809afa
@JonathanWamsley
update pr feedback
cf7044b
@JonathanWamsley
tidy up
13ce08f
@JonathanWamsley
apply scope version update
a19581e
@JonathanWamsley
make expected responses easier to test when metadata/generated files …
eeaa643 
…update
@JonathanWamsley
update to host.name and move status table
c89fc4d
@JonathanWamsley
combine jobmanager and taskmanager metrics
40c0db2
@JonathanWamsley
tidy up
84a0a61
@JonathanWamsley
update generated files
2c2b358
@JonathanWamsley
fix mod version
c0c7436
@JonathanWamsley
update pr feedback
4a7d836
@JonathanWamsley
tidy up
fc5a9b6
@JonathanWamsley
seperated checkpoint in_progress as non-monotonic sum metric
d77c532
@JonathanWamsley
tidy up local receiver
4c244c6
@JonathanWamsley
update version
52e5196
@JonathanWamsley
update cpu descriptions
53620aa
@JonathanWamsley
fix taskmanager/jobs bug and added tests with multiple taskmanagers/jobs
34383e1
@JonathanWamsley
space
57f27c7
@JonathanWamsley
change non_heap to nonheap
681ed51
@JonathanWamsley
update unit
7e49410