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

DEV-1385 expose prometheus metrics for catalog indexing #57

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

moseshll
Copy link
Contributor

@moseshll moseshll commented Nov 4, 2024

  • Add push_metrics to services.
  • See README for ENV variables of interest.
  • Add support for checking the pushgateway for metrics in the appropriate indexer tests.
  • Add a postflight method for the indexer command, all it does right now is finalize the pushgateway job.
  • Randomly TIDY the Gemfile and docker-compose with some list alphabetization.

As mentioned in a ht_tanka PR, JOB_NAME is set to index_catalog in this branch but for consistency should be set to catalog_indexing, I think?

@moseshll moseshll marked this pull request as draft November 4, 2024 22:14
@coveralls
Copy link

coveralls commented Nov 4, 2024

Coverage Status

coverage: 92.552% (+0.004%) from 92.548%
when pulling c4941bd on DEV-1385_prometheus_metrics
into 01164c0 on main.

@moseshll moseshll marked this pull request as ready for review November 6, 2024 19:16
@moseshll moseshll requested a review from aelkiss November 6, 2024 19:16
Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

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

This all seems pleasantly straightforward especially in terms of the integration with each_record.

I'd be a bit inclined to default JOB_NAME to include the specific cictl subcommand being used -- the scenario I'm thinking about is wanting to ensure that ad-hoc runs don't stomp on or confuse the metrics from any more regular run.

docker-compose.yml Show resolved Hide resolved
- Move from `Services` to `BaseCommand` so the Thor `current_command_chain` can be used for this purpose.
- Changes to tests to accommondate variable job name.
@moseshll
Copy link
Contributor Author

moseshll commented Nov 8, 2024

@aelkiss Requesting a quick final once-over because I moved some things around to get at the cictl ... command via Thor current_command_chain. It de-clutters Services a little so I reckon that is okay.

@moseshll moseshll requested a review from aelkiss November 8, 2024 21:00
Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

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

All makes sense to me. We'll give it a try in the testing workflow.

@moseshll moseshll merged commit c767b0a into main Nov 8, 2024
1 check passed
@moseshll moseshll deleted the DEV-1385_prometheus_metrics branch November 8, 2024 21:41
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.

3 participants