-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: add advanced datapath observability config option #1776
feat: add advanced datapath observability config option #1776
Conversation
60ae18d
to
32a8ae3
Compare
Hi @ericyz , WDYT about the variable names? I shortened them to
The default for the mode variable is |
/gcbrun |
I had to merge the latest master. |
/gcbrun |
@apeabody I have |
/gcbrun |
Hi @TheKangaroo - For I just triggered it now. If the previous result was red from an actual run, it's very possible that result was a flake. |
Thanks for the explanation @apeabody. |
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.
Thanks for the contribution @TheKangaroo!
Trying to disable these does not seem to take effect: monitoring_enable_observability_metrics = false
monitoring_observability_metrics_relay_mode = "DISABLED" Not sure if the problems lies in this module or the underlying |
@Markieta
but changed one to test this to
and I think everyting works as expected. Terraform needed like half an hour to complete, but after that, everything seems to be fine. The only thing that was a bit suprising to me ist that the cluster state show true if enalbed and nothing if disabled:
Disabled (after terraform change):
I think it would be better if you opened a new issue describing exactly what is happening. |
Thanks, perhaps it's only an issue for existing cluster (modules) that were created before adding these inputs? I've created an issue. |
fixes #1755
I'm not sure about the variable names though. Any ideas for shorter names?