Skip to content

uhttp client with logger #32

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.

Sign up for GitHub

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

Closed
gontzess wants to merge 2 commits into from
Closed

uhttp client with logger #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2896079
use uhttp client with logger
gontzess Jul 26, 2025
6ea34f3
move logger to client field
gontzess Jul 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 38 additions & 45 deletions pkg/connector/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,20 @@ const (

type Client struct {
httpClient *uhttp.BaseHttpClient
logger *zap.Logger
baseURL *url.URL
apiKey string
resourcesPageSize int
}

// NewClient creates a new Rootly client. Allows for a configurable base URL, API key, and resources page size.
func NewClient(ctx context.Context, baseURL string, apiKey string, resourcesPageSize int) (*Client, error) {
httpClient, err := uhttp.NewBaseHttpClientWithContext(ctx, http.DefaultClient)
logger := ctxzap.Extract(ctx)
httpClient, err := uhttp.NewClient(ctx, uhttp.WithLogger(true, logger))
if err != nil {
return nil, fmt.Errorf("creating HTTP client failed: %w", err)
}
wrapper, err := uhttp.NewBaseHttpClientWithContext(ctx, httpClient)
if err != nil {
return nil, err
}
Expand All @@ -60,7 +66,8 @@ func NewClient(ctx context.Context, baseURL string, apiKey string, resourcesPage
// as it provides automatic rate limiting handling, error wrapping with gRPC status codes,
// and built-in GET response caching
return &Client{
httpClient: httpClient,
httpClient: wrapper,
logger: logger,
Copy link
Contributor

@ggreer ggreer Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly recommend using ctxzap.Extract(ctx) to get the logger. Otherwise it makes things like logger.With(...) cumbersome:

logger := c.logger.With("name", zap.String(val))

logger.Debug(...)

If anyone uses c.logger.Debug() instead of logger.Debug(), the fields specified in .With(...) won't be logged. Also if anyone modifies c.logger, it will change logging in other functions.

Copy link
Contributor Author

@gontzess gontzess Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this all makes sense! i was testing a few different things, and so verifying this less-than-ideal workaround allowed me to see logs from client.go was helpful information at the time, leading me to believe we weren't passing the context properly in lambdas. looks like @kans and @laurenleach identified and fixed the ctx issue in the sdk last week 🥳
ConductorOne/baton-sdk#413

baseURL: parsedURL,
apiKey: apiKey,
resourcesPageSize: resourcesPageSize,
Expand All @@ -76,8 +83,6 @@ func (c *Client) doRequest(
requestBody interface{},
target interface{},
) error {
l := ctxzap.Extract(ctx)

// create the request
reqOptions := []uhttp.RequestOption{
uhttp.WithBearerToken(c.apiKey),
Expand All @@ -92,7 +97,7 @@ func (c *Client) doRequest(
}

// do the request and handle the response
l.Debug("sending request", zap.String("url", url.String()), zap.String("method", method))
c.logger.Debug("sending request", zap.String("url", url.String()), zap.String("method", method))
// Add error response handling
var rootlyError RootlyErrorResponse
respOptions := []uhttp.DoOption{uhttp.WithErrorResponse(&rootlyError)}
Expand Down Expand Up @@ -133,20 +138,18 @@ func (c *Client) generateURL(
// generateCurrentPaginatedURL either parses the URL from the page token, or generates a new URL
// with initial pagination if there's no token.
func (c *Client) generateCurrentPaginatedURL(
ctx context.Context,
pToken string,
path string,
pathParameters ...string,
) (*url.URL, error) {
logger := ctxzap.Extract(ctx)
if pToken != "" {
// this is not the first request to this endpoint
// use the token string, ie a full URL with params already populated based on the prior request
parsedURL, err := url.Parse(pToken)
if err != nil {
return nil, err
}
logger.Debug("Parsed token for paginated URL", zap.String("parsedURL", parsedURL.String()))
c.logger.Debug("Parsed token for paginated URL", zap.String("parsedURL", parsedURL.String()))
return parsedURL, nil
}

Expand All @@ -159,7 +162,7 @@ func (c *Client) generateCurrentPaginatedURL(
},
pathParameters...,
)
logger.Debug("Generated first paginated URL", zap.String("parsedURL", parsedURL.String()))
c.logger.Debug("Generated first paginated URL", zap.String("parsedURL", parsedURL.String()))
return parsedURL, nil
}

Expand All @@ -169,8 +172,7 @@ func (c *Client) IsTest() bool {

// GetUsers fetches users from the Rootly API. It supports pagination using a page token.
func (c *Client) GetUsers(ctx context.Context, pToken string) ([]User, string, error) {
logger := ctxzap.Extract(ctx)
parsedURL, err := c.generateCurrentPaginatedURL(ctx, pToken, ListUsersAPIEndpoint)
parsedURL, err := c.generateCurrentPaginatedURL(pToken, ListUsersAPIEndpoint)
if err != nil {
return nil, "", fmt.Errorf("get-users: %w", err)
}
Expand All @@ -186,14 +188,13 @@ func (c *Client) GetUsers(ctx context.Context, pToken string) ([]User, string, e
if err != nil {
return nil, "", fmt.Errorf("get-users: %w", err)
}
logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
c.logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
return resp.Data, resp.Links.Next, nil
}

// GetTeams fetches the teams from the Rootly API. It supports pagination using a page token.
func (c *Client) GetTeams(ctx context.Context, pToken string) ([]Team, string, error) {
logger := ctxzap.Extract(ctx)
parsedURL, err := c.generateCurrentPaginatedURL(ctx, pToken, ListTeamsAPIEndpoint)
parsedURL, err := c.generateCurrentPaginatedURL(pToken, ListTeamsAPIEndpoint)
if err != nil {
return nil, "", fmt.Errorf("get-teams: %w", err)
}
Expand All @@ -209,7 +210,7 @@ func (c *Client) GetTeams(ctx context.Context, pToken string) ([]Team, string, e
if err != nil {
return nil, "", fmt.Errorf("get-teams: %w", err)
}
logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
c.logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
return resp.Data, resp.Links.Next, nil
}

Expand All @@ -218,13 +219,12 @@ func (c *Client) GetTeamMemberAndAdminIDs(
ctx context.Context,
teamID string,
) ([]int, []int, error) {
logger := ctxzap.Extract(ctx)
if teamID == "" {
logger.Error("get-team-member-and-admin-ids: teamID is required")
c.logger.Error("get-team-member-and-admin-ids: teamID is required")
return nil, nil, fmt.Errorf("get-team-member-and-admin-ids: teamID is required")
}
parsedURL := c.generateURL(GetTeamAPIEndpoint, nil, teamID)
logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))
c.logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))

var resp TeamResponse
err := c.doRequest(
Expand All @@ -243,8 +243,7 @@ func (c *Client) GetTeamMemberAndAdminIDs(

// GetSecrets fetches the secrets from the Rootly API. It supports pagination using a page token.
func (c *Client) GetSecrets(ctx context.Context, pToken string) ([]Secret, string, error) {
logger := ctxzap.Extract(ctx)
parsedURL, err := c.generateCurrentPaginatedURL(ctx, pToken, ListSecretsAPIEndpoint)
parsedURL, err := c.generateCurrentPaginatedURL(pToken, ListSecretsAPIEndpoint)
if err != nil {
return nil, "", fmt.Errorf("get-secrets: %w", err)
}
Expand All @@ -260,14 +259,13 @@ func (c *Client) GetSecrets(ctx context.Context, pToken string) ([]Secret, strin
if err != nil {
return nil, "", fmt.Errorf("get-secrets: %w", err)
}
logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
c.logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
return resp.Data, resp.Links.Next, nil
}

// GetSchedules fetches the schedules from the Rootly API. It supports pagination using a page token.
func (c *Client) GetSchedules(ctx context.Context, pToken string) ([]Schedule, string, error) {
logger := ctxzap.Extract(ctx)
parsedURL, err := c.generateCurrentPaginatedURL(ctx, pToken, ListSchedulesAPIEndpoint)
parsedURL, err := c.generateCurrentPaginatedURL(pToken, ListSchedulesAPIEndpoint)
if err != nil {
return nil, "", fmt.Errorf("get-schedules: %w", err)
}
Expand All @@ -283,7 +281,7 @@ func (c *Client) GetSchedules(ctx context.Context, pToken string) ([]Schedule, s
if err != nil {
return nil, "", fmt.Errorf("get-schedules: %w", err)
}
logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
c.logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
return resp.Data, resp.Links.Next, nil
}

Expand All @@ -292,13 +290,12 @@ func (c *Client) GetScheduleOwnerIDs(
ctx context.Context,
scheduleID string,
) (*int, []string, error) {
logger := ctxzap.Extract(ctx)
if scheduleID == "" {
logger.Error("get-schedule-owner-ids: scheduleID is required")
c.logger.Error("get-schedule-owner-ids: scheduleID is required")
return nil, nil, fmt.Errorf("get-schedule-owner-ids: scheduleID is required")
}
parsedURL := c.generateURL(GetScheduleAPIEndpoint, nil, scheduleID)
logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))
c.logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))

var resp ScheduleResponse
err := c.doRequest(
Expand All @@ -322,16 +319,15 @@ func (c *Client) ListScheduleRotations(
scheduleID string,
pToken string,
) ([]string, string, error) {
logger := ctxzap.Extract(ctx)
if scheduleID == "" {
logger.Error("list-schedule-rotations: scheduleID is required")
c.logger.Error("list-schedule-rotations: scheduleID is required")
return nil, "", fmt.Errorf("list-schedule-rotations: scheduleID is required")
}
parsedURL, err := c.generateCurrentPaginatedURL(ctx, pToken, ListScheduleRotationsAPIEndpoint, scheduleID)
parsedURL, err := c.generateCurrentPaginatedURL(pToken, ListScheduleRotationsAPIEndpoint, scheduleID)
if err != nil {
return nil, "", fmt.Errorf("list-schedule-rotations: %w", err)
}
logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))
c.logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))

var resp ScheduleRotationsResponse
err = c.doRequest(
Expand All @@ -348,12 +344,12 @@ func (c *Client) ListScheduleRotations(
var rotationIDs []string
for _, rotation := range resp.Data {
if rotation.Type != "schedule_rotations" {
logger.Debug("Unexpected type in schedule rotation", zap.String("rotation.Type", rotation.Type))
c.logger.Debug("Unexpected type in schedule rotation", zap.String("rotation.Type", rotation.Type))
continue
}
rotationIDs = append(rotationIDs, rotation.ID)
}
logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
c.logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
return rotationIDs, resp.Links.Next, nil
}

Expand All @@ -364,16 +360,15 @@ func (c *Client) ListScheduleRotationUsers(
rotationID string,
pToken string,
) ([]int, string, error) {
logger := ctxzap.Extract(ctx)
if rotationID == "" {
logger.Error("list-schedule-rotation-users: rotationID is required")
c.logger.Error("list-schedule-rotation-users: rotationID is required")
return nil, "", fmt.Errorf("list-schedule-rotation-users: rotationID is required")
}
parsedURL, err := c.generateCurrentPaginatedURL(ctx, pToken, ListScheduleRotationUsersAPIEndpoint, rotationID)
parsedURL, err := c.generateCurrentPaginatedURL(pToken, ListScheduleRotationUsersAPIEndpoint, rotationID)
if err != nil {
return nil, "", fmt.Errorf("list-schedule-rotation-users: %w", err)
}
logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))
c.logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))

var resp ScheduleRotationUsersResponse
err = c.doRequest(
Expand All @@ -391,7 +386,7 @@ func (c *Client) ListScheduleRotationUsers(
for _, user := range resp.Data {
userIDs = append(userIDs, user.Attributes.UserID)
}
logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
c.logger.Debug("Paginated URL for the next request", zap.String("resp.Links.Next", resp.Links.Next))
return userIDs, resp.Links.Next, nil
}

Expand All @@ -401,9 +396,8 @@ func (c *Client) ListAllScheduleRotationUsers(
ctx context.Context,
rotationID string,
) ([]int, error) {
logger := ctxzap.Extract(ctx)
if rotationID == "" {
logger.Error("list-all-schedule-rotation-users: rotationID is required")
c.logger.Error("list-all-schedule-rotation-users: rotationID is required")
return nil, fmt.Errorf("list-all-schedule-rotation-users: rotationID is required")
}
var userIDs []int
Expand All @@ -414,7 +408,7 @@ func (c *Client) ListAllScheduleRotationUsers(
return nil, fmt.Errorf("list-all-schedule-rotation-users: %w", err)
}

logger.Debug(
c.logger.Debug(
"Schedule rotations users",
zap.Int("number of memberIDs", len(memberIDs)),
zap.String("nextPage", nextPage),
Expand All @@ -436,9 +430,8 @@ func (c *Client) ListOnCallUsers(
ctx context.Context,
scheduleID string,
) ([]int, error) {
logger := ctxzap.Extract(ctx)
if scheduleID == "" {
logger.Error("list-on-call-users: scheduleID is required")
c.logger.Error("list-on-call-users: scheduleID is required")
return nil, fmt.Errorf("list-on-call-users: scheduleID is required")
}
now := time.Now().UTC()
Expand All @@ -448,7 +441,7 @@ func (c *Client) ListOnCallUsers(
"from": now.Format(time.RFC3339),
"to": now.Add(1 * time.Hour).Format(time.RFC3339),
}, scheduleID)
logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))
c.logger.Debug("Generated URL", zap.String("parsedURL", parsedURL.String()))

var resp ScheduleShiftsResponse
err := c.doRequest(
Expand All @@ -465,7 +458,7 @@ func (c *Client) ListOnCallUsers(
var userIDs []int
for _, user := range resp.Included {
if user.Type != "users" {
logger.Debug("Unexpected type in on-call included users", zap.String("user.Type", user.Type))
c.logger.Debug("Unexpected type in on-call included users", zap.String("user.Type", user.Type))
continue
}
userID, err := strconv.Atoi(user.ID)
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ func TestClient_generateCurrentPaginatedURL(t *testing.T) {
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got, err := client.generateCurrentPaginatedURL(ctx, tc.args.pToken, tc.args.path, tc.args.pathParameters...)
got, err := client.generateCurrentPaginatedURL(tc.args.pToken, tc.args.path, tc.args.pathParameters...)
// only get an error if the provided path in unparseable, not an interesting test case
require.Nil(t, err)
require.Equal(t, tc.want, got)
Expand Down
Loading