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

Add support for tornado. #308

Closed
wants to merge 1 commit into from
Closed

Add support for tornado. #308

wants to merge 1 commit into from

Conversation

jmcarp
Copy link

@jmcarp jmcarp commented Sep 23, 2018

AFAICT this doesn't require anything silly, so should be acceptable based on conversation in #114.

[Resolves #114]

@jmcarp jmcarp force-pushed the tornado branch 2 times, most recently from 7578b3c to 0f87a9f Compare September 23, 2018 16:54
[Resolves prometheus#114]

Signed-off-by: Joshua Carp <jm.carp@gmail.com>
@brian-brazil
Copy link
Contributor

Could you re-use the WSGI handler rather than implementing something tornado specific? Then there's only docs to maintain.

@jmcarp
Copy link
Author

jmcarp commented Sep 24, 2018

Yes, but according to https://www.tornadoweb.org/en/stable/wsgi.html#running-wsgi-apps-on-tornado-servers,

WSGI is a synchronous interface, while Tornado’s concurrency model is based on single-threaded asynchronous execution. This means that running a WSGI app with Tornado’s WSGIContainer is less scalable than running the same app in a multi-threaded WSGI server like gunicorn or uwsgi. Use WSGIContainer only when there are benefits to combining Tornado and WSGI in the same process that outweigh the reduced scalability.

Which reminds me that I might also need async versions of generate_latest and collect so that slow collectors don't block the event loop. Which it sounds like don't belong in this library. Will think about this more.

@vribeiro1
Copy link

Are there any plans on merging this pull request? It's very useful for a project of mine.

@brian-brazil
Copy link
Contributor

There's not, but you're free to host it in another repository.

@luiscoms
Copy link

luiscoms commented Feb 7, 2019

+1

@luiscoms
Copy link

luiscoms commented Feb 8, 2019

It could be added in the documentation as an example

@FullDataAlchemist
Copy link

+1

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.

Add support for Tornado based exporter
5 participants