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

Remote write 2 #17

Merged
merged 31 commits into from
May 4, 2022
Merged

Remote write 2 #17

merged 31 commits into from
May 4, 2022

Conversation

mmanciop
Copy link
Contributor

@mmanciop mmanciop commented Nov 9, 2021

Clean up the implementation of Prometheus Remote Write in Grafana Agent.

@simskij simskij self-assigned this Jan 7, 2022
@Abuelodelanada Abuelodelanada requested a review from a team January 7, 2022 17:59
Abuelodelanada
Abuelodelanada previously approved these changes Jan 7, 2022
lib/charms/prometheus_k8s/v0/prometheus_remote_write.py Outdated Show resolved Hide resolved
lib/charms/prometheus_k8s/v0/prometheus_scrape.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
sed-i
sed-i previously approved these changes Jan 18, 2022
Copy link
Contributor

@sed-i sed-i left a comment

Choose a reason for hiding this comment

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

Note that both prom libs have PRs in review.

lib/charms/prometheus_k8s/v0/prometheus_scrape.py Outdated Show resolved Hide resolved
@mmanciop
Copy link
Contributor Author

For debugging purposes, we should annotate the Grafana Agent config with YAML comments how the IPs of the prometheus instances we remote-write to maps to their Juju identity:

integrations:
  prometheus_remote_write:
  - url: http://10.1.250.232:9090/api/v1/write  # Juju model X;Juju application Y; Juju unit Z
  - url: http://10.1.250.233:9090/api/v1/write # Juju model X;Juju application Y; Juju unit W
prometheus:
  configs:
  - name: agent_scraper
    remote_write:
    - url: http://10.1.250.232:9090/api/v1/write  # Comment same as previous case
    - url: http://10.1.250.233:9090/api/v1/write

metadata.yaml Outdated Show resolved Hide resolved
@mmanciop
Copy link
Contributor Author

In local testing, the Grafana Agent built from this PR is failing to push downstream to Prometheus the alert rules from the Cassandra charm.

src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
@simskij simskij dismissed stale reviews from sed-i and Abuelodelanada via a8f38b1 March 28, 2022 11:20
@dstathis dstathis requested review from sed-i and rbarry82 April 12, 2022 23:01
Copy link
Contributor

@sed-i sed-i left a comment

Choose a reason for hiding this comment

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

Re itest failing: I see similar errors too elsewhere.

tests/unit/test_charm.py Show resolved Hide resolved
@github-actions
Copy link

Libraries are not up to date with their remote counterparts. If this was
not intentional, run charmcraft fetch-libs and commit the updated libs
to your PR branch.

stdout
Library charms.loki_k8s.v0.loki_push_api was already up to date in version 0.10.                                                                               
Library charms.observability_libs.v0.kubernetes_service_patch was already up to date in version 0.6.                                                           
Library charms.prometheus_k8s.v0.prometheus_remote_write updated to version 0.4.                                                                               
Library charms.prometheus_k8s.v0.prometheus_scrape updated to version 0.18.    

stderr

sed-i
sed-i previously approved these changes Apr 28, 2022
src/charm.py Outdated Show resolved Hide resolved
tests/unit/test_charm.py Show resolved Hide resolved
Copy link
Contributor Author

@mmanciop mmanciop left a comment

Choose a reason for hiding this comment

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

Alert rule propagation from a prometheus_scrape relation over a prometheus_remote_write one does not seem to work in the following scenario:

