Send correct logs to Saturn logger #40
Conversation
| @@ -84,5 +85,6 @@ func main1() int { | |||
| log.Println(err) | |||
| return 1 | |||
| } | |||
| time.Sleep(5 * time.Second) | |||
There was a problem hiding this comment.
This was for some debugging to ensure that we do eventually see the response of the logging endpoint before shutting down. Lemme remove this.
There was a problem hiding this comment.
@aarshkshah1992 but.. did you push the removal? 🙃 I still see it :D
log.go
Outdated
| req.Header.Set("Content-Type", "application/json") | ||
| req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", l.jwt)) | ||
|
|
||
| resp, err := l.client.Do(req) |
There was a problem hiding this comment.
rather than having this a specific saturn logging needs a jwt bearer token - which is a pretty different level of abstraction than the rest of this library so far - i wonder if we can have this header set by the configured http client.
so e.g. we set the specific client/url at https://github.com/ipfs/bifrost-gateway/blob/main/blockstore.go#L111
we could do something like the final code block at https://developer20.com/add-header-to-every-request-in-go/ to configure the JWT token in the same place, which seems more local than splitting it between there and here.
There was a problem hiding this comment.
I have removed this token from the config.
caboose.go
Outdated
| if config.SaturnLoggerJWT == "" || len(config.SaturnLoggerJWT) == 0 { | ||
| return nil, errors.New("JWT token required for Saturn Logger") | ||
| } |
There was a problem hiding this comment.
iiurc last time we talked about this @guanzo noted it is ok to simply disable sending logs to logger when JWT token is not set.
|
Filed ipfs-inactive/bifrost-gateway#51 for Bifrost to make the required changes. |
|
@willscott @lidel Don't worry about the failing CI test here. Has already been fixed in #39. |
|
@aarshkshah1992 - the top of the PR indicates we're waiting to see a 200 response from log ingester. what needs to happen there / should we merge and tag this now? |
|
@willscott Merging this as there's some config work that needs to happen on Bifrost to test this end -end. Work captured at ipfs-inactive/bifrost-gateway#51. |
Fixes #38.