search

python-microservices / pyms

Library of utils to create REST Python Microservices
https://python-microservices.github.io/home/
GNU General Public License v3.0
269 stars 45 forks source link

WIP: refactor(metrics): migrate from prometheus client to opentelemetry #187

Open alexppg opened 4 years ago

alexppg commented 4 years ago

With OT, it will be easier to switch backends when we want to.

It would solve the metrics part of this issue. https://github.com/python-microservices/pyms/issues/181.

alexppg commented 4 years ago

It mostly works, I'm only having one issue with the Histogram. Waiting until somebody answers, since the documentation is pretty poor. https://github.com/open-telemetry/opentelemetry-python/issues/1229

Agalin commented 4 years ago

It seems that OpenTelemetry has yet to define a way to run under Gunicorn / multiprocess environment (see https://github.com/open-telemetry/opentelemetry-python/issues/365) so I'd argue that for now metrics should support both direct Prometheus integration (with #179 fixes) and OpenTelemetry for different use cases (maybe simply as different services).

alexppg commented 4 years ago

I agree. I've been playing with OT for a couple of days and I believe that we should treat them, for now, as beta features, for both metrics and traces. It's still on very early days, it has some limitations.

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 1065

Changes Missing Coverage Covered Lines Changed/Added Lines %
pyms/flask/app/create_app.py 9 20 45.0%
pyms/flask/services/opentelemetry.py 0 62 0.0%
<!-- Total: 9 82 10.98% -->
Totals Coverage Status
Change from base Build 1055: -3.8%
Covered Lines: 1838
Relevant Lines: 1927
💛 - Coveralls
alexppg commented 4 years ago

The open-telemetry/opentelemetry-python#1229 issue is fixed, but I've found the next one. https://github.com/open-telemetry/opentelemetry-python/issues/1255

alexppg commented 4 years ago

Ok, so the issue is that Histograms still don't exist on OT with Prometheus backend. I guess I'll just leave them out until there's some support, so I can finish this MR and begin the others, which might be more useful.