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

feat(metrics/collector): support remote write proxy #3221

Merged
merged 1 commit into from Aug 21, 2023

Conversation

swiatekm-sumo
Copy link
Contributor

The remote write proxy was originally disabled for the otel metrics collector. Until we've added the ability to disable persistent connections in the upstream collector, it's still useful for load balancing.

Checklist

  • Changelog updated or skip changelog label added
  • Documentation updated
  • Template tests added for new features
  • Integration tests added or modified for major features

@swiatekm-sumo swiatekm-sumo force-pushed the feat/metrics-collector/remote-write-proxy branch 2 times, most recently from 281c358 to 817fe7e Compare August 21, 2023 08:34
@github-actions github-actions bot added the documentation documentation label Aug 21, 2023
@swiatekm-sumo swiatekm-sumo marked this pull request as ready for review August 21, 2023 08:59
@swiatekm-sumo swiatekm-sumo requested a review from a team as a code owner August 21, 2023 08:59
@swiatekm-sumo swiatekm-sumo force-pushed the feat/metrics-collector/remote-write-proxy branch from 817fe7e to 1e2f899 Compare August 21, 2023 09:02
@github-actions github-actions bot removed the documentation documentation label Aug 21, 2023
This is based on internal dogfooding. The collector is, in general, is
heavily memory-bound, much like Prometheus.
@swiatekm-sumo swiatekm-sumo force-pushed the feat/metrics-collector/remote-write-proxy branch from 1e2f899 to 70843e2 Compare August 21, 2023 09:03
@@ -364,6 +364,7 @@ var (
"otelcol_otelsvc_k8s_ip_lookup_miss",
"otelcol_otelsvc_k8s_other_deleted",
"kube_pod_container_status_waiting_reason",
"kube_pod_container_status_terminated_reason",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just another flaky metric I found while testing this change.

Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

Should we consider the same for logs?

@swiatekm-sumo
Copy link
Contributor Author

Should we consider the same for logs?

In that case I think it's better to wait for the upstream setting. Remote write proxy is already here, so using it is very little additional effort. This is also a much smaller problem for logs, where we have a lot more collectors than metadata Pods.

@swiatekm-sumo swiatekm-sumo merged commit 5bcac2f into main Aug 21, 2023
40 checks passed
@swiatekm-sumo swiatekm-sumo deleted the feat/metrics-collector/remote-write-proxy branch August 21, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants