Skip to content

Logging#1146

Merged
sonichi merged 92 commits into
microsoft:mainfrom
cheng-tan:logging
Feb 15, 2024
Merged

Logging#1146
sonichi merged 92 commits into
microsoft:mainfrom
cheng-tan:logging

Conversation

@cheng-tan

@cheng-tan cheng-tan commented Jan 4, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Why are these changes needed?

Adding the logging capability for openai >= 1, 4 tables get logged:

  • chat_completions: tracking each llm run
  • agents: tracking the agent's name, type, configurations
  • oai_client: tracking the AzureOpenAI or OpenAI client configurations
  • oai_wrapper: tracking OpenAIWrapper configurations

Related issue number

#7

TODO:

  • track cost in db
  • track session id
  • pass in db name as argument
  • start logging/stop logging
  • logging oai wrapper in a new table
  • logging oai client in a new table
  • improve error handling
  • update doc
  • tests
  • versioning db schema?
  • option to dump logging to console

Checks

@codecov-commenter

codecov-commenter commented Jan 4, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: 58 lines in your changes are missing coverage. Please review.

Comparison is base (f68c09b) 37.85% compared to head (9adf462) 61.92%.

Files Patch % Lines
autogen/logger/sqlite_logger.py 74.19% 23 Missing and 9 partials ⚠️
autogen/logger/base_logger.py 70.96% 8 Missing and 1 partial ⚠️
autogen/oai/client.py 71.42% 4 Missing and 2 partials ⚠️
autogen/logger/logger_factory.py 66.66% 2 Missing and 2 partials ⚠️
autogen/runtime_logging.py 90.90% 1 Missing and 2 partials ⚠️
autogen/agentchat/groupchat.py 33.33% 1 Missing and 1 partial ⚠️
autogen/agentchat/user_proxy_agent.py 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #1146       +/-   ##
===========================================
+ Coverage   37.85%   61.92%   +24.07%     
===========================================
  Files          50       56        +6     
  Lines        5733     5985      +252     
  Branches     1297     1450      +153     
===========================================
+ Hits         2170     3706     +1536     
+ Misses       3391     1946     -1445     
- Partials      172      333      +161
Flag Coverage Δ
unittests 61.73% <76.77%> (+23.88%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ekzhu ekzhu added the dev label Jan 4, 2024
victordibia
victordibia reviewed Jan 5, 2024
View reviewed changes
Comment thread autogen/oai/client.py Outdated
sonichi
sonichi reviewed Jan 5, 2024
View reviewed changes

@sonichi sonichi left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevin666aa do you think the usage summary should be part of the telemetry or separate?
@cheng-tan adding tests + docs early can help others and yourself clarify the way of using it and I generally recommend those to be written even before any implementation. For code formatting, use pre-commit.

Comment thread autogen/oai/client.py Outdated
Comment thread autogen/oai/client.py Outdated
Comment thread autogen/oai/client.py Outdated
Comment thread autogen/oai/client.py Outdated
afourney
afourney reviewed Jan 5, 2024
View reviewed changes
Comment thread autogen/oai/telemetry.py Outdated
Comment thread autogen/oai/telemetry.py Outdated
radman-x
radman-x approved these changes Jan 5, 2024
View reviewed changes

@radman-x radman-x left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok with this, given my previous comments are addressed (even if in a future release).

@ekzhu ekzhu mentioned this pull request Jan 14, 2024
Closed
3 tasks
@cheng-tan cheng-tan mentioned this pull request Jan 17, 2024
Closed
ekzhu
ekzhu reviewed Feb 13, 2024
View reviewed changes
Comment thread autogen/logger/logger_factory.py Outdated
@ekzhu

ekzhu commented Feb 13, 2024

Copy link
Copy Markdown
Contributor

@cheng-tan there is a test error in open ai test.

@cheng-tan

cheng-tan commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

thanks @ekzhu, it only happens on the ci working on fixing it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@victordibia victordibia victordibia approved these changes

@afourney afourney afourney approved these changes

@davorrunje davorrunje Awaiting requested review from davorrunje

@yiranwu0 yiranwu0 Awaiting requested review from yiranwu0

@qingyun-wu qingyun-wu Awaiting requested review from qingyun-wu

@gagb gagb Awaiting requested review from gagb

@sonichi sonichi Awaiting requested review from sonichi

+2 more reviewers

@ekzhu ekzhu ekzhu approved these changes

@radman-x radman-x radman-x approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@cheng-tan @codecov-commenter @afourney @davorrunje @ekzhu @victordibia @sonichi @radman-x