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.

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

Log less #2718

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Log less #2718

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dbdb02f
Log less
Sneagan Nov 20, 2024
2ebcbcd
Move redaction into function and add test
Sneagan Nov 22, 2024
e09aa43
Precompile regex
Sneagan Nov 25, 2024
7f8fe8f
Merge branch 'master' into fixup/log-less
Sneagan Dec 6, 2024
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
15 changes: 11 additions & 4 deletions libs/wallet/provider/uphold/uphold.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"net/http/httputil"
"net/url"
"os"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -236,25 +237,31 @@ func submit(
if err != nil {
return nil, resp, fmt.Errorf("%w: %s", errorutils.ErrFailedBodyRead, err.Error())
}
sbody := string(body)
for _, p := range []string{`"description":\s*"[^"]+"\s*,?`, `"\w+Country":\s*"[^"]+"\s*,?`} {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: could we extract this block into a function so we can test it

func redactData(body string) string {
	d := description.ReplaceAllString(body, "")
	c := country.ReplaceAllString(d, "")
	
	return strings.ReplaceAll(c, ",}", "}")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done.

Copy link
Contributor

@clD11 clD11 Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

question: do we have an example of the response we are trying to redact this from?

we could add an example response to a test, I wonder if we can use an or instead of having two separate passes or the regex

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 added a test with an example object.

re := regexp.MustCompile(p)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: instead of compiling the regex on every call of the method and for each value can we just compile them once and reuse

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 think the readability cost of moving the regex out of here is worse than the performance cost of recompiling it. This call flow spends most of its time waiting for i/o and regex compilation time will be negligible.

sbody = re.ReplaceAllString(sbody, "")
}
sbody = regexp.MustCompile(`,}`).ReplaceAllString(sbody, "}")
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: could we use strings.ReplaceAll(c, ",}", "}") here


if logger != nil {
logger.Debug().
Str("path", "github.com/brave-intl/bat-go/wallet/provider/uphold").
Str("type", "http.Response").
Int("status", resp.StatusCode).
Str("headers", string(jsonHeaders)).
Msg(string(body))
Msg(sbody)
}

if resp.StatusCode/100 != 2 {
var uhErr upholdError
if json.Unmarshal(body, &uhErr) != nil {
return nil, resp, fmt.Errorf("Error %d, %s", resp.StatusCode, body)
if json.Unmarshal([]byte(sbody), &uhErr) != nil {
return nil, resp, fmt.Errorf("Error %d, %s", resp.StatusCode, sbody)
}
uhErr.RequestID = resp.Header.Get("Request-Id")
return nil, resp, uhErr
}
return body, resp, nil
return []byte(sbody), resp, nil
}

type createCardRequest struct {
Expand Down
Loading