maxDepth 인자를 createNextRoutesUrlGroup 함수에 추가합니다. by pcfulife · Pull Request #11 · NaverPayDev/prometheus

pcfulife · 2026-03-16T05:29:42Z

Related Issue

createKoaPrometheusExporter 함수에 대한 질문 및 건의입니다. #10

Describe your changes

maxDepth: URL의 최대 깊이 설정 (기본값 Number.MAX_SAFE_INTEGER)
- 예: maxDepth가 3일 때, /naver/pay/front/end => /naver/pay/frond로 Trim
trimDynamic: Dynamic 라우트도 trim 하도록 설정 (기본값 false)
- 예: trimDynamic이 false면 /naver/pay/article/10 => /naver/pay/article/[id]로 Trim되지 않음

Request

…hen using `normalizeNextRoutesPath`

…namic routes when using `normalizeNextRoutesPath`

claude · 2026-03-16T05:29:46Z

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

2-one-week

코드 잘봤습니다! 감사합니다~

…eters

pcfulife · 2026-03-16T06:22:19Z

@2-one-week Koa와 Hono 쪽에 파라미터를 추가하는것이 누락되어서 새 커밋 추가하였습니다!

2-one-week

확인했습니다

boxersb · 2026-03-16T06:48:22Z

packages/koa/src/middleware/metrics.ts

