-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore: adds dashboard. #222
Conversation
example/grafana/dashboard.json
Outdated
@@ -0,0 +1,234 @@ | |||
{ |
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.
If this or other files are copied from somewhere it would be good to have a link somewhere.
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.
Actually this was done myself from try/fail. Couldn't find a decent dashboard to work for me (there are many and also an official but I couldn't make them work)
a585957
to
0eb0c07
Compare
Shall we borrow some metrics from https://grafana.com/grafana/dashboards/13277-istio-wasm-extension-dashboard/ ? cc @zirain |
absolutely, you should. |
example/grafana/grafana.ini
Outdated
instance_name = "grafana" | ||
|
||
[security] | ||
admin_user = admin |
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.
I think it's better to set these as environment variables in the docker-compose rather than a file to avoid making an example of insecure values
https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#override-configuration-with-environment-variables
GC_SECURITY_ADMIN_USER
/ _PASSWORD
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.
I would not overcomplicate it as this is just for example purposes and inside the scope of docker compose.
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.
Especially if moving the files it's probably ok - though I don't know if it's particularly complicated using variables either :P
d8419f5
to
021e3a3
Compare
Reviews should be addressed, PTAL. I would love to merge an initial memory dashboard also for doing some comparisons about #252. |
Often people talks about memory consumption by coraza-proxy-wasm, however it would be easier to understand what is going on with the config applied plus a dashboard screenshot, hence this PR.
Related to #191 #219
Closes #221