-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: sync server metrics #662
Conversation
Include host source for host metrics in observer. Add throttling for Docker and kgw logs to reduce volume. Introduce internal metrics filtering to control metric usage and prevent excessive billing.
WalkthroughThe changes involve modifications to the Changes
Suggested reviewers
🪧 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 (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
deployments/observer/vector-sources.yml (4)
13-13
: LGTM! Consider updating the comment for collectors.The changes look good. Adding the 'host' collector will provide more comprehensive metrics about the host system. The modification in the 'cgroups' section improves the structure and readability of the configuration.
Consider updating the comment for collectors to reflect the current list:
collectors: # default are [cpu disk filesystem load host memory network cgroups]Also applies to: 28-31
34-37
: LGTM! Consider linking this source with its filter.The addition of the
in-metrics-internal
source is good for collecting internal metrics. The warning comment is crucial to prevent potential performance issues.Consider adding a comment that links this source to its corresponding filter (
out-metrics-internal-filter
) to make the relationship clearer:in-metrics-internal: type: internal_metrics # This source is filtered by the 'out-metrics-internal-filter' transform
52-78
: LGTM! Monitor the impact of new throttles.The addition of
logs-docker-throttle
andout-logs-kgw-throttle
transforms is a good approach to manage log volume. The exclusion conditions and throttle settings appear well-considered for their respective use cases.Consider implementing monitoring for these throttles to ensure they're not overly aggressive:
- Track the number of throttled events vs total events.
- Monitor for any unexpected gaps in important log data.
- Be prepared to adjust the
window_secs
andthreshold
values based on observed behavior in production.
80-93
: LGTM! Consider documenting metric selection rationale.The addition of the
out-metrics-internal-filter
is crucial for managing internal metrics and controlling costs. The filter is well-implemented and correctly linked to thein-metrics-internal
source.Consider adding a brief explanation for why these specific metrics (
component_discarded_events_total
andcomponent_errors_total
) were chosen to be retained. This documentation will be valuable for future maintenance and understanding of the system's monitoring strategy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- deployments/observer/vector-sources.yml (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
deployments/observer/vector-sources.yml (1)
46-49
: LGTM! Verify impact of source renaming.The renaming of 'out-logs-docker' to 'logs-docker' improves naming consistency. However, ensure that all references to this source throughout the configuration have been updated accordingly.
Run the following script to check for any remaining references to the old source name:
✅ Verification successful
Verified: 'out-logs-docker' successfully renamed to 'logs-docker'.
All references to 'out-logs-docker' have been updated accordingly with no remaining occurrences found in YAML files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to 'out-logs-docker' # Test: Search for 'out-logs-docker' in all YAML files rg --type yaml 'out-logs-docker'Length of output: 32
Script:
#!/bin/bash # Description: Locate all YAML files and search for 'out-logs-docker' # Find all YAML files and search for 'out-logs-docker' fd --extension yaml --type f | xargs rg 'out-logs-docker' --context 2 # Check the exit status if [ $? -eq 0 ]; then echo "References to 'out-logs-docker' found. Please update them accordingly." else echo "No references to 'out-logs-docker' found in YAML files." fiLength of output: 216
@outerlook |
Description
During Grafana setup, some modifications were made to servers. This PR synchronizes what was performed with the repo.
Related Problem
just related to #415
How Has This Been Tested?
Summary by CodeRabbit
New Features
Improvements