-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add base_url_gap for Grafana #1141
Conversation
This reverts commit d43ddf4.
lib/config/logging.go
Outdated
@@ -91,6 +91,7 @@ func (l *LokiConfig) Validate() error { | |||
|
|||
type GrafanaConfig struct { | |||
BaseUrl *string `toml:"base_url"` | |||
BaseUrlCI *string `toml:"base_url_github_ci"` // Base URL for the dashboard via GAP proxy used on CI |
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.
// Base URL for the dashboard via GAP proxy used on CI
-> // URL of GAP proxy used on CI for Grafana
lib/logstream/logstream_handlers.go
Outdated
h.grafanaUrl = shortUrl | ||
} else { | ||
if h.loggingConfig.Grafana == nil || h.loggingConfig.Grafana.BaseUrl == nil { | ||
return "", errors.New("grafana base URL is not set in logging config") |
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.
I don't think we need to return error. Why not display what we can? which is the part of url without hostname? in that way if you forget to set it in the config you will still have an usable url like: /dashboard-ulr&var=1...
so you can just manually paste it after Grafana URL and get a working link
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.
@Tofel yeah, we can. I'll do it
Quality Gate passedIssues Measures |
Related PR in core: smartcontractkit/chainlink#14475
Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.
Why
The changes enhance Grafana logging configuration and log stream handling, particularly for CI environments. By introducing a new configuration parameter for Grafana to specify a base URL for dashboard access through a GAP proxy, it simplifies accessing dashboards from CI pipelines. Furthermore, the adjustment in log stream handlers to utilize this new URL for generating dashboard links ensures that the links are correctly generated for environments where direct access to Grafana might be restricted.
What
BaseUrlGap
field toGrafanaConfig
struct. This addition allows specifying a base URL for the Grafana dashboard through a GAP proxy, primarily used in CI environments.GetLogLocation
method to useBaseUrlGap
fromGrafanaConfig
if available. This change ensures that the generated Grafana dashboard URL is accessible in environments where a GAP proxy is needed for dashboard access.