-
Notifications
You must be signed in to change notification settings - Fork 115
feat(dedicated.cdn): service log-tail migration to l2c #21811
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates the dedicated CDN service logs tab from a legacy implementation to the new l2c (log-to-customer) framework. The migration replaces custom log-tailing functionality with reusable components from @ovh-ux/manager-log-to-customer.
Changes:
- Replaced legacy log viewing UI with l2c components for live-tail and data-streams
- Updated translation files to support new log descriptions and removed obsolete keys
- Introduced new services, routing configurations, and controllers following l2c patterns
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/translations/*.json | Updated translation keys from legacy format to l2c-compatible descriptions |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.service.js | New service to fetch CDN log kinds from API |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.routing.js | New routing configuration for logs with l2c integration |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.module.js | Module setup importing l2c dependencies |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.html | Template using l2c live-tail component |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.controller.js | Controller managing l2c component bindings |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.constants.js | Constants for tracking, guide links, and log configuration |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/live-tail/* | Live-tail component wrapping l2c functionality |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/data-streams/* | Data streams listing and subscription components |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/index.js | Updated module to integrate new l2c components |
| packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/cdn-dedicated-manage-logs.* | Removed legacy log controller, routes, and template |
Comments suppressed due to low confidence (1)
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/live-tail/live-tail.template.html:1
- Corrected spelling of 'LOG_SUSBSCRIPTION' to 'LOG_SUBSCRIPTION'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.routing.js
Outdated
Show resolved
Hide resolved
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.html
Outdated
Show resolved
Hide resolved
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.constants.js
Outdated
Show resolved
Hide resolved
11d99ae to
63ffcc3
Compare
...dedicated/client/app/cdn/dedicated/manage/logs/data-streams/translations/Messages_de_DE.json
Outdated
Show resolved
Hide resolved
...manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/translations/Messages_de_DE.json
Show resolved
Hide resolved
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/data-streams/index.js
Show resolved
Hide resolved
...ager/apps/dedicated/client/app/cdn/dedicated/manage/logs/data-streams/data-streams.module.js
Outdated
Show resolved
Hide resolved
d737d23 to
dcb7f05
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const CDN_DEDICATED_LOGS_GUIDE = { | ||
| IE: {}, | ||
| FR: {}, | ||
| }; | ||
|
|
||
| CDN_DEDICATED_LOGS_GUIDE.NL = CDN_DEDICATED_LOGS_GUIDE.IE; | ||
| CDN_DEDICATED_LOGS_GUIDE.WE = CDN_DEDICATED_LOGS_GUIDE.IE; | ||
| CDN_DEDICATED_LOGS_GUIDE.WW = CDN_DEDICATED_LOGS_GUIDE.IE; |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CDN_DEDICATED_LOGS_GUIDE.IE and CDN_DEDICATED_LOGS_GUIDE.FR are referenced but never defined, causing these assignments to set all values to undefined. Define the base guide URLs for IE and FR subsidiaries before these assignments.
| apiVersion: '<', | ||
| trackingHits: '<', | ||
| logKinds: '<', | ||
| logSubscriptionApiData: '=', |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using two-way binding ('=') is an anti-pattern for components in AngularJS. This should use one-way binding ('<') instead, with data changes communicated through callbacks.
| logSubscriptionApiData: '=', | |
| logSubscriptionApiData: '<', |
dcb7f05 to
e418e5f
Compare
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/logs.routing.js
Outdated
Show resolved
Hide resolved
e418e5f to
2002384
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/live-tail/live-tail.template.html:1
- This template file is identical to logs.html. The duplication should be eliminated by either removing this file if it's unused, or consolidating into a shared template if both are needed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/manager/apps/dedicated/client/app/cdn/dedicated/manage/logs/live-tail/index.js
Show resolved
Hide resolved
2002384 to
1baf1ae
Compare
0af58b9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
resolves: #CDN-1385 Signed-off-by: Etienne GAUTIER <etienne.gautier@ovhcloud.com>
0af58b9 to
de3590a
Compare
de3590a to
55bc246
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| redirectTo, | ||
| resolve: { | ||
| breadcrumb: /* @ngInject */ ($translate) => | ||
| $translate.instant('cdn_logs'), |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The breadcrumb translation key was changed to cdn_logs, but the provided FR translations diff only adds cdn_logs_description and removes the previous cdn_tabs_logs key. This will likely display the raw key or an empty breadcrumb for fr_FR. Add a cdn_logs entry in the translations (for at least fr_FR) or update the breadcrumb to use an existing key.
| $translate.instant('cdn_logs'), | |
| $translate.instant('cdn_logs_description'), |
| getLogKinds() { | ||
| return this.$http.get('/cdn/dedicated/log/kind').then(({ data }) => data); |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint doesn’t include the dedicated CDN productId/serviceName, while the other log endpoints in routing are scoped under /cdn/dedicated/${productId}/log/.... If the API is service-scoped (which the other URLs strongly suggest), this call will hit the wrong route (or 404) and make the logs page redirect away due to empty kinds. Consider passing productId into getLogKinds(productId) (or building the URL from the same base) so it targets /cdn/dedicated/${productId}/log/kind (or the correct service-scoped path).
| getLogKinds() { | |
| return this.$http.get('/cdn/dedicated/log/kind').then(({ data }) => data); | |
| getLogKinds(productId) { | |
| return this.$http | |
| .get(`/cdn/dedicated/${productId}/log/kind`) | |
| .then(({ data }) => data); |
| import routing from './data-streams.routing'; | ||
| import controller from './data-streams.controller'; | ||
|
|
||
| const moduleName = 'cdnDedicatedLogsDataStreams'; | ||
|
|
||
| angular | ||
| .module(moduleName, [ | ||
| uiRouter, | ||
| 'ngTranslateAsyncLoader', | ||
| 'oui', | ||
| 'pascalprecht.translate', | ||
| ovhManagerLogToCustomer, | ||
| ]) | ||
| .controller('CdnDedicatedLogsDataStreamsController', controller) | ||
| .config(routing) | ||
| .run(/* @ngTranslationsInject:json ./translations */); |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A data-streams.component.js file is added in this PR, but this module doesn’t import/register it (no .component(...)). Either wire the component into the module (import it and register it) or remove the unused component file to avoid dead code and confusion about which template/controller is actually used.
resolves: #CDN-1385
Description
This PR aims to replace the dedicated CDN service logs tab by the new l2c features, routes and components.