feat(worker): add locality to hosted activity spans - #986
Conversation
|
5 clusters identified |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideAdds validated Cloudflare IP-geolocation locality/region/country metadata to hosted Worker MCP activity spans, propagates it through the Worker observer, enriches span attributes with tool taxonomy and geography, and documents the new telemetry and privacy semantics. Sequence diagram for propagating Cloudflare geography into Worker activity spanssequenceDiagram
actor Client
participant Worker as serveMcpRequest
participant Telemetry as worker_telemetry
participant ObserverFactory as createWorkerToolObserver
participant Span as WorkerObservationSpan
Client->>Worker: HTTP request
Worker->>Telemetry: getCloudflareGeography(request)
Telemetry-->>Worker: CloudflareGeography
Worker->>Telemetry: createWorkerUserHash(apiKey)
Telemetry-->>Worker: userHash
Worker->>Telemetry: getCloudflareColo(request)
Telemetry-->>Worker: cloudflareColo
Worker->>ObserverFactory: createWorkerToolObserver({ userHash, cloudflareColo, geoLocalityName, geoLocalityRegion, geoCountryCode })
ObserverFactory-->>Worker: ToolObservationScope
Worker->>Span: span.setAttribute("user.hash", userHash)
Worker->>Span: span.setAttribute("cloudflare.colo", cloudflareColo)
Worker->>Span: span.setAttribute("geo.locality.name", geoLocalityName)
Worker->>Span: span.setAttribute("geo.locality.region", geoLocalityRegion)
Worker->>Span: span.setAttribute("geo.country.code", geoCountryCode)
Worker->>Span: span.setAttribute("hevy.feature", taxonomy.feature)
Worker->>Span: span.setAttribute("mcp.tool.kind", taxonomy.kind)
Worker->>Span: span.setAttribute("mcp.tool.operation", taxonomy.operation)
Worker->>Span: span.end()
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Cloudflare Worker preview
|
Bundle ReportBundle size has no change ✅ |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Aug 11, 2026 7:19p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
PR Summary by Qodofeat(worker): add bounded Cloudflare locality to hosted activity spans
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #986 +/- ##
==========================================
+ Coverage 73.75% 73.94% +0.18%
==========================================
Files 92 92
Lines 6107 6136 +29
Branches 1733 1744 +11
==========================================
+ Hits 4504 4537 +33
+ Misses 1023 1018 -5
- Partials 580 581 +1 ☔ View full report in Codecov by Harness. |
Unit Test Results 1 files 78 suites 24s ⏱️ Results for commit e410c0e. ♻️ This comment has been updated with latest results. |
![gitstream-cm[bot]](https://avatars.githubusercontent.com/in/230441?s=60&v=4){kind=small}
There was a problem hiding this comment.
✨ PR Review
The PR is well-structured: validation helpers are guarded by tight allow-list regexes, the changeset and privacy-policy updates are present, and test coverage exercises both valid and malformed inputs. Two concrete issues are worth addressing before merge.
2 issues detected:
🐞 Bug - Each geo field is evaluated by `safeGeoValue` twice — once in the conditional and once as the spread value — so a mutable accessor could produce different results on the second call. 🛠️
Details: safeGeoValue is invoked twice for each of cf?.city and cf?.region: once for the truthiness guard and once to produce the value. If cf exposes a getter that could change between accesses (e.g. a Proxy-wrapped object), the second call could return undefined even though the first returned a truthy string, silently dropping the field. Beyond correctness, the double invocation needlessly repeats the trim, whitespace-collapse, and Unicode regex for every request.
File: packages/worker/src/worker-telemetry.ts (43-51)
🛠️ A suggested code correction is included in the review comments.
🧹 Maintainability - The same security-sensitive normalisation/validation function is duplicated across two files with no shared source of truth, so future changes to the regex or length cap must be applied in two places.
Details: safeGeoValue (and its associated GEO_VALUE_PATTERN / MAX_GEO_VALUE_LENGTH constants) is defined independently in both worker-telemetry.ts and worker-observer.ts. The two copies are currently byte-for-byte identical, but they will silently diverge if the allow-list regex or the length cap is ever tightened. Because this is security-relevant input sanitisation, a silent divergence could lead to one path accepting values that the other rejects.
File: packages/worker/src/worker-telemetry.ts (23-30)
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
| return { | ||
| ...(safeGeoValue(cf?.city) | ||
| ? { localityName: safeGeoValue(cf?.city) } | ||
| : {}), | ||
| ...(safeGeoValue(cf?.region) | ||
| ? { localityRegion: safeGeoValue(cf?.region) } | ||
| : {}), | ||
| ...(countryCode ? { countryCode } : {}), | ||
| }; |
There was a problem hiding this comment.
🐞 Bug - Double safeGeoValue Call: Store the result of each safeGeoValue call in a local variable and reuse it:
const localityName = safeGeoValue(cf?.city);
const localityRegion = safeGeoValue(cf?.region);
return {
...(localityName ? { localityName } : {}),
...(localityRegion ? { localityRegion } : {}),
...(countryCode ? { countryCode } : {}),
};| return { | |
| ...(safeGeoValue(cf?.city) | |
| ? { localityName: safeGeoValue(cf?.city) } | |
| : {}), | |
| ...(safeGeoValue(cf?.region) | |
| ? { localityRegion: safeGeoValue(cf?.region) } | |
| : {}), | |
| ...(countryCode ? { countryCode } : {}), | |
| }; | |
| const localityName = safeGeoValue(cf?.city); | |
| const localityRegion = safeGeoValue(cf?.region); | |
| return { | |
| ...(localityName ? { localityName } : {}), | |
| ...(localityRegion ? { localityRegion } : {}), | |
| ...(countryCode ? { countryCode } : {}), | |
| }; |
Is this review accurate? Use 👍 or 👎 to rate it
If you want to tell us more, use /gs feedback e.g. /gs feedback this review doesn't make sense, I disagree, and it keeps repeating over and over
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe worker now extracts bounded Cloudflare locality, region, and country metadata, validates it, and adds it to hosted activity spans. Tests cover propagation and rejection rules. Privacy, dashboard, data dictionary, and release documentation define the telemetry policy. ChangesCloudflare geography telemetry
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant serveMcpRequest
participant getCloudflareGeography
participant WorkerToolObserver
participant HostedActivitySpan
Client->>serveMcpRequest: MCP request
serveMcpRequest->>getCloudflareGeography: Read Cloudflare geography metadata
getCloudflareGeography-->>serveMcpRequest: Sanitized locality, region, and country
serveMcpRequest->>WorkerToolObserver: Provide observer geography options
WorkerToolObserver->>HostedActivitySpan: Emit validated geography attributes
Possibly related PRs
Suggested reviewers: 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/telemetry-dashboards.md`:
- Around line 47-52: In docs/telemetry-dashboards.md lines 47-52, either remove
user_hash from persisted metric labels or define a 30-day retention and
restricted-access rule for the trace-derived user activity profile. In
docs/privacy-policy.md lines 48-51, disclose the same retention and access
policy for any metric that persists user_hash with geography; keep both
documents consistent.
In `@packages/worker/src/worker-telemetry.ts`:
- Around line 5-29: Reject identifier-shaped numeric geography values such as
IPv4 addresses, coordinates, and numeric postal codes before safeGeoValue
returns them; apply the same validation in
packages/worker/src/worker-observer.ts lines 20-24 and keep its sanitizer at
lines 230-243 aligned with the extraction sanitizer. Add corresponding rejection
tests in packages/worker/src/worker-telemetry.test.ts lines 54-66 and
span-attribute omission tests in packages/worker/src/worker-observer.test.ts
lines 132-156.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 008f3202-b5a5-4441-b0f7-97c0c81161d3
📒 Files selected for processing (10)
.changeset/quiet-cities-usage.mddocs/privacy-policy.mddocs/telemetry-dashboards.mddocs/telemetry-data-dictionary.mdpackages/worker/src/worker-observer.test.tspackages/worker/src/worker-observer.tspackages/worker/src/worker-telemetry.test.tspackages/worker/src/worker-telemetry.tspackages/worker/src/worker.test.tspackages/worker/src/worker.ts
|
Addressed the review concerns in commit
Validation passed: |
Code Review ✅ Approved 1 resolved / 1 findingsAdds bounded Cloudflare IP-geolocation metadata to hosted Worker activity spans with privacy-safe normalization, addressing the redundant safeGeoValue calls. No issues found.
✅ 1 resolved✅ Quality: Redundant safeGeoValue calls in getCloudflareGeography
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
1 participant
Primary changes
Reviewer walkthrough
packages/worker/src/worker-telemetry.tsextracts and bounds Cloudflare locality, region, and country metadata.packages/worker/src/worker.tsforwards sanitized geography alongside the existing user hash and colo values.packages/worker/src/worker-observer.tsapplies defense-in-depth validation and emits taxonomy and geography attributes; focused tests cover extraction, propagation, and omission.Correctness and invariants
Testing and QA
mise exec -- npm run checkmise exec -- npm run check:typesmise exec -- npm run worker:dry-runmise exec -- npm run test:prmise exec -- npm run test:performancemise exec -- npm run check:changesetThis PR is the Worker-side prerequisite for the separate cardinality-conscious Collector span-metrics pipelines.
Summary by Sourcery
Propagate bounded Cloudflare IP-geolocation metadata into hosted Worker MCP activity spans and document the new privacy-reviewed telemetry fields and retention semantics.
New Features:
Enhancements:
Documentation:
Tests:
✨ PR Description
Purpose: Add bounded Cloudflare IP-geolocation attributes (city, region, country) to hosted MCP activity spans for privacy-reviewed geographic usage aggregation.
Main changes:
safeGeoValue()andsafeCountryCode()validation functions with regex patterns to sanitize and bound geographic fields in worker-observer.ts and worker-telemetry.tsWorkerToolObserverOptionsinterface withgeoLocalityName,geoLocalityRegion,geoCountryCodefields and span attribute emission logicgetCloudflareGeography()function extracting bounded Cloudflare IP-geolocation from request metadata, integrated into MCP request handlerGenerated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines
Summary by CodeRabbit
New Features
Documentation