[otel] Log UI language from the frontend #4062
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis change adds UI language logging functionality across the stack. A new POST endpoint Changes
Sequence DiagramsequenceDiagram
participant User
participant i18n as i18n Module
participant Backend as Backend Wrapper
participant Client as API Client
participant Controller as Backend Controller
participant OTEL as OTEL Tracing
User->>i18n: changeLanguage()
i18n->>i18n: updateLangFromUser()
i18n->>Backend: uiLanguage(lang)
Backend->>Client: userApi.uiLanguage(lang)
Client->>Controller: POST /v1/users/uilanguage
Controller->>OTEL: Start activity "logging current UI language"
OTEL->>OTEL: Tag: ui_language = lang
OTEL->>Controller: Activity recorded
Controller->>Client: 200 OK
Client->>Backend: Promise resolved
Backend->>i18n: void
i18n->>User: Language updated & logged
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4062 +/- ##
==========================================
- Coverage 74.59% 74.57% -0.03%
==========================================
Files 294 295 +1
Lines 10908 10938 +30
Branches 1367 1372 +5
==========================================
+ Hits 8137 8157 +20
- Misses 2375 2385 +10
Partials 396 396
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Backend/Controllers/UserController.cs (1)
98-106: Consider validating theuilangparameter.The endpoint accepts any string for
uilangand logs it directly to OTEL telemetry. While this is low-risk since it only affects logging, consider validating against a list of supported languages to ensure cleaner telemetry data and prevent logging of arbitrary/malicious strings.If validation isn't needed for this use case, this is acceptable as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Backend/Controllers/UserController.cs(1 hunks)src/api/api/user-api.ts(5 hunks)src/backend/index.ts(1 hunks)src/i18n/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/backend/index.ts (1)
src/api/api/user-api.ts (3)
uiLanguage(890-906)uiLanguage(1094-1098)uiLanguage(1449-1456)
Backend/Controllers/UserController.cs (2)
Backend/Controllers/LiftController.cs (2)
HttpGet(346-360)IActionResult(497-504)Backend/Otel/OtelService.cs (1)
OtelService(6-18)
src/i18n/index.ts (2)
src/api/api/user-api.ts (3)
uiLanguage(890-906)uiLanguage(1094-1098)uiLanguage(1449-1456)src/backend/index.ts (1)
uiLanguage(731-733)
src/api/api/user-api.ts (1)
src/api/base.ts (2)
RequestArgs(38-41)BASE_PATH(20-20)
🔇 Additional comments (2)
src/backend/index.ts (1)
731-733: LGTM!The function correctly wraps the API call with authentication headers and follows the established patterns in this file.
src/api/api/user-api.ts (1)
1-13: Auto-generated file - no manual review required.This file is auto-generated by OpenAPI Generator based on the backend API specification. The generated code correctly implements the new
uiLanguageendpoint following the existing patterns.
There was a problem hiding this comment.
Pull request overview
This PR adds OpenTelemetry logging for the UI language from the frontend, enabling better tracking of user language preferences. The logging is triggered during login, page refresh, and when users update their UI language in settings.
- Added a new backend endpoint to log UI language as an OpenTelemetry tag
- Generated TypeScript API client code for the new endpoint
- Integrated UI language logging into the language update flow in the i18n module
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Backend/Controllers/UserController.cs | Adds new UiLanguage endpoint that logs the UI language as an OpenTelemetry tag |
| src/api/api/user-api.ts | Auto-generated TypeScript API client code for the new uiLanguage endpoint |
| src/backend/index.ts | Adds wrapper function uiLanguage to call the backend API endpoint |
| src/i18n/index.ts | Updates updateLangFromUser to log the resolved UI language after language changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Backend/Controllers/UserController.cs (1)
98-106: LGTM! Endpoint follows established patterns.The implementation correctly uses POST, creates OTEL activities consistent with other controllers, and properly tags the telemetry data. The dual-tag pattern (operation descriptor + actual language value) provides good observability.
Consider adding basic validation for the
uilangparameter to ensure it's a non-empty string and optionally matches expected language code patterns (e.g., 2-3 letter codes). While not critical for a logging endpoint, it would prevent noise in telemetry data:public IActionResult UiLanguage([FromBody, BindRequired] string uilang) { + if (string.IsNullOrWhiteSpace(uilang)) + { + return BadRequest("UI language cannot be empty"); + } using var activity = OtelService.StartActivityWithTag(otelTagName, "logging current UI language"); activity?.SetTag("ui_language", uilang); return Ok(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Backend/Controllers/UserController.cs(1 hunks)src/api/api/user-api.ts(5 hunks)src/backend/index.ts(2 hunks)src/i18n/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/backend/index.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/i18n/index.ts (2)
src/api/api/user-api.ts (3)
uiLanguage(894-910)uiLanguage(1098-1102)uiLanguage(1453-1460)src/backend/index.ts (1)
uiLanguage(732-734)
Backend/Controllers/UserController.cs (1)
Backend/Otel/OtelService.cs (1)
OtelService(6-18)
src/api/api/user-api.ts (1)
src/api/base.ts (2)
RequestArgs(38-41)BASE_PATH(20-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test_coverage (22)
- GitHub Check: Analyze (csharp)
- GitHub Check: test_build (8.0.x)
- GitHub Check: docker_build
- GitHub Check: tox (3.12)
🔇 Additional comments (2)
src/i18n/index.ts (1)
54-60: LGTM! Error handling properly implemented.The fire-and-forget pattern with
.catch()is appropriate for telemetry logging. The console warning provides debugging visibility without blocking the language update flow. This correctly addresses the previous review concern about unhandled promise rejections.src/api/api/user-api.ts (1)
500-548: LGTM! Auto-generated API client code follows standard patterns.The generated code correctly implements the UI language endpoint across all API layers (param creator, FP interface, factory, and class). The POST method and request structure align with the backend endpoint definition.
Also applies to: 888-910, 1092-1102, 1241-1253, 1446-1460
|
do we want to bother if it wasn't updated? |
imnasnainaec
added
bug
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 1 file and made 1 comment.
Reviewable status: 1 of 5 files reviewed, all discussions resolved.
src/i18n/index.ts line 56 at r2 (raw file):
Previously, jasonleenaylor (Jason Naylor) wrote…
do we want to bother if it wasn't updated?
Explanatory comment added.
jasonleenaylor
left a comment
jasonleenaylor
left a comment
There was a problem hiding this comment.
@jasonleenaylor reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec).
Alternative to #4063
Triggered by any of the following:
Also: fixes bug that layout doesn't switch to RTL when user changes UI language to Arabic. (It does switch when the UI language is automatically matching the browser language.)
This change is
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.