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

Fix loading gem due missing PrometheusExporter constant #224

Merged
merged 4 commits into from
Mar 4, 2022

Conversation

theseanything
Copy link
Contributor

  • Fix issue where GovukPrometheusExporter module prevented the gem to load due to missing constant. It was unable to find the top level module name "PrometheusExporter" and need to be explicity required.

  • Lazy load the prometheus_exporter dependency for only apps that use GovukPrometheusExporter. This prevents the library from being required by apps the don't need it.

  • Load GovukPrometheusExporter only for Rails apps, as it loads middleware into Rack and explicitly depends on Rails being present.

There was a bug were the prometheus_exporter gem was incorrectly required
by the module. It was unable to find the top level module name
"PrometheusExporter" and need to be explicity required.
This delays the require of prometheus_exporter until the configure
method is called. This prevents the library from being required by apps
the don't need it.
The initialiser modules loads middleware into Rack and explicity depends
on Rails being present.
Copy link
Contributor

@marcpomfret marcpomfret left a comment

Choose a reason for hiding this comment

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

Thanks for sorting this.

@theseanything theseanything merged commit 1208816 into main Mar 4, 2022
@theseanything theseanything deleted the fix-loading-govuk-prometheus-exporter branch March 4, 2022 16:23
kevindew added a commit to alphagov/manuals-publisher that referenced this pull request Mar 7, 2022
This resolves the build errors for version 4.4.0. See: alphagov/govuk_app_config#224
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.

2 participants