Decorator mode vs client wrapper

[LucBERTON]

Description

Investigate ways to implement this package through wrappers or decorators.

Solution

Client wrappers

from openai import OpenAI
from genai_impact import OpenAI

client = OpenAI()

response = client.chat.completions.create(
    model="gpt-3.5-turbo",
    messages=[...]
)

impacts = response.impacts
print(impacts)
# Outputs: Impacts(energy=XX, energy_unit='kWh', gwp=XX ...) 

Patch decorator

from openai import OpenAI
from genai_impact import impact_estimator

@impact_estimator()
def summarize(client: OpenAI, doc: str)
        response = client.chat.completions.create(
                model="gpt-3.5-turbo",
                messages=[...]  # prompt summarize document
        )
        return response.choices[0].message.content

if __name__ == "__main__":
        client = OpenAI()
        document = "Lorem ipsum..."
        summarize(client, document)
        # After running this function the impacts are dumped in a CSV file

Maybe Patchy project could be useful : https://pypi.org/project/patchy/

Considerations

N/A

Additional context

N/A

[samuelrince]

After trying to implement unit tests that do not require sending a real request (and thus do not require a valid API key for each provider) I understood that our current implementation is broken (sorry for that), BUT we have an opportunity to make the package even simpler!

An example of why it is broken:

from openai import import OpenAI
from genai_impact import OpenAI as GenAIClientForOpenAI

def make_a_demo_request(client):
    return client.chat.completions.create(...)

client_openai = OpenAI()
client_genay = GenAIClientForOpenAI()

make_a_demo_request(client_genai)       # Response will include impacts
make_a_demo_request(client_openai)      # Response will include impacts even if it is not expected.

That is because the wrapper we use in our version of the client is a global wrapper. Meaning that the official OpenAI client will have the same behavior as our client because it is being patched by our client. In fact, just importing the package genai_impact will enable the patch.

This is not an expected behavior, and I think we should be inspired by what openllmetry does with wrapt. Basically, they initialize and use "tracers" that implement the same kind of wrappers as we do, but for monitoring metrics. The API is quite simple:

from openai import OpenAI
from traceloop.sdk import Traceloop

Traceloop.init()

client = OpenAI()

client.chat.completions.create(...) # Traceloop will monitor this function call using the initialized wrapper from the `Traceloop.init()`.

I think it is best that we do not reinvent the wheel and just mimic their API to initialize our wrappers. Basically, we could have:

from openai import OpentAI
from genai_impact import GenAIImpact    # This name is really uggly #7 

GenAIImpact.init()

client = OpenAI()

client.chat.completions.create(...) # This call will return the usual response with the impacts included

I am still not clear on how to test this properly, but I think it can make the package even easier to implement and use, what's your opinion on this? @LucBERTON @aqwvinh

If we go that way, it is no longer the decorator mode vs client wrapper, but the "tracer mode". If, on the contrary, we don't go that way, we need much more reflection on how to build and override the API clients (potentially complex), and I don't think the decorator mode will be easy too...

[LucBERTON]

Nice that you realized that early.

I am not sure if I fully understand how the openllmetry tracers works.

Don't they have the same issue as we have? Once they Traceloop.init(), then any call to client.chat.completions.create() will also return the additionnal data ?

Can't we just move the wrap_function_wrapper in the init of our class ?

class OpenAI(_OpenAI):
    def __init__(self, **kwargs: Any) -> None:
        super().__init__(**kwargs)
        wrap_function_wrapper(
            "openai.resources.chat.completions", "Completions.create", chat_wrapper
        )

[samuelrince]

Don't they have the same issue as we have? Once they Traceloop.init(), then any call to client.chat.completions.create() will also return the additionnal data ?

Yes, technically, but is it an issue? I am saying that we could use the same behavior so that the user of genai_impact do not need to change any other imports for OpenAI, MistralAI, etc. Just one import and an init and that's it!

If we want to keep the clients (same with decorators) we need to change the way we patch the function and no longer use wrap_function_wrapper. I think doing it with a context manager can solve the issue, something like that:

class OpenAI(_OpenAI):

    def __init__(...): ...

    def __getattr__(self, item: name):
        with path("openai.resources.chat.completions", "Completions.create", chat_wrapper):
            getattr(self, name)

With the patch function from wrapt (or patchy) that should exist, I have not checked, but already did that for tests before (using context manager).

[aqwvinh]

I agree that it's nice of the user doesn't have to change their imports for OpenAI, etc. However I'm not familiar enough with a tracer mode

[LucBERTON]

I started working for anthropic tracers on branch feat/anthropic-tracer

It's not fully functionnal yet, feel free to work on this branch

[samuelrince]

Thx @LucBERTON I have pushed my updates on your branch as well. I have updated openai and mistralai and implemented tests (without sending a real query).

[samuelrince]

I think we need a mechanism to check if we haven't already instrumented a tracer or not to avoid doing two times in a row.

[adrienbanse]

Thanks @samuelrince, this explains why I wasn't able to execute my __main__.py after Anthropic's implementation

[adrienbanse]

I'll have a look at how openllmetry checks whether the tracer is already initialized

[adrienbanse]

@samuelrince A very simple fix would be to modify the Tracer class as follows

class Tracer:
    initialized = False
    @staticmethod
    def init() -> None:
        if Tracer.initialized:
            print("Tracer already initialized")
        else:
            init_instruments()
            Tracer.initialized = True

It's written in feat/tracer_init, I'll add tests soon if we go for this solution