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

Added ASGI application #512

Merged
merged 10 commits into from
Feb 19, 2020
Merged

Added ASGI application #512

merged 10 commits into from
Feb 19, 2020

Conversation

Skeen
Copy link
Contributor

@Skeen Skeen commented Feb 14, 2020

This PR introduces an ASGI counterpart to the current WSGI app.

The ASGI code itself is moved into a seperate module asgi.py, which is conditionally included as it is only valid in Python 3 (due to async/await calls).

@Skeen Skeen force-pushed the feature/asgi branch 2 times, most recently from 59bfb87 to 331199e Compare February 14, 2020 20:55
Signed-off-by: Emil Madsen <sovende@gmail.com>
@brian-brazil
Copy link
Contributor

I'd prefer not to have to maintain yet another copy of the exposition http handling for a subtly difference interface. Is there not some interface all of these can share?

@Skeen
Copy link
Contributor Author

Skeen commented Feb 14, 2020

I understand your hesitance to adapt yet another interface. The README file is already quite crowded.

As for whether there is a common interface all of these can share, I wanna say yes, there is the PEP-3333 standard WSGI and it's spiritual successor ASGI.

  • Supporting just these two should be enough, as any web-framework should be able to integrate with one or the other, depending on whether it's a synchronous or asynchronous framework.

My last though is that WSGI and ASGI should not be considered equal to the frameworks. As they are gateway interfaces and thus more general than specific frameworks.

@Skeen
Copy link
Contributor Author

Skeen commented Feb 14, 2020

Note: I have closed the PR for the Quart extension of the README file, I included it solely for completeness to have a flask-equivalent ASGI framework example.

@brian-brazil
Copy link
Contributor

It doesn't matter to me why this exists, it matters that we're already duplicating this code 3 times for various things that were popular at various times.

@Skeen
Copy link
Contributor Author

Skeen commented Feb 15, 2020

How would you feel about cleaning up, by deprecating the twisted code, removing the flask and python http documentation and going solely with the code+examples for the gateway interfaces?

The gateway interfaces produces production quality endpoints in standard defined ways, and the integrations are simple in most languages, either via routers, middleware or adapters.

For Flask/Django it's common practice to utilize WSGI middlewares, for Quart/Django it's common practice to utliize ASGI middleware. Twisted has a WSGIResource which can call any WSGI app, thus I don't see the need for the MetricsResource for twisted.

Signed-off-by: Emil Madsen <sovende@gmail.com>
@brian-brazil
Copy link
Contributor

Is it possible to get twisted to use one of the *sgi apis? That way it could be replaced by docs.

@Skeen
Copy link
Contributor Author

Skeen commented Feb 15, 2020

Yea, that is very much possible.

To use the make_wsgi_app as a twisted resource, simply replace MetricsResource(), with: WSGIResource(reactor, reactor.getThreadPool(), make_wsgi_app()).
Or use txasgiresource to do the same thing with the make_asgi_app, by replacing MetricsResource() with ASGIResource(make_asgi_app()).

The example in the README becomes:

from prometheus_client import make_wsgi_app
from twisted.web.wsgi import WSGIResource
from twisted.web.server import Site
from twisted.web.resource import Resource
from twisted.internet import reactor

root = Resource()
root.putChild(b'metrics', WSGIResource(reactor, reactor.getThreadPool(), make_wsgi_app()))

factory = Site(root)
reactor.listenTCP(8000, factory)
reactor.run()

Using WSGI, and:

from prometheus_client import make_asgi_app
from txasgiresource import ASGIResource
from twisted.web.server import Site
from twisted.web.resource import Resource
from twisted.internet import reactor
from functools import partial

root = Resource()
root.putChild(b'metrics', ASGIResource(lambda scope: partial(make_asgi_app(), scope)))

factory = Site(root)
reactor.listenTCP(8000, factory)
reactor.run()

Using ASGI and txasgiresource.

Either way all the framework specific code for twisted can be deprecated/removed.

Potentially an alias can be setup, such as:

from twisted.web.wsgi import WSGIResource
from twisted.internet import reactor
from . import exposition, REGISTRY
MetricsResource = lambda registry=REGISTRY: WSGIResource(reactor, reactor.getThreadPool(), exposition.make_wsgi_app(registry))

To maintain backwards compatability.

@brian-brazil
Copy link
Contributor

If we can do that then and avoid duplicating the parameter handing code, that sounds good.

@Skeen
Copy link
Contributor Author

Skeen commented Feb 16, 2020

I will do it when I can get around to it.

How do you feel about the MetricsHandler, and start_http_server + _ThreadingSimpleServer?

We could eliminate _ThreadingSimpleServer by making start_http_server an alias of start_wsgi_server. If we did that we could additionally deprecate MetricsHandler, and potentially rewrite it to wrap and call the WSGI app from make_wsgi_app? - How do you feel about it this?

@brian-brazil
Copy link
Contributor

MetricsHandler is used by users, so it should continue to work. I'm okay with switching around how start_http_server works as long as it can still handle concurrent scrapes.

