fix(backend): adding empty port as supported for tempo #2704
Conversation
| @@ -25,7 +25,7 @@ type HttpClient struct { | |||
| } | |||
|
|
|||
| func NewHttpClient(name string, config *datastoreresource.HttpClientConfig, callback HttpCallback) DataSource { | |||
| endpoint, _ := url.Parse(config.Url) | |||
| endpoint, _ := urlx.Parse(config.Url) | |||
There was a problem hiding this comment.
This fixes an annoying but with the URL lib not setting up the expected values when parsing the url
There was a problem hiding this comment.
What is the main difference between using url.Parse and urlx.Parse? Is there a way to keep url.Parse?
There was a problem hiding this comment.
Here's the main difference https://github.com/goware/urlx#difference-between-urlx-and-neturl
We are using it here as well https://github.com/kubeshop/tracetest/blob/0bcfa3a3624e29ec8c71d534f3341dd12c315f2d/server/executor/trigger/http.go
There was a problem hiding this comment.
What is my concern: https://github.com/goware/urlx is a small library without changes for 2 years and we are using only a portion of its capability.
My thought is that we can transform the url.URL return like this lib does.
There was a problem hiding this comment.
Hmm... In that case, we can keep it and mark a tech debt to eliminate this dependency in the future.
| @@ -23,7 +23,7 @@ import ( | |||
| ) | |||
|
|
|||
| func tempoDefaultPorts() []string { | |||
| return []string{"9095"} | |||
| return []string{"9095", ""} | |||
There was a problem hiding this comment.
For http urls like www.grafana.com/traces the port is "not" setup
Ideally, I'd like us to change this logic to allow each integration read the config file and based on the values decide what is the expected list of port instead of having a static list
xoscar
deleted the
2155-port-number-when-setting-up-grafana-with-http-not-shown-correctly
branch
fix(backend): adding empty port as supproted for tempo
This PR removes the warning for http ports when using tempo as a data store
Changes
Fixes
Checklist