feat: dspy instrumentation enhancements #158
Conversation
axiomofjoy
force-pushed
the
dspy-module-retriever
branch
from
26966b0 to
fd39918
Compare
dosubot
bot
added
the
size:XL
...tation/openinference-instrumentation-dspy/src/openinference/instrumentation/dspy/__init__.py
Show resolved
Hide resolved
| SpanAttributes.OUTPUT_MIME_TYPE: OpenInferenceMimeTypeValues.JSON.value, | ||
| } | ||
| dict( | ||
| _flatten( |
There was a problem hiding this comment.
not sure I follow why flatten is need in these cases
There was a problem hiding this comment.
It's not needed in several cases, only in cases where the attribute values include a list of objects. I included everywhere for the sake of consistency, but can remove if it's confusing.
| _flatten( | ||
| { | ||
| OPENINFERENCE_SPAN_KIND: CHAIN.value, | ||
| INPUT_VALUE: _get_input_value( |
There was a problem hiding this comment.
nit: _get_input_value isn't actually generic is it? It's specific to predict right?
There was a problem hiding this comment.
I think it's pretty generic? The problem it is trying to solve is that the contents of args and kwargs depends on whether the user passed arguments positionally or via keyword arguments. That is a problem we'll face for every kind of span.
| span.record_exception(exception) | ||
| raise | ||
| span.set_attributes( | ||
| dict( |
There was a problem hiding this comment.
same as above, can't flatten just return a dict?
There was a problem hiding this comment.
_flatten is copy-pasted from our other instrumentation libraries. I think there's some value in keeping it the same in case we want to factor it out at some point.
| "usage": {"prompt_tokens": 39, "completion_tokens": 396, "total_tokens": 435}, | ||
| "system_fingerprint": None, | ||
| } | ||
| responses.add( |
| @pytest.fixture(scope="module") | ||
| @pytest.fixture() | ||
| def in_memory_span_exporter() -> InMemorySpanExporter: | ||
| return InMemorySpanExporter() | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| @pytest.fixture() |
There was a problem hiding this comment.
The instrumentor was not being properly uninstrumented for some reason.
Retriever.forwardModule.forwardviaModule.__call__ColBERTv2.__call__requests-mockforresponses, which has wider adoption