Will this be usable in Fedora? Can we make it optional, add an option, and move the monitoring from api_v1.py to separate file?

Move this to decorator.

Rename to decision_exception_counter. Anyway, this can be created by decorator, right?

"performaces" => "performance"

This is actually "automatically" provided by Prometheus: https://github.com/prometheus/client_python#three-step-demo

Rebased.

I think you forgot to remove this.

You can import prometheus_client only if it's enabled (since it might not be even installed).

No, this is needed in the /metrics endpoint

I can do that... but if it is not installed nothing in this PR makes sense... I should remove the metrics endpoint (or return nothing if is not installed). And I should remove the decorators in the other endpoints, but this is not possible or we won't have any data for the metrics...

Also the Jenkins pylint is complaining because it cannot find the prometheus_client. I don't know how to fix it without removing everything. Ok, I can put a condition in the import library, but what about the decorators?

Could not this be sent when the application is setup? Also is there no other option to configure Prometheus but only by env vars?

i would just # noqa this.

@lholecek when you deploy the app it should get installed as all the rest of the libs.

@gnaponie It looks fine, just a one minor thing. It adds the /metric endpoint + deps but also adds the metrics. It should have been split into 2 commits. But i don't think its a big deal for now.

I tried to put it in the app set up, but there is a "order" importing issue. If you put it in the set up of the app this will be "executed" after the check for this env var, so it will fail.

The env var is needed "only" for the multiprocessing mode (that for us is mandatory...). I tried to put it in the conf, but I didn't manage to make it work. I'm favourable to other suggestions... I also don't like much this solution.

when you deploy the app it should get installed as all the rest of the libs.

@mcurlej I understand this. But Prometheus is not used in Fedora GW instance. No reason to use it there (prometheus_client should be optional dependency).

That also means that the code needs to be more modular so that monitoring stuff can be used only if it's enabled.

Eh, I'd vote just to keep theprometheus_client dep regardless.

• Since prometheus is the "preferred" monitoring solution for openshift applications, there's a good chance that the Fedora crew will set up an instance some day. I asked them in #fedora-admin today if they have thoughts about it. We'll see what they say.
• We're also moving to a model where we produce only one docker image, and deploy it to all our environments. This means that even if the code allowed prometheus_client to be optional, it would still be installed in that one docker image anyways. Given that, it seems like unnecessary work to put exception blocks and conditionals all around it.

Let's keep it as it is.

Eh, I'd vote just to keep the prometheus_client dep regardless.

OK with me, it's simpler, but I would also like still keep GW as modular as possible in future.

@gnaponie You put the dependency back into spec file if it'll be needed regardless the app configuration.

FYI, @kevin tells me there's a prometheus instance available in staging Fedora that we could populate: https://prometheus-openshift-metrics.app.os.stg.fedoraproject.org/

It's there as an experiment, but they're not really using it yet.

Thanks Ralph.

I've submitted the https://pagure.io/greenwave/pull-request/301 to fix the Jenkins job.

You can remove this now, since it the dependency is in spec file again.

You can remove python3-prometheus_client also from here.

Fix the name as for the variable above (..._counter).

I've put this one here because the Jenkins job was failing because it didn't find the dependence...

Rebased... again. This PR is never ending :)

s/occured/occurred/g

@lholecek rebased to fix pylint. Can you check? Thank you

I think, you have to specify this only once in the file.

+1

For the jenkins failure, try:

diff --git a/Jenkinsfile b/Jenkinsfile
index 1ed1a20..04d9d9d 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -51,7 +51,7 @@ node('fedora-28') {
     sh '''
     sudo dnf -y builddep greenwave.spec
     sudo dnf -y install python3-flake8 python3-pylint python3-sphinx \
-        python3-sphinxcontrib-httpdomain python3-pytest-cov python3-prometheus_client
+        python3-sphinxcontrib-httpdomain python3-pytest-cov
     '''
     /* Needed to get the latest /etc/mock/fedora-28-x86_64.cfg */
     sh 'sudo dnf -y update mock-core-configs'
diff --git a/greenwave.spec b/greenwave.spec
index 0eb39e8..d6d5d03 100644
--- a/greenwave.spec
+++ b/greenwave.spec
@@ -20,6 +20,7 @@ BuildRequires:  python3-requests
 BuildRequires:  python3-PyYAML
 BuildRequires:  python3-dogpile-cache
 BuildRequires:  python3-fedmsg
+BuildRequires:  python3-prometheus_client
 BuildArch:      noarch
 Requires:  python3-flask
 Requires:  python3-requests

Rebased.

@ralph @lholecek our Jenkins jobs are failing again with strange Java tracebacks. Can I merge this PR in your opinion? I would merge it.

our Jenkins jobs are failing again with strange Java tracebacks.

I think they should be working again as of today. :)