Expose chaos controller metrics in helm chart #228
Conversation
Signed-off-by: yeya24 <yb532204897@gmail.com>
| # targetPort: http | ||
| # protocol: TCP | ||
| # name: http | ||
| - port: 8080 |
There was a problem hiding this comment.
I remove the port var here since I think it is useless. We can just use a hardcode 8080 here
Codecov Report
@@ Coverage Diff @@
## master #228 +/- ##
=======================================
Coverage 43.64% 43.64%
=======================================
Files 18 18
Lines 685 685
=======================================
Hits 299 299
Misses 353 353
Partials 33 33Continue to review full report at Codecov.
|
| containerPort: 9443 # Customize containerPort | ||
| - name: http | ||
| containerPort: 8080 |
There was a problem hiding this comment.
Can we switch to an infrequently used port? eg: 20180
There was a problem hiding this comment.
May I ask any difference between 8080 and 20180? Since we are not using hostport here and any access to controller-manager is via Service. So I think any port is okay. I am not sure if I miss something.
There was a problem hiding this comment.
I think 8080 is used for common service and this port may be used by chaos-dashboard or the other services. At here, we just need to export our metric.
There was a problem hiding this comment.
Metric is also an HTTP endpoint. And the default value is 8080 as well https://github.com/pingcap/chaos-mesh/blob/09afa9d7074b88bdb143e6f8bcbdd9e7b69c4b3c/cmd/controller-manager/main.go#L67. Do you think we need to change it?
There was a problem hiding this comment.
OK. I will update them in this PR
There was a problem hiding this comment.
Updated. Please take a look
Signed-off-by: yeya24 <yb532204897@gmail.com>
Signed-off-by: yeya24 yb532204897@gmail.com
What problem does this PR solve?
What is changed and how does it work?
This is a prerequisite of adding Prometheus to chaos-mesh. Exposing 8080 in chaos controller-manager enables Prometheus to get metrics from controller-manager.
Check List
Tests
Code changes
Side effects
Related changes
Does this PR introduce a user-facing change?: