Skip to content
This repository was archived by the owner on Sep 21, 2021. It is now read-only.

Add initial Prometheus Support #533

Merged
merged 4 commits into from
Apr 24, 2018
Merged

Add initial Prometheus Support #533

merged 4 commits into from
Apr 24, 2018

Conversation

pearj
Copy link
Collaborator

@pearj pearj commented Apr 20, 2018

Description

Add a Prometheus /metrics endpoint to allow monitoring of the zalenium server, initially the JVM and Jetty metrics. In the future, we would add specific zalenium metrics. This lays down the initial foundation.

To enable Prometheus to work, it needed to be integrated into the Jetty server, so AspectJ has been used to access the necessary classes.

Registration of the zalenium specific servlets has been moved into Aspect, so that we get greater control of the URLs that we can use.

I also fixed a bug where the cleanup servlet would crash if it tried to delete files/directories that didn't exist.

Additionally, the Nginx redirect problem has been fixed.

Motivation and Context

This allows us to get greater insight into the performance of zalenium and how it uses resources. Prometheus has become the defacto standard for monitoring kubernetes clusters, so it makes to most sense to use it.

How Has This Been Tested?

I ran few multi-threaded selenium tests and checked the /metrics endpoint. I also tested the existing zalenium servlets to ensure they still operated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • All new and existing tests passed.

@codecov-io
Copy link

codecov-io commented Apr 20, 2018

Codecov Report

Merging #533 into merge-533 will decrease coverage by 0.9%.
The diff coverage is 15.55%.

@@               Coverage Diff               @@
##             merge-533     #533      +/-   ##
===============================================
- Coverage        80.95%   80.04%   -0.91%     
+ Complexity         585      584       -1     
===============================================
  Files               32       33       +1     
  Lines             2688     2741      +53     
  Branches           233      233              
===============================================
+ Hits              2176     2194      +18     
- Misses             372      406      +34     
- Partials           140      141       +1

@pearj
Copy link
Collaborator Author

pearj commented Apr 20, 2018

👍

Copy link
Contributor

@parberge parberge left a comment

Choose a reason for hiding this comment

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

Haven't tested anything, but looks good 😀

@pearj
Copy link
Collaborator Author

pearj commented Apr 20, 2018

👍

@parberge
Copy link
Contributor

So this should resolve #353

@parberge
Copy link
Contributor

👍

@diemol
Copy link
Contributor

diemol commented Apr 23, 2018

Cool, thanks @pearj!

What do you think? Should we merge this before #527 or after?

@diemol
Copy link
Contributor

diemol commented Apr 23, 2018

Maybe before since the other one is way bigger?

@pearj
Copy link
Collaborator Author

pearj commented Apr 23, 2018

Merge this before. I originally raised it as a pull request to @dspasojevic’s fork. But he suggested I create it as a separate request because it’s easier to merge.

@diemol diemol changed the base branch from master to merge-533 April 24, 2018 06:13
@diemol
Copy link
Contributor

diemol commented Apr 24, 2018

Sounds good, I'll merge it to a protected branch to run the complete CI.

Thanks again!

@diemol diemol merged commit 732d6ed into zalando:merge-533 Apr 24, 2018
@diemol diemol mentioned this pull request Apr 25, 2018
pearj added a commit to pearj/zalenium that referenced this pull request Apr 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants