Skip to content

Adds healthchecks using the OKComputer gem #68

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

Merged
danschmidt5189 merged 1 commit into from
Nov 21, 2025
Merged

Adds healthchecks using the OKComputer gem #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ gem 'geoblacklight', '~> 4.4.2'
gem 'importmap-rails'
gem 'jbuilder'
gem 'jquery-rails'
gem 'okcomputer', '~> 1.19'
gem 'omniauth'
gem 'omniauth-cas', '3.0.0'
gem 'omniauth-rails_csrf_protection', '~> 1.0'
Expand Down
5 changes: 5 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ GEM
faraday-retry (2.2.1)
faraday (~> 2.0)
ffi (1.17.0-aarch64-linux-gnu)
ffi (1.17.0-arm64-darwin)
ffi (1.17.0-x86_64-darwin)
ffi (1.17.0-x86_64-linux-gnu)
ffi (1.17.0-x86_64-linux-musl)
Expand Down Expand Up @@ -281,13 +282,16 @@ GEM
nio4r (2.7.4)
nokogiri (1.16.7-aarch64-linux)
racc (~> 1.4)
nokogiri (1.16.7-arm64-darwin)
racc (~> 1.4)
nokogiri (1.16.7-x86_64-darwin)
racc (~> 1.4)
nokogiri (1.16.7-x86_64-linux)
racc (~> 1.4)
oj (3.16.7)
bigdecimal (>= 3.0)
ostruct (>= 0.2)
okcomputer (1.19.0)
omniauth (2.1.2)
hashie (>= 3.4.6)
rack (>= 2.2.3)
Expand Down Expand Up @@ -542,6 +546,7 @@ DEPENDENCIES
importmap-rails
jbuilder
jquery-rails
okcomputer (~> 1.19)
omniauth
omniauth-cas (= 3.0.0)
omniauth-rails_csrf_protection (~> 1.0)
Expand Down
13 changes: 13 additions & 0 deletions config/initializers/okcomputer.rb
View file
Open in desktop
Copy link
Member

@anarchivist anarchivist Nov 20, 2025

Choose a reason for hiding this comment

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

meta question: do we care about doing healthchecks for geoserver and spatial within geodata?

Copy link
Member Author

@danschmidt5189 danschmidt5189 Nov 20, 2025

Choose a reason for hiding this comment

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

Good suggestion! Yes, both are critical to GeoData's functionality and the checks should be pretty easy to write. I'll see what I can do.

Copy link
Member Author

@danschmidt5189 danschmidt5189 Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@anarchivist I worked through this a bit last night, and I think these checks are a big enough lift to warrant a separate PR. The crux is that unlike with the Solr URL, there's no easy way to get at the GeoServer and Spatial URLs worth monitoring. That data is in Solr, so either we query it at startup or check-time, or we inject it (e.g. ENV['CHECKS_GEOSERVER_URL'] and some conditional logic).

So there's quite a bit more decision-making there. I think we should get this basic configuration out now, then address GeoServer/Spatial in a follow-up PR.

What do you think?

Copy link
Member

@anarchivist anarchivist Nov 21, 2025

Choose a reason for hiding this comment

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

👍

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# initializers/okcomputer.rb
# Health checks configuration

OkComputer.logger = Rails.logger
OkComputer.check_in_parallel = true

# Check that DB migrations have run
OkComputer::Registry.register 'database-migrations', OkComputer::ActiveRecordMigrationsCheck.new

# Check the Solr connection
# Requires the ping handler on the solr core (<core>/admin/ping).
core_baseurl = Blacklight.default_index.connection.uri.to_s.chomp('/')
OkComputer::Registry.register 'solr', OkComputer::SolrCheck.new(core_baseurl)
5 changes: 5 additions & 0 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'okcomputer'

Rails.application.routes.draw do
mount Blacklight::Engine => '/'
root to: 'catalog#index'
Expand Down Expand Up @@ -40,4 +42,7 @@
concerns :gbl_downloadable
end
resources :download, only: [:show]

# Map OkComputer's /health/all.json to /health
get '/health', to: 'ok_computer/ok_computer#index', defaults: { format: :json }
Copy link
Member Author

@danschmidt5189 danschmidt5189 Nov 21, 2025

Choose a reason for hiding this comment

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

@awilfox @anarchivist So here's an alternative — we mount OKComputer with its default setting (all routes under /okcomputer) but map /health to the endpoint we actually want. This way we get the simple default OKC configuration but with the same /health behavior as our other apps. Thoughts?

Copy link
Member

@anarchivist anarchivist Nov 21, 2025

Choose a reason for hiding this comment

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

🎉

end
2 changes: 1 addition & 1 deletion solr/geodata-test/solrconfig.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@
the PingRequestHandler.
relative paths are resolved against the data dir
-->
<str name="healthcheckFile">server-enabled.txt</str>
<!-- <str name="healthcheckFile">server-enabled.txt</str> -->
Copy link
Member

@awilfox awilfox Nov 20, 2025

Choose a reason for hiding this comment

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

Does this need to remain commented or should it be removed?

Copy link
Member Author

@danschmidt5189 danschmidt5189 Nov 20, 2025

Choose a reason for hiding this comment

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

I see arguments for both, but I went with this because it minimizes the diff with the solrconfig.xml that ships with GeoBlacklight. If we deleted it, we'd also want to delete the preceding lines/comments.

Copy link
Member

@anarchivist anarchivist Nov 20, 2025

Choose a reason for hiding this comment

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

can you explain more about why are we changing this? more context: https://solr.apache.org/docs/8_0_0/solr-core/org/apache/solr/handler/PingRequestHandler.html

Copy link
Member Author

@danschmidt5189 danschmidt5189 Nov 20, 2025

Choose a reason for hiding this comment

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

That config requires the named file to exist in the data directory. If it's not there, then even though /admin/ping is configured it returns a 503. I figured this was the easiest way to get it working, the alternative being scaffolding that file into each core's datadir.

Note that Solr is firewall'd in staging / prod, so we're not opening things up any more than they currently are.

Copy link
Member

@anarchivist anarchivist Nov 21, 2025

Choose a reason for hiding this comment

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

👍 ; i think this is fine to go, then.

</requestHandler>

<requestHandler name="/analysis/field"
Expand Down
2 changes: 1 addition & 1 deletion solr/geodata/solrconfig.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@
the PingRequestHandler.
relative paths are resolved against the data dir
-->
<str name="healthcheckFile">server-enabled.txt</str>
<!-- <str name="healthcheckFile">server-enabled.txt</str> -->
</requestHandler>

<requestHandler name="/analysis/field"
Expand Down
19 changes: 19 additions & 0 deletions spec/requests/okcomputer_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
require 'rails_helper'

RSpec.describe 'OKComputer', type: :request do
it 'is mounted at /okcomputer' do
get '/okcomputer'
expect(response).to have_http_status :ok
end

it 'returns all checks at /health' do
get '/health'
expect(response).to have_http_status :ok
expect(response.parsed_body.keys).to match_array %w[
default
database
database-migrations
solr
]
end
end