Add prometheus metrics by nss10 · Pull Request #75 · uc-cdis/gen3-workflow

nss10 · 2025-03-21T16:38:51Z

Link to JIRA ticket if there is one: MIDRC-857

New Features

Add prometheus metrics

github-actions · 2025-03-21T16:39:25Z

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

coveralls · 2025-03-21T22:10:00Z

Pull Request Test Coverage Report for Build 14802796595

Details

0 of 0 changed or added relevant lines in 0 files are covered.
36 unchanged lines in 4 files lost coverage.
Overall coverage increased (+1.1%) to 85.114%

Files with Coverage Reduction	New Missed Lines	%
app.py	3	94.34%
config.py	5	72.5%
auth.py	8	89.16%
routes/ga4gh_tes.py	20	83.33%

Totals
Change from base Build 14092288361:	1.1%
Covered Lines:	486
Relevant Lines:	571

💛 - Coveralls

nss10 · 2025-04-01T21:41:49Z

This shall be merged after this cloud automation PR -- uc-cdis/cloud-automation#2762

nss10 · 2025-04-01T21:52:26Z

bin/_common_setup.sh


 # Source the environment variables from the metrics setup script
-# source "${CURRENT_DIR}/setup_prometheus"
+source "${CURRENT_DIR}/setup_prometheus"


This is nice, but bin/_common_setup.sh was never called when the service was launched. Wanted to find out if we are planning to use this script to run the service in the future. If that is the case, then I think we can remove this section in cloud-auto PR

Let's try to match what's done in gen3-user-data-library if we can

Copy that! gen3-user-data-library uses a script called run.sh which initates the service, I will update that and invoke the shell script in the Dockerfile (here)[https://github.com/uc-cdis/gen3-workflow/blob/b3004eb0643f00ae5902fbf909ec80374b37b2e3/Dockerfile#L39C1-L41C27]

Copy that! gen3-user-data-library uses a script called run.sh which is designed to initiate the service, I have included that change and invoked the shell script in the Dockerfile here

paulineribeyre · 2025-04-10T15:08:51Z

bin/_common_setup.sh


 # Source the environment variables from the metrics setup script
-# source "${CURRENT_DIR}/setup_prometheus"
+source "${CURRENT_DIR}/setup_prometheus"


Let's try to match what's done in gen3-user-data-library if we can

docs/metrics.md

paulineribeyre · 2025-04-10T15:12:20Z

gen3workflow/metrics.py

+
+
+class Metrics(BaseMetrics):
+    def __init__(self, prometheus_dir: str, enabled: bool = True) -> None:


i think prometheus_enabled instead of just enabled would be better in case we want to expand the "metrics" class later... but maybe not worth updating cdis-python-utils...

I can create a PR with this change, but it might not be worth releasing a new version just for this.

gen3workflow/app.py

gen3workflow/config-default.yaml

gen3workflow/app.py

bin/_common_setup.sh

bin/setup_prometheus

paulineribeyre · 2025-04-16T21:51:17Z

bin/_common_setup.sh

+  echo "Loading environment variables from ${CURRENT_DIR}/.env"
+  PROMETHEUS_MULTIPROC_DIR=$(grep 'PROMETHEUS_MULTIPROC_DIR:' /src/gen3-workflow-config.yaml | awk -F': ' '{print $2}' | tr -d '"')


mm the config is only at /src/gen3-workflow-config.yaml for cloud-automation deployments. If you run the service locally for example, it's not there.

Also i don't see PROMETHEUS_MULTIPROC_DIR in the default config, should we add it there?

and i don't get the reference to .env since we don't use that here

Thanks!

This script is only used when launching the service in an environment with multiple workers using gunicorn. In that scenario, it’s mandatory to have PROMETHEUS_MULTIPROC_DIR set before the application starts.
For local development, we continue using python run.py, where the Prometheus directory is set during the initialization of the metrics object.

The value is already added in the config-default.yaml here and also in config.py here

The .env comment was an oversight -- I'll update that!

Gotcha, could you add a comment explaining that it's made to work with cloud-automation

Not sure how i missed it

bin/run.sh

paulineribeyre

@nss10 Could you make sure the last bullet in the ticket description is done before we merge this

gen3workflow/app.py

Add setup + dependencies and barebones metrics.py

e244c90

nss10 added 2 commits March 21, 2025 11:42

set additionalProperties to True

1140140

Update config-default.yaml

ec8bd64

nss10 added 5 commits March 27, 2025 13:14

Add api count metrics, add get_user_id method

47b0b11

Revert get_user_id with get_token_claims where exceptions are expected

236b812

Update app.include_router to app.mount for metrics

0771fa1

Add a debug log

2e3b0c7

Change metric name

2a1d4b8

nss10 force-pushed the feat/add_prometheus branch from 682c5f3 to 2a1d4b8 Compare April 1, 2025 18:09

nss10 commented Apr 1, 2025

View reviewed changes

nss10 marked this pull request as ready for review April 1, 2025 21:58

nss10 requested a review from paulineribeyre April 1, 2025 21:58

paulineribeyre reviewed Apr 10, 2025

View reviewed changes

nss10 and others added 8 commits April 15, 2025 17:19

Add run.sh and respond to PR comments

367f1e3

Update _common_setup setup_prometheus shell scripts

b3004eb

Add metrics unit test and update pyproject.toml

7dc52ec

Update poetry lock

e209cbb

Apply automatic documentation changes

021b0f0

Uncommenting database part for unit tests

afb1d51

Seperate setting up database to be able to use it only for tests

83dd428

Wrap metric tracking within a try-catchi

d16fba8

nss10 requested review from Avantol13 and paulineribeyre April 16, 2025 15:38

paulineribeyre reviewed Apr 16, 2025

View reviewed changes

nss10 added 2 commits April 17, 2025 10:13

Respond to PR comments

a6993f1

Fix unit test

2c6ba2c

nss10 requested a review from paulineribeyre April 17, 2025 16:35

paulineribeyre reviewed Apr 21, 2025

View reviewed changes

gen3workflow/app.py Outdated Show resolved Hide resolved

nss10 added 3 commits April 21, 2025 15:33

Update the position for _setup_db.sh in run.sh

9a4f81a

Remove #TODO

069412e

Add a comment regardign cloud-auto dependency

87565f2

nss10 requested a review from paulineribeyre May 2, 2025 20:15

Update comment

d3588fd

paulineribeyre approved these changes May 2, 2025

View reviewed changes

nss10 merged commit d2e6e28 into master May 2, 2025
8 checks passed

paulineribeyre deleted the feat/add_prometheus branch May 8, 2025 16:18



		class Metrics(BaseMetrics):
		def __init__(self, prometheus_dir: str, enabled: bool = True) -> None:

		echo "Loading environment variables from ${CURRENT_DIR}/.env"
		PROMETHEUS_MULTIPROC_DIR=$(grep 'PROMETHEUS_MULTIPROC_DIR:' /src/gen3-workflow-config.yaml \| awk -F': ' '{print $2}' \| tr -d '"')

Conversation

nss10 commented Mar 21, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Features

Uh oh!

github-actions bot commented Mar 21, 2025

Uh oh!

coveralls commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 14802796595

Details

💛 - Coveralls

Uh oh!

nss10 commented Apr 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nss10 Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nss10 Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulineribeyre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nss10 commented Mar 21, 2025 •

edited by atlassian bot

Loading

coveralls commented Mar 21, 2025 •

edited

Loading

nss10 Apr 16, 2025 •

edited

Loading

nss10 Apr 17, 2025 •

edited

Loading