fix: update distributed_tracing.go to include a service proxy #502
Conversation
zhuje
requested review from
danielmellado and
simonpasquier
and removed request for
a team
|
Hi @zhuje. Thanks for your PR. I'm waiting for a rhobs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
zhuje
changed the title
| if plugin.Spec.TroubleshootingPanel != nil { | ||
| if plugin.Spec.TroubleshootingPanel.Korrel8r.Name != "" { | ||
| proxyName = plugin.Spec.TroubleshootingPanel.Korrel8r.Name | ||
| } | ||
| if plugin.Spec.TroubleshootingPanel.Korrel8r.Namespace != "" { | ||
| proxyNamespace = plugin.Spec.TroubleshootingPanel.Korrel8r.Namespace | ||
| } | ||
| } |
There was a problem hiding this comment.
The code here is for korrel8r and the troubleshooting panel. But I think the intention here is to define this fields in the DistributedTracingConfig spec so they can be configured if not using the defaults
There was a problem hiding this comment.
I removed this in the latest commit.
| @@ -29,13 +30,36 @@ func createDistributedTracingPluginInfo(plugin *uiv1alpha1.UIPlugin, namespace, | |||
| extraArgs = append(extraArgs, fmt.Sprintf("-features=%s", strings.Join(features, ","))) | |||
| } | |||
|
|
|||
| proxyName, proxyNamespace := "distributed-tracing-console-plugin", "openshift-tracing" | |||
There was a problem hiding this comment.
| proxyName, proxyNamespace := "distributed-tracing-console-plugin", "openshift-tracing" | |
| defaultServiceName, defaultServiceNamespace := "distributed-tracing-console-plugin", "openshift-tracing" |
There was a problem hiding this comment.
Name and namespace are passed in as parameters to the createDistributedTracingPluginInfo function and we should use them so that the service and the proxy don't get out of sync.
There was a problem hiding this comment.
I updated this in the latest commit.
Signed-off-by: Jenny Zhu <[email protected]>
zhuje
changed the title
|
/lgtm |
|
/ok-to-test |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgbernalp, simonpasquier, zhuje The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Updates are needed to reflect the changes in PR: openshift/distributed-tracing-console-plugin#15, which allows the distributed-tracing-console-plugin to use this proxy service at the endpoint :
/api/proxy/plugin/distributed-tracing-console-plugin/backend