-
Notifications
You must be signed in to change notification settings - Fork 174
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
feat(output): composite output handling #458
Conversation
… output management
Hello, |
Hello, thanks for reporting your use case! |
601a95a
to
3bab18c
Compare
The main problem is, standard management of prometheus metrics goes through in-memory representations of counters, gauges, etc. This is how it works in abstractions like micrometer, and this is how it is implemented in BentoML as well. My understanding is that prometheus currently supports an official library in python that does just that, it manages metrics representation and can expose them in a variety of ways, but it doesn't look like it has that much traction at the moment, and at the very least, it is not how metrics were implemented in a BentoML application. So we cannot rely on a "standard" prometheus implementation of this use-case. On the other hand, it doesn't make any sense for codecarbon to have a native integration with the BentoML metrics API. Enabling composition-based expansion points however, would allow this use-case, and many more. |
I made a separate "demo" PR of a relatively non-invasive way of unifying "live" output handler executions. Output: unifying execution of 'live' output handlers #459 But even writing this, it looks like we could go even further and add some kind of flag or wrapper around |
Thanks for providing this example. cc @inimaz |
If I understand correctly, you plan on leveraging the fact that BentoML's implementation of prometheus metrics is backed by the official client, so in theory as long as the client uses a single registry for all its metrics, manipulating the client directly should result in CodeCarbon metrics to show up in BentoML's I'm still partial to having the ability to expand output strategies in a more flexible manner, but for our current use-case I could see that working, yes :-) |
Hello, |
If you go that way indeed it would probably make sense to start leveraging the common abstraction and work by compositing However, in the initial PR I made an attempt to not introduce breaking changes in the current behaviour, depending on your stance on API evolution this could be the right moment for deprecating the various output-specific configuration options, and make it so instead of something like (this is pseudo-code): emission_tracker = EmissionTracker(
enable_prometheus=True,
prometheus_gateway_endpoint="http://localhost:9000/api",
enable_logging=True,
logging_level=INFO,
#...
) instead it would be entirely determined through composition such as: emission_tracker = EmissionTracker(
outputs=[
PrometheusOutput(gateway="http://localhost:9000/api"),
LoggingOutput(logging_level=INFO),
CSVOutput(),
CustomOutput(),
#...
]
) This would probably make things level but may require some more refactoring work, and preserving back-compatibility may end up being confusing. |
I'm personally okay with that, it extends the tracker capabilities and we could do this addition without breaking the current usage. |
I like this! That could potentially solve as well #541. Just to keep in mind, since we are letting the user save all the config in a
|
Closing this as this has become redundant with #459 |
Hi there,
This one needs some context as to what is being attempted, because from what I could tell our use-case didn't quite fit anywhere in the code base as it currently stand.
We are considering using codecarbon for systematic carbon measurements in a model serving context. The gist of it is we have fleets of BentoML-based applications and we would like to make it so all of them measure their carbon emissions and expose them in quasi-real-time through their
/metrics
prometheus endpoint (which exists by default for all BentoML apps).From what I could gather from the codebase, there already is a "Prometheus integration" in codecarbon but it works with a push gateway, which we absolutely do not want to introduce as it would turn the carbon emission tracking from an opt-out, to an opt-in with additional deployment requirements. Basically, we want the carbon footprint to be tied to existing performance metrics that will be collected for typical monitoring needs.
So, working on how to integrate codecarbon measurements with BentoML metrics reporting, I found that while there is a
BaseOutput
contract that can be extended, the tracker implementations do not allow much in the way of composition.What I did here is naively introduce an arbitrary list of
BaseOutput
implementations expected to be triggered at the same time the API calls are triggered. This way, we can plug a custom output implementation that performs whatever is needed with BentoML's implementations of metric counters, gauge, etc. But, from what I can tell this isn't quite in the spirit of the current codebase.So, what do you think?
From my point of view the more abstract composition model is preferable in every scenario, but I understand it would break compatibility with earlier versions to simply drop the current settings for API and Prometheus. Adding generic composition on top of the current settings looks odd, but at least it's transparent.