Minor changes to GenAi utils lib

[DylanRussell]

Description

Add a helper function for serializing Any types, use it to serialize arguments and result which are Anys before putting them in span attributes..

Fixes # (issue)

• [ x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Unit tests

Does This PR Require a Core Repo Change?

• Yes. - Link to PR:
• [ x] No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• [x ] Followed the style guidelines of this project
• [x ] Changelogs have been updated
• [ x] Unit tests have been added
• [x ] Documentation has been updated

[lmolkova]

we should not need it in the long term - any is a valid span attribute type, we just don't support it yet in python. should we keep this method in the libs that need it?

[DylanRussell]

well anyone that uses this will need it until that change goes through.. is there an open bug or something that I can put here to say we will eventually remove this ?

[lmolkova]

it's not used today by existing instrumentations - it's done internally via

opentelemetry-python-contrib/util/opentelemetry-util-genai/src/opentelemetry/util/genai/_invocation.py

Line 179 in 5caa9ac

def get_content_attributes(

removing public api in the future is costry, so if there is 1-2 spots where it's actually needed copy-paste is better than future breaking changes.

[DylanRussell]

Right that code serializes everything to a string -- that's the same thing I'm doing here -- it sounds like you are saying we should not be doing that IIUC ? but instead do it in the instrumentation libraries so people don't get the wrong idea ?

[lmolkova]

I'm saying that most instrumentation libs don't need this method to be public in the utils. And those who do migh be better with having this logic internal as well so that they won't suffer from utils removing this method in the future.

My main worry is utils becoming a pile of disjoint helpers (what usually happens with utils) which is impossible to clean up or consolidate because it's breaking everyone. If things are internal it's much of a smaller problem to solve.

[DylanRussell]

Ok that's fair.. Can you take a look at the updates i've made ? The issue is I need to serialize an Any. For now I just ended up using the existing utility to do it.

[DylanRussell]

BTW I updated the code to make the GenAiInvocation baseclass itself a context manager, this way we don't need 2 methods to construct the invocations.. we just directly return the invocation via the 1 constructor method, and users can optionally use that as a context manager.. similar to how file object works in python

[DylanRussell]

Ok the context manager change probably needs to be done in 2 steps:

A: Update GenAi Invocation class to be a ContextManager, release new GenAI package..
B: Update all instrumentations to stop relying on the start_xxx methods, and remove all the start_xxx methods..

I suppose there's no way around this ? It's a pain we always need to do these 2 step changes..

[lmolkova]

removing public API is a pain. let's deprecate to avoid breaking everyone. It's impossible today to make different genai libs work in the same app because each of them needs different version of utils.

[DylanRussell]

Why can't we just update all the instrumentations to stop using these functions ? It's not that bad.. It's a pain to maintain 2 versions of each function here. It's easy to make a mistake

[lmolkova]

sorry @DylanRussell could you please reopen it against https://github.com/open-telemetry/opentelemetry-python-genai ?

Also, let's avoid removing public API that are actively used.

[DylanRussell]

Why can't we just update all the instrumentations to stop using these functions ? It's not that much work.. It's a pain to maintain 2 versions of each function here. It's easy to make a mistake

[lmolkova]

Why can't we just update all the instrumentations to stop using these functions ? It's not that much work.. It's a pain to maintain 2 versions of each function here. It's easy to make a mistake

because it is a breaking change to users. They update one (e.g. google genai) and don't update another (e.g. langchain) and now there is no version of utils that works for both. Note: we don't even release libs together, there could be literally no possible version of utils that works with both.

It's easy to update new versions and impossible to update old versions. The path forward: deprecate unnecessary methods, update usages, remove them in the future (e.g. in 6 months).

[DylanRussell]

I'm proposing we would do new releases of all the instrumentations w/ a new version of Utils, so the user could get a working version by updating both langchain / Google genai to the latest version.

Is it better to do a deprecation (in which case the user will start getting a deprecation warning for a while..) or just force them to do a upgrade ?? Is there a downside to forcing a user to upgrade ?

Also WDYT of using types.AttributeValue instead of Any for Tool Arguments and Tool Result, in order to force instrumentations to pass the right type, and not do the json.dumps serialization here which can fail if something can't be serialized ?? And we can switch that over to be types.AnyValue whenever python gets around to making that change (we should revisit your PR: open-telemetry/opentelemetry-python#4587 -- I thought that was good to be merged !)

[DylanRussell]

Also I feel like the Gen AI sem convs should describe how to serialize things, if it isn't obvious. How am I supposed to serialize function arguments/result into a span attribute value ? The sem convs just say these types are "Anys" -- what does that mean ?

[DylanRussell]

Alright anyway lets continue this discussion on open-telemetry/opentelemetry-python-genai#17 -- i'm going to close this.