-
Notifications
You must be signed in to change notification settings - Fork 777
[monitoring] Switch to grafana cloud for monitoring #4149
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: master
Are you sure you want to change the base?
Conversation
fbaf095
to
0ef4d40
Compare
4fe235f
to
74e3cfe
Compare
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.
Pull Request Overview
This PR migrates the monitoring infrastructure from a self-hosted solution to Grafana Cloud, replacing the previous "poc" endpoints with Grafana Cloud's managed services.
- Updates all hardcoded URLs from self-hosted monitoring instances to Grafana Cloud endpoints
- Adds URL configuration support for both Prometheus and Loki collectors
- Updates secret management to include URLs alongside credentials
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/reexecute/c/README.md | Updates Grafana URLs and dashboard IDs in documentation |
tests/fixture/tmpnet/yaml/promtail-daemonset.yaml | Adds Loki URL configuration and updates secret name |
tests/fixture/tmpnet/yaml/prometheus-agent.yaml | Adds Prometheus URL configuration and updates secret name |
tests/fixture/tmpnet/network.go | Updates Grafana URI and dashboard ID constants |
tests/fixture/tmpnet/monitor_processes.go | Refactors collector configuration to include URLs and removes hardcoded endpoints |
tests/fixture/tmpnet/monitor_kube.go | Updates Kubernetes secret creation to include URL configuration |
tests/fixture/tmpnet/check_monitoring.go | Removes URL validation and adds Prometheus path suffix for Grafana Cloud |
tests/fixture/tmpnet/README.md | Updates documentation with new URL requirements and endpoints |
tests/e2e/README.md | Updates documentation with new configuration requirements |
scripts/configure-local-metrics-collection.sh | Updates Grafana link generation |
.github/workflows/*.yml | Updates all workflow files to use new URL-based configuration |
.github/actions/run-monitored-tmpnet-cmd/action.yml | Adds URL inputs and updates default dashboard ID |
Comments suppressed due to low confidence (1)
tests/fixture/tmpnet/monitor_processes.go:464
- Error message is missing 'env' prefix for consistency with other error messages in the same function. Should be 'env var not set' to match the pattern used for other environment variables.
return "", "", "", fmt.Errorf("%s env var not set", passwordEnvVar)
8d9afbc
to
d8ffaf6
Compare
29c04e1
to
7ca3535
Compare
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.
LGTM pending a fix for the rate-limiting.
7d5177b
to
84248fc
Compare
- Also switch to require configuring collection endpoints. No need to expose this internal detail.
664ea18
to
69cf999
Compare
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.
LGTM - left one question on a comment I did not understand
Why this should be merged
Our current monitoring stack was only ever intended to be a stop-gap until a managed alternative became available. Grafana Cloud is that alternative.
How this was tested
Need to be documented in RELEASES.md?
N/A