-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add OTLP log endpoint to convert log events to span annotations #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty cool to do real integration tests (with python and js), though maybe it could be less maintenance to send these signals directly in the integration test with otel java libs.
I don't recall the current state of java and the otel logs api, but yeah pretty sure it can be done directly.
This sort of change is optional until maintaining the examples becomes troublesome. Also, we can drop one of the example languages if the other becomes a problem.
Aside: one configuration I would expect to come quickly is to use solely the "event.name" as the annotation value, so like probably via ENV.
Finally, seems like this is really getting to the point where we need a README, to describe how each signal is translated. e.g. traces (translated) metrics (dropped) logs (recorded as annotations if have a span context and event name)
@Consumes("application/json") | ||
@Produces("application/json") | ||
public Map<String, Object> handleChatCompletion(String requestBody) { | ||
Map<String, Object> response = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure it works, but prefer Map.of
stuff if it is usable. that or just a multi-line json literal. We can use a later JDK in tests than we do for the compiled binary (floor JRE version). This can be done later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this project is compiled with Javav 8, probably just as it was when you scaffolded it.
Is there any reason to use this version? Would there be any problems with changing the base to Java 17?
addressed in cb7a0dc
When I checked, it didn't seem to be supported in Java. If it's supported at the next time I review it, I'll switch to the Java SDK version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first pull request that fixes gh-21.
More use case tests, optimizations and options will be added later.
How to check UI locally
Run the example script using OpenAI+Python
Run the example script using OpenAI+NodeJs
a screenshot and trace data is attached here