Skip to content
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

feat(#1029): improve server api logging #1148

Merged
merged 7 commits into from Feb 15, 2022

Conversation

frascuchon
Copy link
Member

@frascuchon frascuchon commented Feb 14, 2022

This PR improves the server api logging by using loguru. If loguru package is not installed, the "basic" logging will be used.

In future PRs we can customize or parameterize some aspects of new logging engine.

The logging traces looks like that:

2022-02-14 13:18:01.905 | INFO     | uvicorn.lifespan.on:startup:59 - Application startup complete.
2022-02-14 13:18:01.906 | INFO     | uvicorn.server:_log_started_message:206 - Uvicorn running on http://0.0.0.0:6900 (Press CTRL+C to quit)
2022-02-14 13:18:06.438 | INFO     | uvicorn.protocols.http.httptools_impl:send:432 - 127.0.0.1:57502 - "GET /api/me HTTP/1.1" 200
2022-02-14 13:18:07.386 | INFO     | uvicorn.protocols.http.httptools_impl:send:432 - 127.0.0.1:57502 - "GET /api/datasets/ HTTP/1.1" 200
/usr/local/anaconda3/envs/rubrix/lib/python3.8/site-packages/opensearchpy/connection/base.py:208: OpenSearchWarning: Loading the fielddata on the _id field is deprecated and will be removed in future versions. If you require sorting or aggregating on this field you should also include the id in the body of your documents, and map this field as a keyword field that has [doc_values] enabled
  warnings.warn(message, category=OpenSearchWarning)
2022-02-14 13:18:10.220 | INFO     | uvicorn.protocols.http.httptools_impl:send:432 - 127.0.0.1:57502 - "POST /api/datasets/test_some_sort/TokenClassification%3Asearch?limit=5&from=0 HTTP/1.1" 200
2022-02-14 13:18:10.296 | INFO     | uvicorn.protocols.http.httptools_impl:send:432 - 127.0.0.1:57502 - "POST /api/datasets/test_some_sort/TokenClassification%3Asearch?limit=0&from=0 HTTP/1.1" 200
2022-02-14 13:18:17.091 | INFO     | uvicorn.protocols.http.httptools_impl:send:432 - 127.0.0.1:57503 - "GET /api/datasets/ HTTP/1.1" 200
2022-02-14 13:18:18.958 | INFO     | uvicorn.protocols.http.httptools_impl:send:432 - 127.0.0.1:57503 - "POST /api/datasets/test_multiple_mentions_in_same_record/TokenClassification%3Asearch?limit=5&from=0 HTTP/1.1" 200
2022-02-14 13:18:19.027 | INFO     | uvicorn.protocols.http.httptools_impl:send:432 - 127.0.0.1:57503 - "POST /api/datasets/test_multiple_mentions_in_same_record/TokenClassification%3Asearch?limit=0&from=0 HTTP/1.1" 200

Closes #1029

@frascuchon frascuchon self-assigned this Feb 14, 2022
@frascuchon frascuchon added this to In progress in Release via automation Feb 14, 2022
@frascuchon frascuchon moved this from In progress to Review in Release Feb 14, 2022
@codecov
Copy link

codecov bot commented Feb 14, 2022

Codecov Report

Merging #1148 (b71662d) into master (df5fc8a) will decrease coverage by 0.04%.
The diff coverage is 62.06%.

❗ Current head b71662d differs from pull request most recent head 17a501d. Consider uploading reports for the commit 17a501d to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1148      +/-   ##
==========================================
- Coverage   95.06%   95.02%   -0.05%     
==========================================
  Files         123      122       -1     
  Lines        4846     4827      -19     
==========================================
- Hits         4607     4587      -20     
- Misses        239      240       +1     
Flag Coverage Δ
pytest 95.02% <62.06%> (-0.05%) ⬇️

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

Impacted Files Coverage Δ
src/rubrix/server/__main__.py 0.00% <ø> (ø)
src/rubrix/logging.py 63.63% <21.42%> (-31.11%) ⬇️
src/rubrix/server/server.py 85.48% <100.00%> (+4.63%) ⬆️
src/rubrix/server/tasks/search/model.py 95.45% <0.00%> (-0.20%) ⬇️
src/rubrix/server/tasks/commons/metrics/service.py 98.21% <0.00%> (-0.07%) ⬇️
src/rubrix/client/datasets.py 98.19% <0.00%> (-0.02%) ⬇️
src/rubrix/client/models.py 100.00% <0.00%> (ø)
src/rubrix/client/sdk/text2text/models.py 100.00% <0.00%> (ø)
src/rubrix/server/tasks/search/service.py 100.00% <0.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bd8074...17a501d. Read the comment docs.

@frascuchon frascuchon changed the title feat(logging): improve server api logging feat(#1029): improve server api logging Feb 14, 2022
dcfidalgo
dcfidalgo previously approved these changes Feb 15, 2022
Copy link
Contributor

@dcfidalgo dcfidalgo left a comment

Choose a reason for hiding this comment

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

💯

@frascuchon frascuchon merged commit 18efbfb into master Feb 15, 2022
@frascuchon frascuchon deleted the feat/enhanced-server-logging branch February 15, 2022 10:10
@frascuchon frascuchon moved this from Review to Ready to DEV QA in Release Feb 15, 2022
@frascuchon frascuchon moved this from Ready to DEV QA to Approved DEV QA in Release Feb 15, 2022
dcfidalgo pushed a commit that referenced this pull request Feb 16, 2022
* feat(logging): improve server api logging

* chore: rubrix:app

* chore: conditional logging configuration if module loguru not found

* chore: rename logging handler

* test: logging handler is configured properly

* chore: better warning message
@frascuchon frascuchon moved this from Approved DEV QA to Ready to Release QA in Release Mar 3, 2022
frascuchon added a commit that referenced this pull request Mar 3, 2022
* feat(logging): improve server api logging

* chore: rubrix:app

* chore: conditional logging configuration if module loguru not found

* chore: rename logging handler

* test: logging handler is configured properly

* chore: better warning message

(cherry picked from commit 18efbfb)
@frascuchon frascuchon moved this from Ready to Release QA to Approved Release QA in Release Mar 3, 2022
frascuchon added a commit that referenced this pull request Mar 4, 2022
* feat(logging): improve server api logging

* chore: rubrix:app

* chore: conditional logging configuration if module loguru not found

* chore: rename logging handler

* test: logging handler is configured properly

* chore: better warning message

(cherry picked from commit 18efbfb)

chore(#1029): generalize logging configuration message (#1224)

(cherry picked from commit c390911)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Release
Approved Release QA
Development

Successfully merging this pull request may close these issues.

[Trace] Better logging api server configuration
2 participants