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

Histogram metric fixes for container insights #124

Merged

Conversation

chadpatel
Copy link

Description:

  1. some metrics were incorrectly regex'd as histograms, they were not being emitted because the regex was wrong
  2. some container insights metrics are sourced as prometheus histograms, these were not getting converted to deltas, resulting in useless metrics
  3. fix linter errors in unrelated files

example of metrics that are actually gauge and not histogram:

# HELP apiserver_storage_size_bytes [ALPHA] Size of the storage database file physically allocated in bytes.
# TYPE apiserver_storage_size_bytes gauge
apiserver_storage_size_bytes{cluster="etcd-0"} 1.0092544e+07
apiserver_storage_db_total_size_in_bytes{endpoint="http://10.0.160.16:2379"} 1.0092544e+07
apiserver_storage_db_total_size_in_bytes{endpoint="http://10.0.32.16:2379"} 1.0080256e+07
apiserver_storage_db_total_size_in_bytes{endpoint="http://10.0.96.16:2379"} 1.0084352e+07

I don't have etcd_db_total_size_in_bytes in my cluster because it has been replaced with apiserver_storage_db_total_size_in_bytes

Link to tracking Issue:

Testing: end to end

gauge fixes:
Screenshot 2023-10-16 at 3 21 50 PM
Screenshot 2023-10-16 at 3 22 34 PM

delta histograms:
Screenshot 2023-10-17 at 11 00 04 AM

Documentation:

@@ -81,15 +81,11 @@ type numberDataPointSlice struct {

// histogramDataPointSlice is a wrapper for pmetric.HistogramDataPointSlice
type histogramDataPointSlice struct {
// Todo:(khanhntd) Calculate delta value for count and sum value with histogram
Copy link
Author

Choose a reason for hiding this comment

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

I removed these comments because I don't think it is accurate to say that it is a "bug" that we need to change the behavior on. Changing the behavior here is a breaking change for customers. To implement the "issue" we would need to make this configurable or onboard highly specific use-cases (like the one in this PR)

@chadpatel chadpatel merged commit 6e9de26 into amazon-contributing:aws-cwa-dev Oct 18, 2023
66 of 78 checks passed
@chadpatel chadpatel deleted the fix-metrics-oct17 branch October 18, 2023 18:24
lisguo pushed a commit to lisguo/opentelemetry-collector-contrib that referenced this pull request Oct 20, 2023
* bug fix - for container insights prometheus sourced metrics, convert them to deltas.  Also fix linter errors

* fix some metrics which are not actually histograms
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.

3 participants