-
Notifications
You must be signed in to change notification settings - Fork 226
Replace telegraf prometheus plugin #1709
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
Replace telegraf prometheus plugin #1709
Conversation
processor has no functionality. it is included in the prometheus -> emf pipeline
// Validate does not check for unsupported dimension key-value pairs, because those | ||
// get silently dropped and ignored during translation. |
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'm guessing this was copied from somewhere. Consider removing it for now.
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.
yeah, copied from somewhere else. I'll remove the comments
) | ||
|
||
type prometheusAdapaterProcessor struct { | ||
*Config |
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: Is there a reason we want to embed the Config?
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.
No particular reason. Following convention from other processors (kueueattributes, gpuattributes, ec2tagger)
"go.uber.org/zap" | ||
) | ||
|
||
type prometheusAdapaterProcessor struct { |
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: typo
type prometheusAdapaterProcessor struct { | |
type prometheusAdapterProcessor struct { |
"go.uber.org/zap" | ||
) | ||
|
||
func TestProcessMetricsForKueueMetrics(t *testing.T) { |
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: Looks like this was copied from the kueueattributes processor?
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.
yeah it was. I'll fix the name
- batch/prometheus/cloudwatchlogs | ||
receivers: | ||
- telegraf_prometheus | ||
- prometheus |
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.
We'll want to add a name to this receiver (e.g. prometheus/cloudwatchlogs
) so we can differentiate it from the one configured in the metrics section. These can and should be distinct prometheus receivers unless we find a way to dedup them.
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.
gotcha, thanks. will update
Description of the issue
The CloudWatch Agent does not support pushing Prometheus exponential histograms to CloudWatch Logs via EMF. Any exponential histograms are simply dropped by the agent. This is because the telegraf-based prometheus receiver doesn't support them. The OpenTelemetry one does support exponential histograms, so we are working on migrating all prometheus receiver uses to the OpenTelemetry plugin.
Description of changes
Staging some changes into a non-personal feature branch for replacing the telegraf-based Prometheus receiver with the OpenTelemetry-based Prometheus receciver.
The prometheus adapter processor will be used to translate the metrics emitted by the OpenTelemetry prometheus receiver to match what the Telegraf prometheus receiver would have output to maintain backwards compatibility.
This change "breaks" the current Prometheus -> EMF pipeline in the sense that the metrics do not show up as they used to, so these changes are not yet ready to merge to main.
License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
Run Prometheus server to vend prometheus metrics. Configure agent to collect prometheus metrics and push to CW Logs. Metrics show up in CloudWatch, e.g.
Requirements
Before commit the code, please do the following steps.
make fmt
andmake fmt-sh
make lint