Skip to content
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

🛠️ new logger/zerologadapter for retryable requests #4903

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

afiune
Copy link
Contributor

@afiune afiune commented Nov 21, 2024

We use the retryablehttp package to retry http requests and we always want the logs to go to debug level so, we need an adapter.

We have implemented this adapter many times, this change removes all the duplicated code and creates a util package that all providers can use.

Example:

retryClient := retryablehttp.NewClient()
retryClient.Logger = zerologadapter.New(log.Logger)

We use the `retryablehttp` package to retry http requests and we always
want the logs to go to `debug` level so, we need an adapter.

We have implemented this adapter many times, this change removes all the
duplicated code and creates a util package that all providers can use.

Example:

```go
retryClient := retryablehttp.NewClient()
retryClient.Logger = zerologadapter.New(log.Logger)
```

Signed-off-by: Salim Afiune Maya <afiune@mondoo.com>
Copy link
Contributor

github-actions bot commented Nov 21, 2024

Test Results

3 162 tests  +15   3 161 ✅ +15   1m 26s ⏱️ -6s
  372 suites + 1       1 💤 ± 0 
   28 files   ± 0       0 ❌ ± 0 

Results for commit d0dc2e8. ± Comparison against base commit a90bf3a.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@preslavgerchev preslavgerchev left a comment

Choose a reason for hiding this comment

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

nice simplification @afiune

@czunker
Copy link
Contributor

czunker commented Nov 21, 2024

Just rebase on main.
#4905 should have fixed the failing tests.

// Copyright (c) Mondoo, Inc.
// SPDX-License-Identifier: BUSL-1.1

package zerologadapter
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put it inside the providers-sdk/v1 to make it clear that providers can and should use it, Ill move it though.

@afiune afiune changed the title 🛠️ new util/zerologadapter for retryable requests 🛠️ new logger/zerologadapter for retryable requests Nov 21, 2024
@afiune afiune merged commit 55dbd36 into main Nov 21, 2024
16 checks passed
@afiune afiune deleted the afiune/zerolog-adapter branch November 21, 2024 17:08
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants