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

Add logging #92

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add logging #92

wants to merge 5 commits into from

Conversation

DR0P-database
Copy link
Contributor

@DR0P-database DR0P-database commented Feb 11, 2025

Add logging middleware for any request and deliver logs to marketing_api

Изменения

Добавил middleware, который обрабатывает абсолютно каждый запрос HTTP/HTTPS
Это встроенный способ FastAPI глобально перехватывать и модифицировать все HTTP-запросы и ответы.
Можно изменять заголовки, логировать, валидировать, перехватывать ошибки и многое другое.

Детали реализации

Check-List

  • Вы проверили свой код перед отправкой запроса?
  • Вы написали тесты к реализованным функциям?
  • Вы не забыли применить форматирование black и isort для Back-End или Prettier для Front-End?

As middleware for any request
@DR0P-database DR0P-database self-assigned this Feb 11, 2025
Copy link

github-actions bot commented Feb 11, 2025

Code Coverage

Coverage Report
FileStmtsMissCoverMissing
rating_api
   __main__.py440%1–7
   exceptions.py34779%35–37, 48–50, 58
rating_api/models
   base.py55591%24–27, 76
   db.py73297%61, 74
rating_api/routes
   base.py521277%55–56, 59–60, 63, 66–70, 79, 103–105
   comment.py1012278%34, 38, 56, 74, 109–111, 132–141, 184, 194–199, 206, 230, 246
   exc_handlers.py23387%33, 40, 47
   lecturer.py841483%132–155, 171, 179, 197, 203
rating_api/schemas
   base.py12467%6–9
TOTAL5467387% 

Summary

Tests Skipped Failures Errors Time
40 0 💤 0 ❌ 0 🔥 15.138s ⏱️

@DR0P-database
Copy link
Contributor Author

DR0P-database commented Feb 11, 2025

ДОРАБОТАТЬ:

  • На какую ссылку отправлять логи и где ее брать?
  • Если нужен user_id, то как его прокинуть?
image
  • В каком формате отправлять данные лога? Сейчас в POST marketing/action нету поля для status code и не совсем соответствуют поля
  • Что мы логируем? Абсолютно все, включая 2xx или только 4xx/5xxx?
  • Какой текст ошибки возвращать в лог при обработки исключения если ошибки 5xx?
  • Тесты

Copy link
Contributor

@parfenovma parfenovma left a comment

Choose a reason for hiding this comment

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

я не знаю как хендлить ошибки в middleware, нужно ли делать

try:...
except Exception as e:
# compute something
raise e

то есть нужно ли ререйзить ошибки или нет (но вроде нет и у тебя норм всё)

rating_api/routes/base.py Outdated Show resolved Hide resolved
rating_api/routes/base.py Outdated Show resolved Hide resolved
Copy link

💩 Code linting failed, use black and isort to fix it.

@parfenovma
Copy link
Contributor

Примерно следующее пишем в маркетинг:

{
user_id=-2, // Для бекендовых событий принтера используем -1, тут мб -2 сделать
action="POST",
path_from="https://api.test.profcomff.com/rating/comment"
path_to="",
additional_data={"auth_user_id": 2323423, "request": ...., "response": response.text, "response_status_code": ....}
}
  1. На какую ссылку отправлять логи и где ее брать?
    Зависит от env-а. В Проде на прод https://api.profcomff.com/marketing/v1/action
    в тесте на тест https://api.test.profcomff.com/marketing/v1/action
    локально на локально поднятый маркетинг
  2. Если нужен user_id, то как его прокинуть?
    Для принтера user_id = -1 (он тоже пишет бекендовые события в маркетинг)
    Для других апишек предлагаю так же делать отрицательные user_id (-2, например, для рейтинга)
    а пользовательский user_id можно взять из токена, который приходит в request-е с фронта, и писать его в additional_data
  3. Что мы логируем? Абсолютно все, включая 2xx или только 4xx/5xxx?
    Всё логгируем)
  4. Какой текст ошибки возвращать в лог при обработки исключения если ошибки 5xx?
    Интерфейс не должен поменяться, пользователю отдаем 500 Internal server error, в маркетинг пишем такой же ответ
    как отдаем пользователю

Про тесты
Юнит тесты: mockаем ручки маркетинга, тестируем логику отправки бекендовых событий (для разных статусов)
Интеграционные тесты (вау, такого ещё не было): Нужно проверить что робот рейтинга (@Temmmmmo помоги сделать робота в аутхе пж) имеет токен со скоупом на запись в маркетинг. Также нужно проверить что ничего не падает при неверном запросе (422) ни в рейтинге, ни в маркетинге (в голову приходит, например, чтомы json кривой отправили в маркетинг, будем ждать ответа от маркетинга и/или ретраить что-то и упадем везде по таймауту. На такое тоже надо тесты бахнуть)

@Temmmmmo
Copy link
Member

  1. Ссылку просто сделай глобальной переменной, а для теста и прода я отдельно на гитхабе пропишу значения. Роут для ссылки бери, который предоставил Миша.

@Temmmmmo
Copy link
Member

  1. Чаще всего, 500 ошибка значит, что она просто не задокументирована четырехсотым кодом)

@Temmmmmo
Copy link
Member

Про тесты.
Миша сказал про мок.
Как это сделать, посмотри в conftest.py. Там есть строчка, где мокается UnionAuth. Нужно просто сделать аналогично. Плюс можно прописать возвращаемое значение мокнутой ручки.

@parfenovma
Copy link
Contributor

  1. Чаще всего, 500 ошибка значит, что она просто не задокументирована четырехсотым кодом)

5хх это ошибка на стороне сервера, а 4хх на стороне клиента

Copy link

💩 Code linting failed, use black and isort to fix it.

@DR0P-database
Copy link
Contributor Author

Не знаю как получить токен => user_id

Не могу написать тесты, так же интеграционные

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants