Instrumentation for Lanchain4j's OpenAI Chat Model#24
Instrumentation for Lanchain4j's OpenAI Chat Model#24Andrew Kent (realark) merged 1 commit intomainfrom
Conversation
8db59a5 to
8c94b9c
Compare
8c94b9c to
f09d73b
Compare
| @SneakyThrows | ||
| void testSyncChatCompletion() { | ||
| // Mock the OpenAI API response | ||
| wireMock.stubFor( |
There was a problem hiding this comment.
TODO: my next quality-of-life change will be to switch out stubs for VCR-like replay (wiremock supports this apparently). Some time in the next week or two
David Elner (delner)
left a comment
There was a problem hiding this comment.
Some questions but nothing blocking!
| public record Options(String providerName) {} | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private static <T> T getPrivateField(Object obj, String fieldName) |
There was a problem hiding this comment.
This question is just for my own education: it looks like we're using reflection to access the private fields in order to instrument them, correct? What are the performance/stability risks associated with reflection? Are there other practical alternatives for instrumentation?
In the Ruby world, we generally would avoid accessing private fields because of the potential for instability (e.g. someone in a patch version changes the API.)
There was a problem hiding this comment.
Yes that's right, we're using reflection. There isn't much risk in this case because we'll just fail to apply instrumentation if something goes wrong
Performance is pretty good with reflection, but even if it wasn't this is only done once during client build
There isn't a viable alternative right now, but once we get into auto instrumentation for java we'll have more options
| Span span = startNewSpan(getSpanName(providerInfo)); | ||
| try (Scope scope = span.makeCurrent()) { | ||
| tagSpan(span, request, providerInfo); | ||
| final long startTime = System.nanoTime(); |
There was a problem hiding this comment.
What is nanoTime? Is it wall clock time or is it something else?
There was a problem hiding this comment.
Basically wall clock time. It's an increasing nanosecond counter from an arbitrary starting point
| + " after %d attempts", | ||
| minSpanCount, spans.size(), attempts)); | ||
| } | ||
| Thread.sleep(1000); |
There was a problem hiding this comment.
Why do you need to wait & sleep? To read off the OTel thread? Is there a faster, more directly way to do this synchronously in the test suite?
There was a problem hiding this comment.
Usually waiting isn't needed but some of the streaming tests finish their spans after this method is invoked for the first time
I feel like there should be a better way to do this, but the only gotcha is I'm using the built in otel utils to collect spans so I'm not sure what hooks I would have to insert concurrency signaling stuff
I'm making some other changes to the test harness in another branch. I'll add this to that work. At the very least I can dial down the sleep time (10ms should be plenty)
No description provided.