-
Notifications
You must be signed in to change notification settings - Fork 641
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
Library for uploading blobs as part of instrumentation #GenAI #MultiModal #3065
Comments
Good things is we are not tracing api using blobs at the moment :) Anyway I think this must be discussed at the genai semconv call https://docs.google.com/document/d/1EKIeDgBGXQPGehUigIRLwAUpRGa7-1kXB736EaYuJ2M/edit?tab=t.0#heading=h.ylazl6464n0c before doing anything python specific |
Thanks. I'll join the SemConv call to discuss further. It's also recently come to my attention that there is another example of this in the |
Also, this has come up pre-otel and also for some in the ecosystem today. Here are couple notes of interest as "blob processing" isn't a new technique generally, and usually presents itself generically for http req/resp Q/A use cases which also have content types that can be huge, binary or both Here are a couple open source variants I'm aware of from the past |
Discussed in today's GenAI SIG meeting. Outcome of the conversation:
@lmolkova is this a fair summary? With regard to the last one of where / how to bind in lieue of |
What's the right place to get started? I'm thinking it would go something like:
... with:
I'm assuming the imports would look something like:
I'll confess I'm not terribly familiar with Thanks! |
I think we can define the interfaces and logic inside of the For actual blob uploader impls, we could either use optional dependencies ( |
Thanks, Aaron. I will get started on that shortly. |
I think due to us having separated out the topics, the frequently requested HTTP layer req/resp logging. Is it valid to steer folks interested in that, this way? |
That would seem reasonable to me. |
Started draft of pull request here: It's still a work in progress, but I figured a draft would be good in that it will make it easier to follow along as well as to leave comments during development. |
I still need to address some style and polish issues, but the PR is essentially there now in: |
I've reviewed #3122, I think there is a much simpler and more scalable approach we should take. |
Thank you for working on this @michaelsafyan! We've been working on something very similar at Logfire. We've thought hard about this and come to the realisation that it's not as easy as it seems. I'll share some off the cuff points from these conversations that I think are worth considering:
Personally I would like:
All of this said the design we've been iterating on internally looks like this: with logfire.span(
'asking {model}',
model='gpt',
prompt=TextBlob(text_data),
image=Blob(binary_data, content_type='png'),
)
... Or more in an OTEL SDK style: with tracer.span(
'asking llm',
attributes={'model': 'gpt'},
blobs={'prompt': TextBlob(text_data), 'image': Blob(binary_data, content_type='png')},
)
... (This can even be typed as blobs: list[Blob[Any]]`) The SDK would then decide if the attribute should actually be uploaded via a presigned URL or if it should just be sent inline, based on the size and type of the attribute in our case. When the data is blobbed what would be uploaded in its place would be an authenticated URL. To query it users will have to unblob the data: select *
from records
where unblob(attributes->>'prompt') like '%you are a professional%' Which will be a no-op for things uploaded inline or do efficient batch, parallel reconciling of data that is stored out of line in object storage. Because the data stored is authenticated URLs it's possible for:
And there are no issues with expiring presigned urls, etc. For the sampling issue the best I can think of is that the wire protocol support some way to sending a summary of sampled data. E.g. the collector sends a payload to |
Thanks @adriangb. I think sampling and memory are the most immediately problematic concerns which we need conceptual answers for before continuing work on #3122. My proposal:
|
Those are great suggestions to do something but yeah I fear those are questions we do need to answer. My suggestion is that a first step is to support binary data as attributes. That solves at least some use cases and should not be controversial at all, it just needs implementation work. |
@adriangb thanks for raising those issues. I agree that this is not GenAI specific. With respect to pre-signed URLs, I do think there is value in being able to actually hide the content from observability backend so that authorization is conferred out-of-band of the URL; for example, some of this data may be highly sensitive (more so than other observability data), and so separating auth from linking can actually be beneficial here. (Yes, if the backend is granted the auth, then being able to display inline and index, etc. is good; but it should also be possible to create linkage without granting access to the telemetry backend and without every user of the telemetry backend having access to the underlying content). With respect to sampling, I agree that this should be applied after the span is sampled. (And there should maybe even be a Making this a part of the protocol is an interesting thought. If the main concern, though, is having to modify multiple libraries in multiple languages, implementing in Go and adding to the Collector can cover pretty much all cases (by having other languages route to the Collector, which then would do the blob upload within the collector). Having to implement a blob upload endpoint is also a lot of work (in addition to being difficult organizationally in terms of soliciting agreement from all stakeholders and ensuring a protocol that everyone can agree on). |
@samuelcolvin thanks for those suggestions. Deferring the upload until sampling makes sense. I had assumed, perhaps incorrectly, that this code wouldn't effectively get called unless the span was being sampled. Do you have a particular mechanism in mind for how the sampling decision could/should get propagated into the uploader code? Collecting the data in temp files and uploading later also seems like a reasonable request. Happy to make these changes if there is agreement to proceed with the approach in principle. |
What problem do you want to solve?
Many (if not most) observability backends are not built to accept arbitrarily large blobs.
However, GenAI observability introduces a use case where users want to log full prompt/response data. In "multi-modal" use cases where large files (PDFs, PNGs, JPEGs, etc.) are involved, users still want these to be recorded. An example use case: the user prompts an LLM with a PDF and asks a question about it, asking to generate an infographic summarizing the essential details; the LLM responds with summary text as well as a JPEG infographic summarizing the salient points. GenAI Semantic conventions are aiming towards recording the prompt/response details in event bodies (which end up in logs), but many logging backends are not capable of receiving these large blob payloads.
In OpenLLMetry, this has been addressed with an
ImageUploader
, which is very specific to the Traceloop backend. It is likely that generic instrumentation of GenAI frameworks (such as moving frameworks from OpenLLMetry to the OTel Python repo) may require a more generic alternative to provide such capability/functionality.Describe the solution you'd like
At a high-level, I would like to separate this out into:
Common
NOT_UPLOADED
A constant used as a response when failing to upload.
Blob
Encapsulates the raw payload together with associated properties:
The
from_data_uri
function can construct aBlob
from a URI likedata:image/jpeg;base64,...
(or other, similar, base64-encoded data URIs).Consumption Interface
get_blob_uploader
A function used in instrumentation to retrieve the uploader for a certain context/usage:
This allows different uploaders to be configured for different contexts/usages (think the
logger
library in Python). It always returns an uploader of some kind (falling back to a default that reports an error and drops the data).BlobUploader
Provides a way to upload data asynchronously, getting the URL to which the data is expected to land.
The
upload_async
function returns quickly with a URL where the data is expected to get uploaded and, in the background, will attempt to write the data to the returned URL. May returnNOT_UPLOADED
if uploading is disabled.Helpers:
detect_content_type
,generate_labels_for_X
If instrumentation does not know the content type, it may use the following to generate it:
Instrumentation should use helpers such as the below to populate a minimum set of labels:
The above ensures that
labels
includes a minimum subset of metadata needed to tie back to the original source.Provider Interface
SimpleBlobUploader
Base class that providers can use to implement blob uploading without dealing with asynchronous processing, queuing, other associated details.
blob_uploader_from_simple_blob_uploader
Method that constructs a
BlobUploader
given the simpler interface:Default implementations
Ideally, it should be possible to specify the following environment variables:
BLOB_UPLOAD_ENABLED
BLOB_UPLOAD_URI_PREFIX
... and to have things work "out of the box" where
BLOB_UPLOAD_ENABLED
is true and whereBLOB_UPLOAD_URI_PREFIX
starts with any of the following:It should be easy for additional providers to be added, in support of additional prefixes.
Conventions
The above proposal is independent of conventions for actually putting this into use/practice.
I had originally envisioned a more expansive set of conventions for this (Blob Reference Properties), but I think it's also fine that narrower-scoped conventions are defined where needed (e.g. within GenAI, for a specific GenAI event type, proposing that a specific event body field be a URI referencing content that has been uploaded). This proposal is focused primarily on the technical details of how such uploading be performed and less on when to do so.
I'd ideally like to keep these two things orthogonal, focusing in this proposal on enabling the capability of having such properties come into existence and, separately, define when we take advantage of this capability.
Describe alternatives you've considered
Not separating out "SimpleBlobUploader" and "blob_uploader_from_simple_blob_uploader"
This would likely lead to duplication of effort related to handling of async processing, however. It might also make it difficult to centrally control configuration related to the async processing such as the size of the queue, the number of threads to dedicate to this background processing, whether to use threads or processes, etc.
Not providing "labels"
This would make it hard to include metadata in the upload that makes it possible to link from a blob to the observability that generated it. For example, suppose that GCS, S3, Azure Blob, or some other provider wanted to enable navigation from the blob in that system to the trace, span, event, etc. that generated the blob; this would be difficult without metadata.
Naming this "ImageUploader"
Although this is what OpenLLMetry calls it, the intended usage is broader in scope.
Naming this something that includes "GenAI"
Although GenAI instrumentation is the motivating use case, this could in theory be useful for other uses. Additionally, while this is for GenAI O11y, this does not make use of GenAI in its implementation which might be suggested by such a name.
Making it strictly synchronous
This is not likely going to fly in instrumentation, where blocking on the upload would introduce excessive latency.
Not having a
use_case
parameter toget_blob_uploader
In the short-term, this would be OK. But this could preclude future enhancements such as to dispatch to different uploader implementations depending on the caller/user of the library. For example, when debugging, it may be useful to capture only a portion of uploads to a local folder while otherwise not injecting/modifying uploads in general. Having this kind of parameter may help to implement such a use case down the line.
Raising an exception instead of returning
NOT_UPLOADED
This is a viable alternative approach. However, I imagine that this could lead to accidental failure to handle the exception, whereas recording "/dev/null" as the value of a property indicating that the data never got uploaded is more likely to be OK. In this case, I'm erring on the side of failing gracefully rather than failing loudly (since the severity of the failure is not significant and can mostly be ignored).
Providing a way to be notified when the upload completes or if it fails
I think this could easily be added in the future without breaking changes, but it creates extra complexity. I'd prefer to omit this piece from an initial solution, since I think this complexity will have little return-on-investment.
Additional Context
No response
Would you like to implement a fix?
Yes
The text was updated successfully, but these errors were encountered: