-
Notifications
You must be signed in to change notification settings - Fork 6.9k
[codex] add otel tracing #7844
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
[codex] add otel tracing #7844
Conversation
8d28453 to
48d2cf7
Compare
pakrym-oai
left a comment
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.
Should we add tests?
Without tests we'll quickly regress these in one of the refactorings.
48d2cf7 to
93543fc
Compare
updated. |
93543fc to
a41f8f5
Compare
jif-oai
left a comment
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.
Nice!!! Will be so helpful for debugging
| } | ||
|
|
||
| fn otel_event_manager( | ||
| fn otel_manager( |
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 test building function seems to be defined in different place. Should we have just one in the otel crate?
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.
will refactor this in follow up review, since it's not related to tracing itself.
pakrym-oai
left a comment
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.
Looks good, pending @jif-oai's comments.
c6eefdc to
93551c6
Compare
93551c6 to
9e8f98a
Compare
No description provided.