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

Refine API request logging struct #1852

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

seokho-son
Copy link
Member

@seokho-son seokho-son commented Oct 4, 2024

  • Request logging 활용도 향상을 위한 출력 구조 refine

개선

  • 중복 소지가 있던 Method=OPTIONS 는 로깅에서 제외
  • id, method 를 의도적으로 대문자로 변경하여, 우선 기록되도록 순서 변경
  • byte in out 등 활용도가 낮은 항목 제외 등
cb-tumblebug       | ⇨ http server started on [::]:1323
cb-tumblebug       | 5:28AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728019710091268611 Method=GET URI=/tumblebug/ns?option=id clientIP=10.0.2.2 latency="644.494µs" status=200
cb-tumblebug       | 5:28AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728019710164470559 Method=GET URI=/tumblebug/ns/default/mci?option=id clientIP=10.0.2.2 latency=1.70523ms status=200
cb-tumblebug       | 5:28AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728019710185217384 Method=GET URI=/tumblebug/ns/default/resources/vNet?option=id clientIP=10.0.2.2 latency=3.637388ms status=200
cb-tumblebug       | 5:28AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728019710191162710 Method=GET URI=/tumblebug/ns/default/resources/securityGroup?option=id clientIP=10.0.2.2 latency=1.291026ms status=200
cb-tumblebug       | 5:28AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728019710195235879 Method=GET URI=/tumblebug/ns/default/resources/sshKey?option=id clientIP=10.0.2.2 latency=6.497271ms status=200
cb-tumblebug       | 5:28AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728019710859276801 Method=GET URI=/tumblebug/connConfig?filterVerified=true&filterRegionRepresentative=true clientIP=10.0.2.2 latency=20.881379ms status=200
cb-tumblebug       | 5:33AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728020010807717742 Method=POST URI=/tumblebug/ns clientIP=10.0.2.2 latency=3.935357ms status=200
cb-tumblebug       | 5:33AM INF src/api/rest/server/middlewares/zerologger.go:58 > request ID=1728020031105813503 Method=DELETE URI=/tumblebug/ns/newns clientIP=10.0.2.2 latency=7.953928ms status=200

@seokho-son seokho-son requested a review from yunkon-kim as a code owner October 4, 2024 05:43
@seokho-son
Copy link
Member Author

Staging 관련 commit은 빠르게 반영이 필요하여, 해당 PR에 포함하였습니다. (직전 프리릴리스에 포함되어야 했던 사항 보완)

@seokho-son
Copy link
Member Author

PTAL @yunkon-kim

Copy link
Member

@yunkon-kim yunkon-kim left a comment

Choose a reason for hiding this comment

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

@seokho-son LGTM!!

마이너한 수정 사항 이외에 별다른 이슈는 없을 것으로 보여서 Approve 했습니다 :-)

Str("bytes_in", v.ContentLength).
Int64("bytes_out", v.ResponseSize).
Msg("request")
if v.Method != "OPTIONS" {
Copy link
Member

Choose a reason for hiding this comment

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

다음 const를 활용하시면 좋을 것 같습니다. "net/http" pkg import 필요
: 확인해보니, echo.OPTIONS는 deprecated되었습니다... (https://pkg.go.dev/github.com/labstack/echo/v4#pkg-constants)

Suggested change
if v.Method != "OPTIONS" {
if v.Method != http.MethodOptions {

(확인 차 답글을 남겨 놓는 부분입니다.)

  1. Skipper에서 처리하지 않으셨길래 생각해본 바로는, 존재하지 않는 API 요청(Error)에 대해서는 로그를 남기시려는 의도로 이해됩니다.

  2. 다음 코드를 보니 이후에 조건이 추가되는 것을 고려하고 계신 것 같습니다. :-)

if v.Error == nil
  if v.Method != "OPTIONS"

Copy link
Member Author

Choose a reason for hiding this comment

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

네네. 일단 OPTIONS라도 error라면 출력되는 것을 고려하였습니다.

@seokho-son
Copy link
Member Author

@yunkon-kim 제안 사항까지 반영 완료하였습니다. :)

@yunkon-kim
Copy link
Member

/approve

@github-actions github-actions bot added the approved This PR is approved and will be merged soon. label Oct 4, 2024
@cb-github-robot cb-github-robot merged commit 2eceb00 into cloud-barista:main Oct 4, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved and will be merged soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants