perf: reduce OpenTelemetry metric volume - #895
Conversation
Cloudflare Worker preview
|
MCP tool token costMeasured with
Component totals
Change from baseline
Per-tool changes
Component changes
Per-tool breakdown
Per-component counts are diagnostic and non-additive because keys and separators live in complete tool objects. Per-tool counts encode each complete tool object independently. The total encodes the complete |
Bundle ReportChanges will increase total bundle size by 241 bytes (0.1%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: hevy-mcp-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #895 +/- ##
==========================================
+ Coverage 80.70% 80.75% +0.05%
==========================================
Files 70 70
Lines 4058 4054 -4
Branches 1149 1147 -2
==========================================
- Hits 3275 3274 -1
+ Misses 400 398 -2
+ Partials 383 382 -1 ☔ View full report in Codecov by Harness. |
Unit Test Results 1 files 66 suites 8s ⏱️ Results for commit 2f312c5. |
📝 WalkthroughWalkthroughThe PR normalizes API endpoint labels for metrics, removes result-derived dimensions from tool metrics, and changes metric export from 10 to 30 seconds. Tests cover endpoint labeling, trace preservation, removed attributes, and the new interval. ChangesTelemetry metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Tick the box to add this pull request to the merge queue (same as
|
PR Summary by Qodoperf: reduce OpenTelemetry metric volume
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/node/src/utils/hevy-client-observability.ts`:
- Around line 37-43: Update normalizeHevyMetricEndpoint to accept a dynamic
prefix only when it is followed by one non-empty path segment and no trailing
slash or additional “/...” segments; otherwise return “unknown”. Add regression
tests covering the collection path ending in “/” and a path with an extra
segment, while preserving valid single-resource identifier matching.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b22e434f-d28f-4610-8a2f-a56d03e3bf8f
📒 Files selected for processing (7)
.changeset/quiet-metrics-volume.mdpackages/node/src/utils/hevy-client-observability.test.tspackages/node/src/utils/hevy-client-observability.tspackages/node/src/utils/telemetry.test.tspackages/node/src/utils/telemetry.tspackages/node/src/utils/tool-observer.test.tspackages/node/src/utils/tool-observer.ts
💤 Files with no reviewable changes (1)
- packages/node/src/utils/tool-observer.ts
| function normalizeHevyMetricEndpoint(endpoint: string): string { | ||
| const path = endpoint.split("?")[0] ?? ""; | ||
| if (SAFE_STATIC_ENDPOINTS.has(path)) return path; | ||
| return ( | ||
| SAFE_DYNAMIC_ENDPOINTS.find(([prefix]) => path.startsWith(prefix))?.[1] ?? | ||
| "unknown" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 \
--glob '*.ts' \
--glob '*.tsx' \
--glob '!**/node_modules/**' \
'(/v1/(body_measurements|exercise_history|exercise_templates|routine_folders|routines|workouts)|endpoint[[:space:]]*[:=])' \
packagesRepository: chrisdoc/hevy-mcp
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- packages/node/src/utils/hevy-client-observability.ts ---\n'
sed -n '1,180p' packages/node/src/utils/hevy-client-observability.ts
printf '\n--- packages/node/src/utils/hevy-client-observability.test.ts relevant tests ---\n'
sed -n '115,175p' packages/node/src/utils/hevy-client-observability.test.ts
printf '\n--- packages/hevy-client/src/hevy-client-kubb.ts normalization duplicate ---\n'
sed -n '200,235p' packages/hevy-client/src/hevy-client-kubb.ts
printf '\n--- static normalization behavior for dynamic prefix inputs ---\n'
python3 - <<'PY'
from pathlib import Path
import re, ast, json
path = Path('packages/node/src/utils/hevy-client-observability.ts')
text = path.read_text()
m = re.search(r'(?ms)(\n const |function) normalizeHevyMetricEndpoint\([\s\S]*?\n\)(?:\s*=>\s*[^{]+)?\s*\{([\s\S]*?)\n}', text)
if not m:
print('function not found')
raise SystemExit(1)
func = m.group(0)
start, end = text[:m.start()].count('\n')+1, text[:m.end()].count('\n')+1
print(f'normalizeHevyMetricEndpoint definition lines {start}-{end}')
print(func)
# Extract the arrays by parsing the TS snippet with JS semantic-ish extraction from source text.
static_set = set()
dynamic = []
for block in text.split('const '):
if 'SAFE_STATIC_ENDPOINTS' in block or 'SAFE_DYNAMIC_ENDPOINTS' in block:
src = block[:block.index(';')+1]
# strip const/assign and use Node eval safely? Actually contains Set constructor syntax not plain JS?
# We can manually convert relevant TypeScript into JS by replacing Set literals.
js = src
js = re.sub(r'const\s+\w+\s*=', '', js)
# Replace Set([...]) around the whole expression or inside assignment with array.
# We'll use a more permissive extraction approach:
pass
PYRepository: chrisdoc/hevy-mcp
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- packages/node/src/utils/hevy-client-observability.ts ---'
sed -n '1,180p' packages/node/src/utils/hevy-client-observability.ts
printf '%s\n' ''
printf '%s\n' '--- packages/node/src/utils/hevy-client-observability.test.ts relevant tests ---'
sed -n '115,175p' packages/node/src/utils/hevy-client-observability.test.ts
printf '%s\n' ''
printf '%s\n' '--- packages/hevy-client/src/hevy-client-kubb.ts normalization duplicate ---'
sed -n '200,235p' packages/hevy-client/src/hevy-client-kubb.ts
printf '%s\n' ''
printf '%s\n' '--- generated resource endpoints under /v1/... ---'
rg -n '"url": "/v1/[^"]*" url:`/v1/|function .*Url\(\) =>' packages/hevy-client/src/generated/client/api -g '*.ts' | head -200
printf '%s\n' ''
printf '%s\n' '--- deterministic dynamic prefix behavior from source constants ---'
python3 - <<'PY'
import re, ast
from pathlib import Path
path = Path('packages/node/src/utils/hevy-client-observability.ts')
text = path.read_text()
m = re.search(r'(?ms)function normalizeHevyMetricEndpoint\(endpoint: string\): string \{([\s\S]*?)\n\}', text)
if not m:
raise SystemExit('normalizeHevyMetricEndpoint not found')
body = m.group(1)
print(body)
static_m = re.search(r'new Set\(\[(?P<body>[\s\S]*?)\]\s*,', text)
dynamic_m = re.search(r'SAFE_DYNAMIC_ENDPOINTS\s*(?:=|as\s)+\[(?P<body>[\s\S]*?)\]\s*as const;', text)
print('static count', len(ast.literal_eval('[' + static_m.group('body') + ']')) if static_m else '?')
print('dynamic entries')
for raw in re.split(r',\s*(?=/\')', dynamic_m.group('body')) if dynamic_m else []:
raw = raw.strip()
match = re.match(r'\["([^"]+)", "([^"]+)"\]', raw)
if match:
print((match.group(1), match.group(2)))
static = set(ast.literal_eval('[' + static_m.group('body') + ']'))
dynamics = [(match.group(1), match.group(2)) for match in (re.finditer(r'\["([^"]+)", "([^"]+)"\]', dynamic_m.group('body')) or [])]
cases = ['/v1/workouts/', '/v1/workouts/id/extra', '/v1/workouts/id?foo=bar']
for case in cases:
path_part = case.split('?', 1)[0] or ''
out = path_part if path_part in static else next(((found, templ) for prefix, templ in dynamics if path_part.startswith(prefix)), 'unknown')
print({'input': case, 'normalized': out})
PYRepository: chrisdoc/hevy-mcp
Length of output: 8806
Bound dynamic metric matching to single resource paths.
startsWith(prefix) also matches /v1/workouts/ and /v1/workouts/id/extra, but the generated endpoint contract treats these collection IDs as :workoutId rather than unknown. If these labels model one resource identifier, add a non-empty segment check without a trailing / or /..., and add regression tests for both edge cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/node/src/utils/hevy-client-observability.ts` around lines 37 - 43,
Update normalizeHevyMetricEndpoint to accept a dynamic prefix only when it is
followed by one non-empty path segment and no trailing slash or additional
“/...” segments; otherwise return “unknown”. Add regression tests covering the
collection path ending in “/” and a path with an extra segment, while preserving
valid single-resource identifier matching.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTo customize comments, go to the Qodo configuration screen, or learn more in the docs. |
Summary
hevy-mcpValidation
npm run buildnpm run checknpm run check:typesnpm run check:changesetnpm run test:prgit diff --checkFollow-up
This is the first source-instrumentation slice of the metrics-volume plan. Collector filtering, Grafana review, and staged live measurement remain separate follow-up work. No production deployment is included in this PR.
Summary by CodeRabbit
unknownlabel.