juju add-model cos
juju deploy cos-lite --channel=edge --trust
juju offer prometheus:receive-remote-write prw
juju add-model agent
juju deploy ./*.charm grafana-agent --resource agent-image='grafana/agent:v0.20.1'
juju deploy spring-music --channel=edge
juju relate spring-music:metrics-endpoint grafana-agent
juju consume cos.prw prometheus
juju relate grafana-agent prometheus

Then open the Prometheus ui, go to the alert rules, despair: metrics from Spring Music come in, alert rules for Spring Music do not, while the alert rules for Grafana Agent itself do.

tests/unit/test_charm.py Show resolved Hide resolved
@dstathis
Copy link
Contributor

Alert rule propagation does not seem to work in the following scenario:

  1. juju add-model cos
  2. juju deploy cos-lite --channel=edge --trust
  3. juju offer prometheus:send-remote-write prw
  4. juju add-model agent
  5. juju deploy ./*.charm grafana-agent ...
  6. juju deploy spring-music --channel=edge
  7. juju relate spring-music:metrics-endpoint grafana-agent
  8. juju consume cos.prw prometheus
  9. juju relate grafana-agent prometheus
  10. Open the Prometheus ui, go to the alert rules, despair

Metrics from Spring Music come in, alert rules do not

@mmanciop I tried this and it's working just fine for me. Could you possibly do it again and copy paste the exact commands?

@mmanciop
Copy link
Contributor Author

mmanciop commented Apr 29, 2022

Alert rule propagation does not seem to work in the following scenario:

  1. juju add-model cos
  2. juju deploy cos-lite --channel=edge --trust
  3. juju offer prometheus:send-remote-write prw
  4. juju add-model agent
  5. juju deploy ./*.charm grafana-agent ...
  6. juju deploy spring-music --channel=edge
  7. juju relate spring-music:metrics-endpoint grafana-agent
  8. juju consume cos.prw prometheus
  9. juju relate grafana-agent prometheus
  10. Open the Prometheus ui, go to the alert rules, despair

Metrics from Spring Music come in, alert rules do not

@mmanciop I tried this and it's working just fine for me. Could you possibly do it again and copy paste the exact commands?

I think I found the PBCAK. I was building the agent from the wrong commit.

@github-actions
Copy link

Libraries are not up to date with their remote counterparts. If this was
not intentional, run charmcraft fetch-libs and commit the updated libs
to your PR branch.

stdout
Library charms.loki_k8s.v0.loki_push_api was already up to date in version 0.10.                                                                               
Library charms.observability_libs.v0.kubernetes_service_patch was already up to date in version 0.6.                                                           
Library charms.prometheus_k8s.v0.prometheus_remote_write updated to version 0.4.                                                                               
Library charms.prometheus_k8s.v0.prometheus_scrape updated to version 0.18.    

stderr

@github-actions
Copy link

Libraries are not up to date with their remote counterparts. If this was
not intentional, run charmcraft fetch-libs and commit the updated libs
to your PR branch.

stdout
Library charms.loki_k8s.v0.loki_push_api was already up to date in version 0.10.                                                                               
Library charms.observability_libs.v0.kubernetes_service_patch was already up to date in version 0.6.                                                           
Library charms.prometheus_k8s.v0.prometheus_remote_write updated to version 0.4.                                                                               
Library charms.prometheus_k8s.v0.prometheus_scrape updated to version 0.18.    

stderr

@github-actions
Copy link

Libraries are not up to date with their remote counterparts. If this was
not intentional, run charmcraft fetch-libs and commit the updated libs
to your PR branch.

stdout
Library charms.loki_k8s.v0.loki_push_api was already up to date in version 0.10.                                                                               
Library charms.observability_libs.v0.kubernetes_service_patch was already up to date in version 0.6.                                                           
Library charms.prometheus_k8s.v0.prometheus_remote_write updated to version 0.4.                                                                               
Library charms.prometheus_k8s.v0.prometheus_scrape updated to version 0.18.    

stderr

@github-actions
Copy link

Libraries are not up to date with their remote counterparts. If this was
not intentional, run charmcraft fetch-libs and commit the updated libs
to your PR branch.

stdout
Library charms.loki_k8s.v0.loki_push_api was already up to date in version 0.10.                                                                               
Library charms.observability_libs.v0.kubernetes_service_patch was already up to date in version 0.6.                                                           
Library charms.prometheus_k8s.v0.prometheus_remote_write updated to version 0.4.                                                                               
Library charms.prometheus_k8s.v0.prometheus_scrape updated to version 0.18.    

stderr

@github-actions
Copy link

github-actions bot commented May 2, 2022

Libraries are not up to date with their remote counterparts. If this was
not intentional, run charmcraft fetch-libs and commit the updated libs
to your PR branch.

stdout
Library charms.loki_k8s.v0.loki_push_api was already up to date in version 0.10.                                                                               
Library charms.observability_libs.v0.kubernetes_service_patch was already up to date in version 0.6.                                                           
Library charms.prometheus_k8s.v0.prometheus_remote_write updated to version 0.4.                                                                               
Library charms.prometheus_k8s.v0.prometheus_scrape updated to version 0.18.    

stderr

@dstathis dstathis requested a review from sed-i May 2, 2022 16:34
Copy link
Contributor

@rbarry82 rbarry82 left a comment

Choose a reason for hiding this comment

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

Tests are solid, love seeing the mocking gone, rewrite rule is nice.

Feel free to just resolve any of my comments if you don't expect another push. They're not critical in any way.

CONTRIBUTING.md Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
@dstathis dstathis merged commit 9c64de0 into main May 4, 2022
@dstathis dstathis deleted the remote_write_2 branch May 4, 2022 16:07
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.

None yet

7 participants