Signed-off-by: Emil Madsen <sovende@gmail.com>
@Skeen Skeen force-pushed the feature/asgi branch 2 times, most recently from 3f4851f to 4aa2cf7 Compare February 17, 2020 20:01
Signed-off-by: Emil Madsen <sovende@gmail.com>
Signed-off-by: Emil Madsen <sovende@gmail.com>
Signed-off-by: Emil Madsen <sovende@gmail.com>
@Skeen
Copy link
Contributor Author

Skeen commented Feb 17, 2020

What do you think about this?

README.md Outdated Show resolved Hide resolved
prometheus_client/asgi.py Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
tests/test_wsgi.py Outdated Show resolved Hide resolved
prometheus_client/exposition.py Outdated Show resolved Hide resolved
Signed-off-by: Emil Madsen <sovende@gmail.com>
Signed-off-by: Emil Madsen <sovende@gmail.com>
Signed-off-by: Emil Madsen <sovende@gmail.com>
Signed-off-by: Emil Madsen <sovende@gmail.com>
@brian-brazil brian-brazil merged commit 56a8a53 into prometheus:master Feb 19, 2020
@brian-brazil
Copy link
Contributor

Thanks!

@pquentin
Copy link

pquentin commented Feb 27, 2020

@Skeen Thanks for this! It works really well, and I was able to add it to a FastAPI app like this:

from fastapi import FastAPI
import prometheus_client

app = FastAPI()
metrics_app = prometheus_client.make_asgi_app()
app.mount("/metrics", metrics_app)

@brian-brazil Can you please consider releasing a new version to PyPI? Thanks!

@denisSurkov
Copy link

Hi!

ASGI is really cool feature. Especially for fastapi app (see comment above). When are you going to release asgi version to PyPI?

@whg517
Copy link

whg517 commented May 12, 2020

I can't wait to use Prometheus in fastapi. But I noticed that prometheus_client has not released a new version. So I chose to starlette_exporter. At least for now, there are no functional issues. You can try it, too.
@denisSurkov @pquentin

@Skeen Skeen deleted the feature/asgi branch May 12, 2020 08:34
@pquentin
Copy link

@brian-brazil Thank you for the release! 🎉

@aLowMagic
Copy link

@Skeen Thanks for this! It works really well, and I was able to add it to a FastAPI app like this:

from fastapi import FastAPI
import prometheus_client

app = FastAPI()
metrics_app = prometheus_client.make_asgi_app()
app.mount("/metrics", metrics_app)

@brian-brazil Can you please consider releasing a new version to PyPI? Thanks!

could you please remain me that, how can i add a count metrics in the asgi mode? Thanks alot

@Skeen
Copy link
Contributor Author

Skeen commented Sep 7, 2021

could you please remain me that, how can i add a count metrics in the asgi mode? Thanks alot

The same way you'd do it for non-asgi:

from prometheus_client import Counter
c = Counter('my_failures', 'Description of counter')
c.inc()     # Increment by 1
c.inc(1.6)  # Increment by given value

@aLowMagic
Copy link

could you please remain me that, how can i add a count metrics in the asgi mode? Thanks alot

The same way you'd do it for non-asgi:

from prometheus_client import Counter
c = Counter('my_failures', 'Description of counter')
c.inc()     # Increment by 1
c.inc(1.6)  # Increment by given value

Thanks a lot!

@aLowMagic
Copy link

aLowMagic commented Sep 8, 2021

could you please remain me that, how can i add a count metrics in the asgi mode? Thanks alot

The same way you'd do it for non-asgi:

from prometheus_client import Counter
c = Counter('my_failures', 'Description of counter')
c.inc()     # Increment by 1
c.inc(1.6)  # Increment by given value

Hello master, it's me again~
I found that, in the multiprocess mode, if two fastapi(asgi) workers exists, their are two counter exists, and the data will like:
prome_pic

and then i need calculate the qps like:

sum(increase(context_app_calls_total{app_name=~".*", endpoint="/test", instance="0.0.0.0:8000", job="8000_metrics", method="get", status_code="200"}[1m])/60)

will i get correct result like this way? Thanks a lot

@Skeen
Copy link
Contributor Author

Skeen commented Sep 8, 2021

Hi @aLowMagic

I don't think this PR is the right place to ask for help like this, or to have these discussions, as it's unrelated to ASGI itself.
The challenge your facing is a more generic one of combining multiple parallel data-series, irrelevant to how the data series are collected, or which programming language is used to do that collection.

I would suggest you have a look at: https://prometheus.io/community/ - And seek help there :)

@aLowMagic
Copy link

Hi @aLowMagic

I don't think this PR is the right place to ask for help like this, or to have these discussions, as it's unrelated to ASGI itself.
The challenge your facing is a more generic one of combining multiple parallel data-series, irrelevant to how the data series are collected, or which programming language is used to do that collection.

I would suggest you have a look at: https://prometheus.io/community/ - And seek help there :)

Thanks a lot~

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.

6 participants