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

Add prom scraper endpoint #251

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Add prom scraper endpoint #251

merged 3 commits into from
Sep 12, 2023

Conversation

moleske
Copy link
Member

@moleske moleske commented Jul 11, 2022

Thanks for contributing to the capi_release. To speed up the process of reviewing your pull request please provide us with:

One more attempt at #240
Now we don't set up prom scraper mtls if any scraper property is missing

When cloudfoundry/cf-deployment#970 is merged, we won't need to check if the property is set

  • I have viewed signed and have submitted the Contributor License Agreement

  • I have made this pull request to the develop branch

  • I have run CF Acceptance Tests on bosh lite

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 11, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: dalvarado / name: david (52fbedf)
  • ✅ login: moleske / name: M. Oleske (7ed11a1)
  • ✅ login: svkrieger / name: Sven Krieger (64c6cd6)

@sethboyles
Copy link
Member

@moleske can we think about merging this now that cloudfoundry/cf-deployment#970 is merged?

@moleske
Copy link
Member Author

moleske commented Aug 3, 2022

Yes, we should merge this. I was previously waiting for ruby 3.1 things to be good and then the pipeline to be happy before merging. This is good to go whenever now

@moleske
Copy link
Member Author

moleske commented Aug 19, 2022

/easycla

dalvarado and others added 2 commits September 5, 2023 07:56
available

Co-authored-by: Michael Oleske <moleske@pivotal.io>
Co-authored-by: David Alvarado <alvaradoda@vmware.com>
@svkrieger svkrieger force-pushed the add_prom_scraper_endpoint branch 2 times, most recently from 46d3d23 to bfd4a34 Compare September 5, 2023 06:21
@svkrieger
Copy link
Contributor

I hope it's okay to directly push into this branch. I tried this out in a cf foundation and I could verify that the metrics got scraped by the prom_scraper job and were forwarded to Loggregator.

I removed all locations from the server config, which accepts the prom_scraper certs, except the metrics endpoint.
nginx will return a 404 if it receives a request on that port, which does not match the location /internal/v4/metrics. Alternatively we could add a default location, which returns a 403 - forbidden.

Additionally I replaced the catch-all server name with the actual internal server name. Will this endpoint be called by other consumers from outside, which might use other server names rather than the internal one? If not, I think we don't need a catch-all server name here.

@moleske
Copy link
Member Author

moleske commented Sep 5, 2023

I hope it's okay to directly push into this branch. I tried this out in a cf foundation and I could verify that the metrics got scraped by the prom_scraper job and were forwarded to Loggregator.

I'm thrilled you're updating this branch! We have not been able to dedicate people to working on this due to shifting priorities. So please go ahead and update to get it working!

- Remove unnecessary locations from nginx.conf.erb

All locations removed from nginx config, for port which is only used for the metrics endpoint.
Replaced catch-all servername with internal servername.

- Adjust unit tests

Put tests into correct context
Fix naming of contexts (rubocop)

- Adjust nginx.conf.erb template for better indentation and less blank lines
- Improve scrape TLS file templates
@svkrieger svkrieger merged commit 5f253e2 into develop Sep 12, 2023
1 check passed
@svkrieger svkrieger deleted the add_prom_scraper_endpoint branch September 12, 2023 06:31
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.

6 participants