Skip to content

Conversation

@shu-unifra
Copy link
Contributor

@shu-unifra shu-unifra commented Oct 22, 2024

Description

Enable monitoring with Grafana, Prometheus, and Alertmanager.

Related PRs

scroll-tech/scroll-documentation#376
scroll-tech/scroll-sdk-cli#3

Merge Notes

This PR better be merged in sync with the other repository's PR to prevent breaking changes or inconsistencies. Please ensure these PRs are merged simultaneously.

@shu-unifra shu-unifra marked this pull request as ready for review October 24, 2024 12:43
sbaizet-ledger
sbaizet-ledger previously approved these changes Oct 25, 2024
Copy link
Member

@sbaizet-ledger sbaizet-ledger left a comment

Choose a reason for hiding this comment

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

I couldn't test because of right issues but I checked the commit, it seems fine to me.

@shu-unifra
Copy link
Contributor Author

I couldn't test because of right issues but I checked the commit, it seems fine to me.

Sorry for the mistake. I think now is available.

@shu-unifra
Copy link
Contributor Author

@sbaizet-ledger I just found a bug related to Prometheus service discovery and have pushed a commit. Please review it again. Thank you!

@dghelm
Copy link
Contributor

dghelm commented Oct 26, 2024

@sbaizet-ledger can you also advise on any issues needed to make the work for devnet?

Copy link
Contributor

@dghelm dghelm left a comment

Choose a reason for hiding this comment

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

Mostly looks good, but untested at the moment.

I've never seen nginx.ingress.kubernetes.io/ssl-redirect: "true" -- I should probably add this to the CLI tool!

Copy link
Contributor

@dghelm dghelm left a comment

Choose a reason for hiding this comment

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

LGTM -- Followup: integrating into devnet and its default config.toml

@dghelm dghelm merged commit 3521479 into scroll-tech:develop Oct 28, 2024
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