Copilot commented on code in PR #188:
URL: https://github.com/apache/skywalking-helm/pull/188#discussion_r3285993308
##########
chart/skywalking/values.yaml:
##########
@@ -46,8 +46,12 @@ oap:
# null (or remove) if the deployed OAP version predates 10.5 — Horizon
# admin features will be unavailable in that case.
admin: 17128
- # zipkinreceiver: 9411
- # zipkinquery: 9412
+# zipkin_receiver: 9411
+# zipkin_query: 9412
+# promql: 9090
+# logql: 3100
+# traceql: 3200
+# metrics: 1234
Review Comment:
The commented example port keys here (`zipkin_receiver`, `zipkin_query`,
etc.) are (1) not indented under `oap.ports`, so uncommenting them will produce
invalid YAML, and (2) don’t match the existing values keys used elsewhere in
the chart (e.g., templates reference `oap.ports.zipkinreceiver` /
`oap.ports.zipkinquery`). Update these commented examples to use the
established key names and correct indentation under `ports:`.
##########
chart/skywalking/templates/ui-configmap.yaml:
##########
@@ -26,6 +26,7 @@ operator left them unset.
"oap" (dict
"queryUrl" (printf "http://%s:%v"; (include "skywalking.oap.fullname" .)
.Values.oap.ports.rest)
"adminUrl" (printf "http://%s:%v"; (include "skywalking.oap.fullname" .)
(default 17128 .Values.oap.ports.admin))
+ "zipkinUrl" (printf "http://%s:%v/zipkin"; (include
"skywalking.oap.fullname" .) .Values.oap.ports.zipkin_query)
)
Review Comment:
`zipkinUrl` is built from `.Values.oap.ports.zipkin_query`, but the chart’s
existing key naming for these ports is `zipkinquery` (see README and OAP
deployment template). As written, this will render an invalid URL (`:<no
value>`) unless users add a new `zipkin_query` value that the rest of the chart
doesn’t recognize. Use the existing `zipkinquery` key and only set `zipkinUrl`
when that port is configured (or provide a consistent default + corresponding
port/env wiring).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
