Skip to content

add prometheus_exporter gem and feature flag#10287

Merged
timothy-spencer merged 3 commits intomainfrom
tspencer/prometheus_exporter
Mar 22, 2024
Merged

add prometheus_exporter gem and feature flag#10287
timothy-spencer merged 3 commits intomainfrom
tspencer/prometheus_exporter

Conversation

@timothy-spencer
Copy link
Contributor

@timothy-spencer timothy-spencer commented Mar 22, 2024

🛠 Summary of changes

This enables prometheus_exporter functionality in the idp, behind a feature flag, which is off by default.

It also updates the cbor gem version, since it would not compile on my machine.

This functionality is to support metrics gathering that we can use in blue/green deploys.

📜 Testing Plan

  • There is a test for the feature flag built in.
  • I have tested the functionality by hand by running the idp with the feature flag enabled but the prometheus_exporter not running, and it logged errors saying it could not contact the prometheus_exporter, then disabled the feature flag, and the idp then did not log errors.
  • I also tested by looking for metrics in the exporter when the feature flag was enabled/disabled, and there were http metrics when it was enabled, but none when disabled.
  • I asked Mitchell to try running this too, and he said it worked!

Comment on lines +106 to +109
def self.prometheus_exporter?
!Rails.env.test? && IdentityConfig.store.prometheus_exporter
end

Copy link
Contributor

Choose a reason for hiding this comment

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

instead of adding logic like this, would it be easier to just update the test: stanza of the sample application.yml to be false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, in the code example (https://github.com/discourse/prometheus_exporter?tab=readme-ov-file#rails-integration), they seem to not want it to be enabled for test stuff, so I was just copying what they were doing. I also was using the pattern I found in the log_to_stdout feature above it to implement that logic.

I'm not sure it's easier or not, but I was trying to do things the way other things are done. Would you like me to change this?

@zachmargolis
Copy link
Contributor

Does this need any configuration of where to export metrics to?

@timothy-spencer
Copy link
Contributor Author

Does this need any configuration of where to export metrics to?

Nope. It will export to localhost:9394, which is where the local exporter will run. You can see one being fired up in the Procfile, but in k8s, we'll make them be a sidecar. There will be no centralized exporter service, because at scale, it would fall over. We might need to add some special tags to the metrics so we know what version the code is or something, but I'll deal with that as we figure out what the best way to do the blue/green stuff is.

@timothy-spencer timothy-spencer merged commit 2077353 into main Mar 22, 2024
@timothy-spencer timothy-spencer deleted the tspencer/prometheus_exporter branch March 22, 2024 18:05
@aduth
Copy link
Contributor

aduth commented Mar 25, 2024

I noticed this doesn't shut down very gracefully when Ctrl+C'ing to stop the server:

08:05:25 prometheus-exporter.1 | /gems/prometheus_exporter-2.1.0/bin/prometheus_exporter:126:in `sleep': Interrupt
08:05:25 prometheus-exporter.1 | 	from /gems/prometheus_exporter-2.1.0/bin/prometheus_exporter:126:in `run'
08:05:25 prometheus-exporter.1 | 	from /gems/prometheus_exporter-2.1.0/bin/prometheus_exporter:129:in `<top (required)>'
08:05:25 prometheus-exporter.1 | 	from /bin/prometheus_exporter:25:in `load'
08:05:25 prometheus-exporter.1 | 	from /bin/prometheus_exporter:25:in `<top (required)>'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli/exec.rb:58:in `load'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli/exec.rb:58:in `kernel_load'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli/exec.rb:23:in `run'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli.rb:451:in `exec'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor/command.rb:28:in `run'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor.rb:527:in `dispatch'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli.rb:34:in `dispatch'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor/base.rb:584:in `start'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli.rb:28:in `start'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/exe/bundle:28:in `block in <top (required)>'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/exe/bundle:20:in `<top (required)>'
08:05:25 prometheus-exporter.1 | 	from /bin/bundle:25:in `load'
08:05:25 prometheus-exporter.1 | 	from /bin/bundle:25:in `<main>'

@mitchellhenke
Copy link
Contributor

I noticed this doesn't shut down very gracefully when Ctrl+C'ing to stop the server:

08:05:25 prometheus-exporter.1 | /gems/prometheus_exporter-2.1.0/bin/prometheus_exporter:126:in `sleep': Interrupt
08:05:25 prometheus-exporter.1 | 	from /gems/prometheus_exporter-2.1.0/bin/prometheus_exporter:126:in `run'
08:05:25 prometheus-exporter.1 | 	from /gems/prometheus_exporter-2.1.0/bin/prometheus_exporter:129:in `<top (required)>'
08:05:25 prometheus-exporter.1 | 	from /bin/prometheus_exporter:25:in `load'
08:05:25 prometheus-exporter.1 | 	from /bin/prometheus_exporter:25:in `<top (required)>'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli/exec.rb:58:in `load'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli/exec.rb:58:in `kernel_load'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli/exec.rb:23:in `run'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli.rb:451:in `exec'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor/command.rb:28:in `run'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor.rb:527:in `dispatch'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli.rb:34:in `dispatch'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/vendor/thor/lib/thor/base.rb:584:in `start'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/cli.rb:28:in `start'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/exe/bundle:28:in `block in <top (required)>'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors'
08:05:25 prometheus-exporter.1 | 	from /gems/bundler-2.5.6/exe/bundle:20:in `<top (required)>'
08:05:25 prometheus-exporter.1 | 	from /bin/bundle:25:in `load'
08:05:25 prometheus-exporter.1 | 	from /bin/bundle:25:in `<main>'

I think we could remove it from the Procfile.

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.

4 participants