Move fatal logs from prom2json file to main #27
Conversation
Signed-off-by: nmengin <nicolas@containo.us>
nmengin
force-pushed
the
feature/replace-fatal-by-error-logs
branch
from
8a67a46
to
3cd8875
Compare
There was a problem hiding this comment.
Sure, this makes sense if using the code as a library. To my excuse, the code was never meant to be used as a library. It's really just a tool and not very sophisticated. However, if it is useful for you, I won't stand in your way.
I have a commented on a few nits. Should be easy to address, and then we are good to go.
prom2json.go
Outdated
| req, err := http.NewRequest("GET", url, nil) | ||
| if err != nil { | ||
| log.Fatalf("creating GET request for URL %q failed: %s", url, err) | ||
| return fmt.Errorf("creating GET request for URL %q failed: %s", url, err) |
There was a problem hiding this comment.
Perhaps use %v to render err as you have done below?
prom2json.go
Outdated
| } | ||
| req.Header.Add("Accept", acceptHeader) | ||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| log.Fatalf("executing GET request for URL %q failed: %s", url, err) | ||
| return fmt.Errorf("executing GET request for URL %q failed: %s", url, err) |
| } | ||
| defer resp.Body.Close() | ||
| if resp.StatusCode != http.StatusOK { | ||
| log.Fatalf("GET request for URL %q returned HTTP status %s", url, resp.Status) | ||
| return fmt.Errorf("GET request for URL %q returned HTTP status %s", url, resp.Status) |
There was a problem hiding this comment.
resp.Status returns a string.
I can replace %s by %v but not sure it makes sense here.
WDYT @beorn7?
There was a problem hiding this comment.
(But just FYI: %v used on a string renders the string the same way as %s. I often just use %v because it is more flexible.)
cmd/prom2json/main.go
Outdated
| dto "github.com/prometheus/client_model/go" | ||
|
|
||
| "github.com/prometheus/common/log" |
There was a problem hiding this comment.
Please don't re-arrange the import lines. The grouping in blocks follows the general Prometheus style.
(stdlib, then unnamed from other repos, then named from other repos, then unnamed from same repo, then named from same repo)
prom2json.go
Outdated
| dto "github.com/prometheus/client_model/go" | ||
| "github.com/prometheus/common/expfmt" |
There was a problem hiding this comment.
Please leave the grouping of import lines intact. See other file.
Signed-off-by: nmengin <nicolas@containo.us>
|
Thanks. Merging now… |
This PR moves fatal logs from the prom2json functions to the main.
Indeed, the prom2json functions can be used as a library and the
log.Fatalstops the program which calls it.The modifications allow removing the fatal behavior from the prom2json and manage the errors in the main.
Thus, the current behavior of prom2json is not modified.
WDYT @beorn7 ?