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

Metrics for monitoring endpoints #24

Merged
merged 7 commits into from
Aug 16, 2024
Merged

Metrics for monitoring endpoints #24

merged 7 commits into from
Aug 16, 2024

Conversation

51-code
Copy link
Contributor

@51-code 51-code commented May 28, 2024

Fixes #8

Added metrics for:

  • Latency per partition (event's enqueued time - read time)
  • Depth per partition (last enqueued event's offset - accepted event's offset)
  • Data depth estimation (average message size / amount of messages passed)

Dropwizard setup was inspired by CFE-35 Router.java

Needs to be tested, but this is the initial code.

UPDATE: Tests done!

@51-code 51-code added assistance Extra attention, more information or help is needed enhancement New feature or request labels May 28, 2024
@51-code 51-code requested a review from kortemik May 28, 2024 09:47
@51-code 51-code self-assigned this May 28, 2024
@kortemik kortemik requested a review from StrongestNumber9 May 28, 2024 12:16
@kortemik
Copy link
Member

@StrongestNumber9 please check this out

Copy link
Contributor

@StrongestNumber9 StrongestNumber9 left a comment

Choose a reason for hiding this comment

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

Added some notes

@51-code
Copy link
Contributor Author

51-code commented May 28, 2024

Discussed with @kortemik that an EventContextFactory can be implemented for unit testing. When producing new events with the factory, we can then call EventContextConsumer's accept() function and test the metrics by getting the Gauge's and asserting the correct value of getValue(). Later the factory can be used for testing the class's other functionality as well.

I'll start working on developing this and I will add it to the PR when ready.

@51-code
Copy link
Contributor Author

51-code commented Jun 3, 2024

Added testing for the metrics and fixed the issues that came up in the first round of review.

Made a fake object for the Output to get rid of dependencies of the EventContextConsumer object, and the class was refactored with a few new constructors to allow this fake to be injected. Fakes were also done for multiple objects in Azure library for creating "data" for the object, the CheckPointlessEventContextFactory is responsible for data creation.

@51-code 51-code requested a review from StrongestNumber9 June 3, 2024 08:07
Copy link
Contributor

@StrongestNumber9 StrongestNumber9 left a comment

Choose a reason for hiding this comment

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

Some minor tweaks

@51-code
Copy link
Contributor Author

51-code commented Jun 3, 2024

Ready with all the required fixes. @kortemik @StrongestNumber9

Copy link
Contributor

@StrongestNumber9 StrongestNumber9 left a comment

Choose a reason for hiding this comment

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

lgtm at this point

@51-code 51-code requested a review from p000010u August 13, 2024 07:57
@51-code 51-code requested a review from kortemik August 15, 2024 06:14
@kortemik kortemik merged commit 79b1032 into teragrep:main Aug 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assistance Extra attention, more information or help is needed enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement monitoring endpoints
4 participants