+    KoaPrometheusExporterOptions,
+    'nextjs' | 'bypass' | 'normalizePath' | 'formatStatusCode' | 'maxDepth' | 'trimDynamic'
+>): Middleware {
+    const normalizeNextRoutesPath = nextjs ? createNextRoutesUrlGroup(maxDepth, trimDynamic) : undefined


nextjs: false 이면 normalizeNextRoutesPath 기능이 동작하지 않는건데, 기본적으로 maxDepth 가 적용되면 좋겠습니다.

kyungmi · 2026-03-16T06:52:37Z

packages/core/src/next/utils.ts

+                    ? it.page
+                    : `/${it.page
+                          .split('/')
+                          .slice(1, maxDepth + 1)


maxDepth가 없는 경우에는 Number.MAX_SAFE_INTEGER + 1을 하려고 할텐데,
Number.MAX_SAFE_INTEGER 로 기본값 세팅하는 것보다는 maxDepth 유무에 따라 분기처리하는게 안전해 보입니당.

kyungmi · 2026-03-16T06:52:40Z

packages/core/src/next/utils.ts

-                return page
+        for (const it of routes) {
+            if (it.regex.test(withoutBasePathUrl)) {
+                return dynamicRoutes.includes(it) && !trimDynamic


@pcfulife trimDynamic 옵션이 꼭 필요할까요? 대상 라우트가 수백, 수천개 되지 않는 이상에는 솔직히 dynamic 라우트는 카디널리티 측면에서 위험하진 않아 보여서요.

@kyungmi 아, 이건 제가 헷갈렸습니다. 기본값이 trim을 하는게 아니었죠.. 제거하겠습니다.

kyungmi · 2026-03-16T06:52:42Z

packages/core/src/next/utils.ts

-            if (regex.test(withoutBasePathUrl)) {
-                return page
+        for (const it of routes) {
+            if (it.regex.test(withoutBasePathUrl)) {


maxDepth 옵션이 dynamicRoute가 아닐때도 적용 되어야 하지 않을까요?

…trims dynamic routes when using `normalizeNextRoutesPath`" This reverts commit da104a1.

pcfulife · 2026-03-17T04:53:07Z

@boxersb @kyungmi 리뷰 반영하였습니다!

boxersb · 2026-03-18T08:55:00Z

packages/core/src/metrics/util.ts

+
+export const trimUrl = (url: string, maxDepth?: number) => {
+    const withStartingSlashUrl = url.startsWith('/') ? url : '/' + url
+    return typeof maxDepth === 'number' ? withStartingSlashUrl.split('/', maxDepth + 1).join('/') : withStartingSlashUrl


이 typeof 는 undefined 걸러내기위해서 쓰신건가요?
0일때는 빈문자열이 반환됩니다.

그리고, maxDepth 가 3이면 지금 로직으로는 '/foo/bar/baz' -> '/foo/bar' 와 같이 2 depth 까지만 trim 되는것 같습니다.

@boxersb maxDepth가 0인것은 사실 고려하지 않았습니다. /로 반환하는게 좋을까요?

@boxersb

저는 로직이 정상 동작하는 것으로 보이는데.. 혹시 어떤 부분이 문제일까요?

아 + 1 이 있었군요~ 그건 제가 잘못 봤습니다.

maxDepth 가 0일때 아무 액션도 취하지 않으려면 로직을 좀 수정하는게 좋을것 같네요~

@boxersb 0 이하면 에러를 던지고 undefined 일 때 작업을 하지 않으려고 합니다.

boxersb · 2026-03-19T07:02:46Z

packages/core/src/types.ts

    /** Whether to collect default Node.js metrics */
    collectDefaultMetrics?: boolean
+    /** Max number to trim path */
+    maxDepth?: number


옵션명에 설명이 좀 부족한 느낌인데 괜찮을까요?

@boxersb 조금 더 보강해보겠습니다!

core 입장에서의 maxDepth 가 어디의 depth 인지 인지하기 어렵습니다.
maxDepthOfPath 나 maxNormalizeDepth 가 가까울것 같은 느낌인데 좀 안이쁘긴 해요;

@boxersb 아, 옵션명 이야기였군요..! 위에 주석 이야기인줄 알았습니다. 한번 고민해보겠습니다.

boxersb · 2026-03-19T07:58:45Z

packages/hono/src/middleware/metrics.ts

    const extendedNormalizePath = (context: Context) => {
        const url = new URL(context.req.url)
-        return normalizeNextRoutesPath?.(url.href) || normalizePath?.(context) || url.pathname
+        return normalizeNextRoutesPath?.(url.href) || normalizePath?.(context) || trimUrl(url.pathname, maxDepth)


nextjs 일때는 옵션으로 넘기는 normalizePath 나 trimUrl 이 전혀 동작하지 않을텐데요, nextjs 더라도 nextjs 의 manifest 에 없는 uri 도 분석하게 할 수 없을까요?

그리고, 옵션 설계가 지금처럼 nextjs 여부에 따르지 않고, normalizePath 여부에 따라 분기되어야 하는건 아닐지요?

const normalizeRouePath = normalizePath || nextjs ? createNextRoutesUrlGroup(maxDepth) : trimUrl ... return normalizeRoutePath(url, maxDepth)

cc) @2-one-week

@boxersb createNextRoutesUrlGroup 함수 내부에서 manifest에 없는 URL에도 trimUrl을 적용하고 있습니다.

https://github.com/NaverPayDev/prometheus/pull/11/changes#diff-6a2432e163645db1e583ebf4933b5427a8a737aab8eb9101d6b1461f0d384de8R74

옵션 설계를 수정하는 것은 일종의 Breaking changes라서 cc 해주신 것처럼 메인 컨트리뷰터분들 의견도 필요할 것 같습니다.

@boxersb createNextRoutesUrlGroup 함수 내부에서 manifest에 없는 URL에도 trimUrl을 적용하고 있습니다.

https://github.com/NaverPayDev/prometheus/pull/11/changes#diff-6a2432e163645db1e583ebf4933b5427a8a737aab8eb9101d6b1461f0d384de8R74

@pcfulife 아 넵 nextjs manifest 에 없을때도 처리되는것 확인했습니다!

…efined

claude

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

boxersb · 2026-03-20T09:18:06Z

packages/core/src/metrics/util.ts

+        for (const token of url.split('/')) {
+            urlTokens.push(token)
+            if (token) {
+                nonEmptyUrlTokenCount = nonEmptyUrlTokenCount + 1
+            }
+            if (nonEmptyUrlTokenCount === maxDepth) {
+                break
+            }
+        }


첫번째 토큰은 무조건 빈문자열이니까 maxDepth 가 0일때 빈문자열을 리턴하게 되는거군요..

로직은 고치기 전것이 더 좋았는데, 좀 헷갈리네요.

혹시 maxDepth 가 0 일때는 undefined 때랑 동일하게 전체 url을 리턴하게 하면 어떤가요?

@boxersb 반영하였습니다!

…ogic it.

claude

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

boxersb · 2026-03-24T00:44:09Z

packages/hono/src/middleware/metrics.ts

+        return (
+            normalizeNextRoutesPath?.(url.href) ||
+            normalizePath?.(context) ||
+            trimUrl(url.pathname, maxNormalizedUrlDepth)


이거 이름을 normalizeUrlWithTrimming 이걸로 바꾸신것 같은데 반영이 안되었습니다.

pcfulife added 2 commits March 16, 2026 13:52

[NaverPayDev#10] feat: Add new parameter maxDepth which trims URL w…

d9fbee4

…hen using `normalizeNextRoutesPath`

[NaverPayDev#10] feat: Add new parameter trimDynamic which trims dy…

da104a1

…namic routes when using `normalizeNextRoutesPath`

2-one-week previously approved these changes Mar 16, 2026

View reviewed changes

[NaverPayDev#10] bug: Fix hono and koa didn't accept additional param…

e6e5639

…eters

pcfulife dismissed 2-one-week’s stale review via e6e5639 March 16, 2026 06:22

2-one-week previously approved these changes Mar 16, 2026

View reviewed changes

boxersb reviewed Mar 16, 2026

View reviewed changes

kyungmi reviewed Mar 16, 2026

View reviewed changes

pcfulife added 5 commits March 16, 2026 15:53

Revert "[NaverPayDev#10] feat: Add new parameter trimDynamic which …

0707a68

…trims dynamic routes when using `normalizeNextRoutesPath`" This reverts commit da104a1.

[NaverPayDev#10] revert: Remove trimDynamic

1d58325

[NaverPayDev#10] Fix: Only apply to default case

5fc56f3

[NaverPayDev#10] Fix: Trim URL for unnormalized case

36166a8

[NaverPayDev#10] Fix: trimUrl always returns withStartingSlashUrl

7d1d3bf

pcfulife changed the title ~~maxDepth와 trimDynamic 인자를 createNextRoutesUrlGroup 함수에 추가합니다.~~ maxDepth와 인자를 createNextRoutesUrlGroup 함수에 추가합니다. Mar 17, 2026

pcfulife changed the title ~~maxDepth와 인자를 createNextRoutesUrlGroup 함수에 추가합니다.~~ maxDepth 인자를 createNextRoutesUrlGroup 함수에 추가합니다. Mar 17, 2026

pcfulife dismissed 2-one-week’s stale review via 36166a8 March 17, 2026 04:53

boxersb reviewed Mar 19, 2026

View reviewed changes

pcfulife added 2 commits March 19, 2026 18:18

[NaverPayDev#10] Fix: trimUrl now accepts only positive number or und…

95637a8

…efined

[NaverPayDev#10] Fix: trimUrl now follow original starting slash of URL

17e3279

claude bot reviewed Mar 20, 2026

View reviewed changes

[NaverPayDev#10] refactor: More concrete comment

6eea4c0

boxersb reviewed Mar 20, 2026

View reviewed changes

pcfulife added 2 commits March 23, 2026 13:47

[NaverPayDev#10] refactor: Rename to normalizeUrlWithTrimming and rel…

9962d0b

…ogic it.

[NaverPayDev#10] Refactor: Rename parameter

ea93a2c

claude bot reviewed Mar 23, 2026

View reviewed changes

boxersb reviewed Mar 24, 2026

View reviewed changes

Conversation

pcfulife commented Mar 16, 2026

Related Issue

Describe your changes

Request

Uh oh!

claude bot commented Mar 16, 2026

Uh oh!

2-one-week left a comment

Choose a reason for hiding this comment

Uh oh!

pcfulife commented Mar 16, 2026

Uh oh!

2-one-week left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pcfulife commented Mar 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pcfulife Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pcfulife Mar 19, 2026 •

edited

Loading