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

Fix pattern unmatched issue in api log skipper #1492

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

seokho-son
Copy link
Member

  • 기존 코드에는 URL.Path 로 패턴 매칭하여, 쿼리 파라미터 처리가 되지 않는 문제가 있었음.
  • 이를 개선하며, 다중 스트링 중복 매칭을 지원할 수 있게 수정.

@yunkon-kim
Copy link
Member

@seokho-son 의견을 드리기에 앞서서 질문이 있습니다.

하나의 path에 여러 개의 query params가 오는 경우를 고려하고 계신 것일까요?

  • 가정) /mcis?options=status&key1=value1&key2=value2

@seokho-son
Copy link
Member Author

@yunkon-kim 넵, 일차적으로는 mics/{mcis} + 상태옵션. 까지도 제외되도록 하는 것이고, 향후에는 필요에 따라 여러가지 동시 패턴도 지원하게끔 수정한 사항입니다.

@seokho-son
Copy link
Member Author

@yunkon-kim 아 참고로 기존 코드가 정상 동작하지 않았던 이유는
패턴 매칭을 c.Request().URL.Path 기준으로 해서 쿼리파라미터가 포함되어 있지 않았기 때문입니다. :)

최초 커밋의 패턴 단일 스트링에 ? 가 포함되어 있었던 이유는 쿼리 파람까지 포함되어 있던 LogURI 를 기준으로 매칭을 했기 때문입니다.

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 입니다 ^^

참고 - 의도하신 바를 나타내기 위한 마이너한 수정사항을 바로 반영해놓겠습니다.

src/api/rest/server/middlewares/custom-middleware.go Outdated Show resolved Hide resolved
@yunkon-kim yunkon-kim merged commit 02596a6 into cloud-barista:main Apr 4, 2024
3 checks passed
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.

None yet

2 participants