fix: log related issues #4567
fix: log related issues #4567
Conversation
WalkthroughThe pull request introduces enhancements primarily focused on error handling and data processing across several components. Key changes include the addition of Changes
Possibly related PRs
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (5)
Files skipped from review as they are similar to previous changes (4)
Additional context usedBiome
Additional comments not posted (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- web/src/composables/useLogs.ts (13 hunks)
- web/src/composables/useStreams.ts (1 hunks)
- web/src/plugins/logs/Index.vue (1 hunks)
- web/src/plugins/metrics/Index.vue (1 hunks)
- web/src/plugins/traces/Index.vue (1 hunks)
Additional context used
Biome
web/src/composables/useLogs.ts
[error] 1256-1256: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Additional comments not posted (12)
web/src/composables/useStreams.ts (1)
281-284: LGTM!The updated implementation of the
removeSchemaFieldsfunction simplifies the logic by directly filtering out items with names_originaland_o2_idfrom theschemaarray. The change streamlines the function without altering its core functionality.web/src/plugins/traces/Index.vue (1)
92-92: Verify that error details are properly sanitized to prevent XSS.It's great to see more context being added to the error messages by including
errorDetail. However, please ensure that theSanitizedHtmlRenderercomponent is properly sanitizing the HTML content to prevent XSS vulnerabilities, especially if theerrorDetailcan contain user-generated content.Additionally, consider using a templating library or framework mechanism for safely generating the HTML, instead of relying on string concatenation which can be error-prone.
web/src/plugins/logs/Index.vue (1)
236-237: Enhance error message presentation. LGTM!The change improves the user experience by presenting the error details in a visually distinct manner. The line break and heading tag provide clearer separation and emphasis.
web/src/composables/useLogs.ts (9)
39-39: LGTM!The
convertDateToTimestampimport is a valid addition for date conversion utility.
139-139: Looks good!The additions of
errorDetailandhistogramIntervaltodefaultObjectare valid as per the intended enhancements mentioned in the summary.Also applies to: 170-170
1256-1258: Good catch!Extracting
error_detailfrom the API response and storing it improves error reporting granularity.Tools
Biome
[error] 1256-1256: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
2276-2278: Looks good!Storing
error_detailfrom the API response is consistent with the enhanced error reporting mechanism.
2301-2301: Looks good!Storing
error_detailfrom the API response aligns with the enhanced error reporting mechanism.
2380-2431: Nicely refactored!The new histogram data generation logic looks good:
- It replaces the commented out loop with a cleaner approach.
- Using
convertDateToTimestampto derive the start timestamp is a nice touch.- Iterating forward and backward from the start timestamp to populate results is intuitive.
This refactor aligns well with the intended enhancements to histogram data generation mentioned in the summary.
1848-1858: Good cleanup!Removing the commented out loop eliminates dead code and improves code hygiene.
2574-2574: Looks good!Storing
error_detailfrom the API response is consistent with the enhanced error reporting mechanism.
3549-3549: Looks good!Storing
error_detailfrom the API response aligns with the enhanced error reporting mechanism.
| :htmlContent="searchObj.data.errorMsg + '<br><h6>' + searchObj.data.errorDetail + '</h6>'" | ||
| /> |
There was a problem hiding this comment.
Improved error reporting but watch out for potential XSS vulnerability.
Displaying the errorDetail provides additional context for troubleshooting. However, if searchObj.data.errorDetail contains user-generated content, it could be a security risk to render it without sanitization. I recommend validating and sanitizing the errorDetail before rendering it to prevent potential XSS attacks.
Also, consider using a template literal for better readability:
- :htmlContent="searchObj.data.errorMsg + '<br><h6>' + searchObj.data.errorDetail + '</h6>'"
+ :htmlContent="`${searchObj.data.errorMsg}<br><h6>${sanitize(searchObj.data.errorDetail)}</h6>`"Ensure you have a robust sanitization function in place.
Committable suggestion was skipped due to low confidence.
bjp232004
force-pushed
the
o2_id_display
branch
from
5237be5 to
e1fc230
Compare
This PR fixes the issues in #4565
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
These updates collectively enhance the reliability and clarity of logging and error handling within the application.