-
Notifications
You must be signed in to change notification settings - Fork 543
add tbs storage limit and disk-related metrics #19568
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: main
Are you sure you want to change the base?
Conversation
🤖 GitHub commentsJust comment with:
|
|
This pull request does not have a backport label. Could you fix it @rubvs? 🙏
|
71a5fbb to
9eb164d
Compare
9eb164d to
082a2e7
Compare
💚 Build Succeeded
History
|
|
I also ran the test following the steps posted by Ruben, and can confirm that the metrics mapping appears. |
carsonip
left a comment
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.
implementation looks good, 1 nit on metric naming
| sm.storageMetrics.storageLimitGauge, _ = meter.Int64Gauge("apm-server.sampling.tail.storage.storage_limit") | ||
| sm.storageMetrics.diskUsedGauge, _ = meter.Int64Gauge("apm-server.sampling.tail.storage.disk_used") | ||
| sm.storageMetrics.diskTotalGauge, _ = meter.Int64Gauge("apm-server.sampling.tail.storage.disk_total") | ||
| sm.storageMetrics.diskUsageThresholdGauge, _ = meter.Int64Gauge("apm-server.sampling.tail.storage.disk_usage_threshold") |
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.
nit: I wonder if it will be more descriptive / self-explanatory with a suffix e.g. _pct. Also wonder if it better to range from 0 to 1 (correct to 2 d.p. maybe?) instead of 0-100.
| if sm.storageMetrics.diskTotalGauge != nil { | ||
| sm.storageMetrics.diskTotalGauge.Record(context.Background(), int64(usage.TotalBytes)) | ||
| } | ||
| // Record disk usage threshold as a percentage (0-100) |
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.
nit: remember to update comment if we change to 0-1
| func (sm *StorageManager) NewReadWriter(storageLimit uint64, diskUsageThreshold float64) RW { | ||
| // Store configured values for monitoring metrics | ||
| sm.configuredStorageLimit.Store(storageLimit) | ||
| // Store disk usage threshold as percentage (0-100) |
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.
ditto
Motivation/summary
Expose a new set of metrics to enhance TBS observability. The metric fields in the index data are tested in this PR, while the mappings are tested in the corresponding linked PRs. Changes in the elastic/elasticsearch#138131 PR are tested in conjunction with the changed in this PR.
See #15533 (comment) for the detailed overview.
Depends on PR:
Checklist
For functional changes, consider:
How to test these changes
Step 1: Ensure Elasticsearch & Kibana is running
Depends on elastic/elasticsearch#138131 with updates to
monitoring-beats.json.apm-server/docker-compose.ymland change the ES image.Step 2: Create APM Server config
Step 3: Start APM Server
Run APM Server binary directly:
Step 6: Verify Data
Verify data in index
.monitoring-beats-7-*Verify mappings for index
monitoring-beats-7-*Related issues
Part of #15533