Skip to content
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

Add metrics for OS and OSD processes #83

Merged
merged 3 commits into from
Dec 13, 2023

Conversation

gaiksaya
Copy link
Member

Description

Add metrics for OS and OSD processes that later can be used to add alarms. Also modified the unit of measurement for mem and disk usage from NONE (default) to Percent
https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html

Issues Resolved

Part of #74

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Sayali Gaikawad <[email protected]>
Copy link

codecov bot commented Dec 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (68679fc) 79.05% compared to head (e96237b) 79.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #83      +/-   ##
==========================================
+ Coverage   79.05%   79.24%   +0.18%     
==========================================
  Files           6        6              
  Lines         444      448       +4     
  Branches      131      132       +1     
==========================================
+ Hits          351      355       +4     
  Misses         93       93              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rishabh6788
Copy link
Collaborator

@gaiksaya LGTM, did you deploy a stack with these changes to confirm if it is working as expected?

@gaiksaya
Copy link
Member Author

@gaiksaya LGTM, did you deploy a stack with these changes to confirm if it is working as expected?

Yes works as expected

Comment on lines 383 to 402
'cpu_usage',
'cpu_time_system',
'cpu_time_user',
'read_bytes',
'write_bytes',
'pid_count',
],
metrics_collection_interval: 10,
},
];
if (props.dashboardsUrl !== 'undefined') {
procstatConfig.push({
pattern: 'opensearch-dashboards',
measurement: [
'cpu_usage',
'cpu_time_system',
'cpu_time_user',
'read_bytes',
'write_bytes',
'pid_count',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rishabh6788 ,

Not sure if we need all of these metrics. Just pid_count will suffice right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we don't need just the process cpu and other metrics. PID should be enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it. Will only give pid_count metric now. Thanks!

Signed-off-by: Sayali Gaikawad <[email protected]>
@rishabh6788 rishabh6788 merged commit 5ea6747 into opensearch-project:main Dec 13, 2023
5 checks passed
@gaiksaya gaiksaya deleted the monitoring-alarms branch December 13, 2023 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants