-
Notifications
You must be signed in to change notification settings - Fork 4
feat: initial commit of the openlit translator #93
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Pavan Sudheendra <pavan0591@gmail.com>
Signed-off-by: Pavan Sudheendra <pavan0591@gmail.com>
…op translator Signed-off-by: Pavan Sudheendra <pavan0591@gmail.com>
Signed-off-by: Pavan Sudheendra <pavan0591@gmail.com>
Signed-off-by: Pavan Sudheendra <pavan0591@gmail.com>
| _DEFAULT_ATTR_TRANSFORMATIONS = { | ||
| "rename": { | ||
| # OpenLit uses indexed content format, OTel uses structured messages | ||
| "gen_ai.completion.0.content": "gen_ai.output.messages", |
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.
Are we sure there will only be one completion response all the time? I mean would there be gen_ai.completion.1.content and so on?
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.
Good point. Usually it's one, but there could be more. will update the code to handle it
| return result | ||
|
|
||
| # Install the wrapper | ||
| trace.set_tracer_provider = wrapped_set_tracer_provider |
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.
I am concerned if both translator packages are installed one will override the other. I am not sure if both translators will be installed at the same time, but if they are then this a problem.
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.
The underlying framework could either be traceloop or openlit but not both right? So, for a single app, I doubt both will be installed. We could exit or warn the user about the presence of the other translator package.
Thoughts?
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.
maybe I am overthinking, but the execution environment could have both the packages installed and might lead to unexpected results. Maybe a warning or handling the wrapping in a thread safe manner and using wraps from functools?
| rules_spec = data.get("rules") if isinstance(data, dict) else None | ||
| if not isinstance(rules_spec, list): | ||
| logging.warning( | ||
| "[TL_PROCESSOR] %s must contain a 'rules' list", _ENV_RULES |
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.
nit: OpenLit_PROCESSOR?
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.
fixed
|
some test files names have to be updated -> replace |
Signed-off-by: Pavan Sudheendra <pavan0591@gmail.com>
Signed-off-by: Pavan Sudheendra <pavan0591@gmail.com>
Example trace dump of the resulting spans after running the example: