Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit bfbd09e

Browse files
ggilmoreggilmore
committed
fix/http: add logging middleware to default exported external http clients
1 parent b5d7a4f commit bfbd09e

File tree

139 files changed

+912
-653
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

139 files changed

+912
-653
lines changed

Diff for: ‎cmd/frontend/graphqlbackend/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,7 @@ go_test(
510510
"//internal/gitserver/gitdomain",
511511
"//internal/gitserver/protocol",
512512
"//internal/gqlutil",
513+
"//internal/httpcli",
513514
"//internal/observation",
514515
"//internal/oobmigration",
515516
"//internal/ratelimit",

Diff for: ‎cmd/frontend/graphqlbackend/cody_gateway_rate_limit.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ func (r *siteResolver) CodyGatewayRateLimitStatus(ctx context.Context) (*[]RateL
2424
return nil, err
2525
}
2626

27-
cgc, ok := codygateway.NewClientFromSiteConfig(httpcli.ExternalDoer)
27+
logger := r.logger.Scoped("CodyGatewayRateLimitStatus")
28+
cgc, ok := codygateway.NewClientFromSiteConfig(httpcli.ExternalDoer(logger))
2829
if !ok {
2930
// Not configured for chat/autocomplete.
3031
return nil, nil

Diff for: ‎cmd/frontend/graphqlbackend/external_accounts.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func (r *schemaResolver) AddExternalAccount(ctx context.Context, args *struct {
173173

174174
switch args.ServiceType {
175175
case extsvc.TypeGerrit:
176-
err := gext.AddGerritExternalAccount(ctx, r.db, a.UID, args.ServiceID, args.AccountDetails)
176+
err := gext.AddGerritExternalAccount(ctx, r.db, r.logger, a.UID, args.ServiceID, args.AccountDetails)
177177
if err != nil {
178178
return nil, err
179179
}

Diff for: ‎cmd/frontend/graphqlbackend/external_accounts_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func TestExternalAccounts_AddExternalAccount(t *testing.T) {
180180
}
181181
}
182182

183-
gerrit.MockVerifyAccount = func(_ context.Context, _ *url.URL, _ *gerrit.AccountCredentials) (*gerrit.Account, error) {
183+
gerrit.MockVerifyAccount = func(_ context.Context, _ log.Logger, _ *url.URL, _ *gerrit.AccountCredentials) (*gerrit.Account, error) {
184184
return &gerrit.Account{
185185
ID: 1234,
186186
Username: "alice",

Diff for: ‎cmd/frontend/graphqlbackend/external_service.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -311,12 +311,14 @@ func (r *externalServiceResolver) CheckConnection(ctx context.Context) (*externa
311311
ctx, cancel := context.WithTimeout(ctx, 15*time.Second)
312312
defer cancel()
313313

314+
logger := r.logger.Scoped("externalServiceResolver.CheckConnection")
315+
314316
source, err := repos.NewSource(
315317
ctx,
316-
log.Scoped("externalServiceResolver.CheckConnection"),
318+
logger,
317319
r.db,
318320
r.externalService,
319-
httpcli.ExternalClientFactory,
321+
httpcli.ExternalClientFactory(logger.Scoped("httpclient")),
320322
gitserver.NewClient("graphql.check-connection"),
321323
)
322324
if err != nil {

Diff for: ‎cmd/frontend/graphqlbackend/guardrails_test.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"testing"
1010

1111
"github.com/graph-gophers/graphql-go"
12+
"github.com/sourcegraph/log"
1213
"github.com/stretchr/testify/require"
1314

1415
"github.com/sourcegraph/sourcegraph/cmd/frontend/enterprise"
@@ -20,6 +21,7 @@ import (
2021
"github.com/sourcegraph/sourcegraph/internal/database"
2122
"github.com/sourcegraph/sourcegraph/internal/database/dbmocks"
2223
"github.com/sourcegraph/sourcegraph/internal/gitserver"
24+
"github.com/sourcegraph/sourcegraph/internal/httpcli"
2325
"github.com/sourcegraph/sourcegraph/internal/observation"
2426
"github.com/sourcegraph/sourcegraph/lib/errors"
2527
"github.com/sourcegraph/sourcegraph/lib/pointers"
@@ -194,7 +196,9 @@ func makeGatewayEndpoint(t *testing.T) string {
194196

195197
func TestSnippetAttributionReactsToSiteConfigChanges(t *testing.T) {
196198
// Use a regular HTTP client as default external doer cannot hit localhost.
197-
guardrails.MockHttpClient = http.DefaultClient
199+
guardrails.MockHttpClient = func(_ log.Logger) httpcli.Doer {
200+
return http.DefaultClient
201+
}
198202
t.Cleanup(func() { guardrails.MockHttpClient = nil })
199203
// Starting attribution configuration has no endpoints to use.
200204
noAttributionConfigured := schema.SiteConfiguration{

Diff for: ‎cmd/frontend/graphqlbackend/survey_response.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (r *schemaResolver) SubmitSurvey(ctx context.Context, args *struct {
117117
}
118118

119119
// Submit form to HubSpot
120-
if err := hubspotutil.Client().SubmitForm(hubspotutil.SurveyFormID, &surveySubmissionForHubSpot{
120+
if err := hubspotutil.Client(r.logger).SubmitForm(hubspotutil.SurveyFormID, &surveySubmissionForHubSpot{
121121
Email: email,
122122
Score: args.Input.Score,
123123
OtherUseCase: args.Input.OtherUseCase,
@@ -181,7 +181,7 @@ func (r *schemaResolver) SubmitHappinessFeedback(ctx context.Context, args *stru
181181
}
182182

183183
// Submit form to HubSpot
184-
if err := hubspotutil.Client().SubmitForm(hubspotutil.HappinessFeedbackFormID, &data); err != nil {
184+
if err := hubspotutil.Client(r.logger).SubmitForm(hubspotutil.HappinessFeedbackFormID, &data); err != nil {
185185
// Log an error, but don't return one if the only failure was in submitting feedback results to HubSpot.
186186
log15.Error("Unable to submit happiness feedback results to Sourcegraph remote", "error", err)
187187
}

Diff for: ‎cmd/frontend/graphqlbackend/user_usage_stats.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,11 @@ func (r *schemaResolver) LogEvents(ctx context.Context, args *EventBatch) (*Empt
169169
}
170170
}
171171

172-
hubspotutil.SyncUser(userPrimaryEmail, hubspotutil.CodyClientInstalledEventID, &hubspot.ContactProperties{
172+
hubspotutil.SyncUser(r.logger, userPrimaryEmail, hubspotutil.CodyClientInstalledEventID, &hubspot.ContactProperties{
173173
DatabaseID: userID,
174174
})
175175

176-
hubspotutil.SyncUserWithV3Event(userPrimaryEmail, hubspotutil.CodyClientInstalledV3EventID,
176+
hubspotutil.SyncUserWithV3Event(r.logger, userPrimaryEmail, hubspotutil.CodyClientInstalledV3EventID,
177177
&hubspot.ContactProperties{
178178
DatabaseID: userID,
179179
},

Diff for: ‎cmd/frontend/hubspot/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ go_library(
1414
"//internal/httpcli",
1515
"//lib/errors",
1616
"@com_github_google_go_querystring//query",
17+
"@com_github_sourcegraph_log//:log",
1718
"@org_uber_go_atomic//:atomic",
1819
],
1920
)

Diff for: ‎cmd/frontend/hubspot/hubspot.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"time"
1313

1414
"github.com/google/go-querystring/query"
15+
"github.com/sourcegraph/log"
1516
"go.uber.org/atomic"
1617

1718
"github.com/sourcegraph/sourcegraph/internal/httpcli"
@@ -25,13 +26,15 @@ type Client struct {
2526

2627
lastPing atomic.Time
2728
lastPingResult atomic.Error
29+
logger log.Logger
2830
}
2931

3032
// New returns a new HubSpot client using the given Portal ID.
31-
func New(portalID, accessToken string) *Client {
33+
func New(portalID, accessToken string, logger log.Logger) *Client {
3234
return &Client{
3335
portalID: portalID,
3436
accessToken: accessToken,
37+
logger: logger.Scoped("HubSpotClient"),
3538
}
3639
}
3740

@@ -61,7 +64,7 @@ func (c *Client) postForm(methodName string, baseURL *url.URL, suffix string, bo
6164
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
6265
setAccessTokenAuthorizationHeader(req, c.accessToken)
6366

64-
resp, err := httpcli.ExternalDoer.Do(req)
67+
resp, err := httpcli.ExternalDoer(c.logger).Do(req)
6568
if err != nil {
6669
return wrapError(methodName, err)
6770
}
@@ -95,7 +98,7 @@ func (c *Client) postJSON(methodName string, baseURL *url.URL, reqPayload, respP
9598
req.Header.Set("Content-Type", "application/json")
9699
setAccessTokenAuthorizationHeader(req, c.accessToken)
97100

98-
resp, err := httpcli.ExternalDoer.Do(req.WithContext(ctx))
101+
resp, err := httpcli.ExternalDoer(c.logger).Do(req.WithContext(ctx))
99102
if err != nil {
100103
return wrapError(methodName, err)
101104
}
@@ -129,7 +132,7 @@ func (c *Client) get(ctx context.Context, methodName string, baseURL *url.URL, s
129132
ctx, cancel := context.WithTimeout(req.Context(), time.Minute)
130133
defer cancel()
131134

132-
resp, err := httpcli.ExternalDoer.Do(req.WithContext(ctx))
135+
resp, err := httpcli.ExternalDoer(c.logger).Do(req.WithContext(ctx))
133136
if err != nil {
134137
return wrapError(methodName, err)
135138
}

Diff for: ‎cmd/frontend/hubspot/hubspotutil/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,6 @@ go_library(
1111
"//internal/env",
1212
"//lib/errors",
1313
"@com_github_inconshreveable_log15//:log15",
14+
"@com_github_sourcegraph_log//:log",
1415
],
1516
)

Diff for: ‎cmd/frontend/hubspot/hubspotutil/hubspotutil.go

+11-14
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"log" //nolint:logging // TODO move all logging to sourcegraph/log
66

77
"github.com/inconshreveable/log15" //nolint:logging // TODO move all logging to sourcegraph/log
8+
logger "github.com/sourcegraph/log"
89

910
"github.com/sourcegraph/sourcegraph/cmd/frontend/hubspot"
1011
"github.com/sourcegraph/sourcegraph/internal/dotcom"
@@ -52,21 +53,17 @@ func HasAPIKey() bool {
5253
return HubSpotAccessToken != ""
5354
}
5455

55-
func init() {
56+
// Client returns a hubspot client
57+
func Client(l logger.Logger) *hubspot.Client {
5658
// The HubSpot access token will only be available in the production sourcegraph.com environment.
5759
// Not having this access token only restricts certain requests (e.g. GET requests to the Contacts API),
5860
// while others (e.g. POST requests to the Forms API) will still go through.
59-
client = hubspot.New("2762526", HubSpotAccessToken)
60-
}
61-
62-
// Client returns a hubspot client
63-
func Client() *hubspot.Client {
64-
return client
61+
return hubspot.New("2762526", HubSpotAccessToken, l.Scoped("HubSpotClient"))
6562
}
6663

6764
// SyncUser handles creating or syncing a user profile in HubSpot, and if provided,
6865
// logs a user event.
69-
func SyncUser(email, eventID string, contactParams *hubspot.ContactProperties) {
66+
func SyncUser(l logger.Logger, email, eventID string, contactParams *hubspot.ContactProperties) {
7067
defer func() {
7168
if err := recover(); err != nil {
7269
log.Printf("panic in tracking.SyncUser: %s", err)
@@ -79,15 +76,15 @@ func SyncUser(email, eventID string, contactParams *hubspot.ContactProperties) {
7976

8077
// Update or create user contact information in HubSpot, and we want to sync the
8178
// contact independent of the request lifecycle.
82-
err := syncHubSpotContact(context.Background(), email, eventID, contactParams)
79+
err := syncHubSpotContact(context.Background(), l, email, eventID, contactParams)
8380
if err != nil {
8481
log15.Warn("syncHubSpotContact: failed to create or update HubSpot contact", "source", "HubSpot", "eventID", eventID, "error", err)
8582
}
8683
}
8784

8885
// SyncUserWithV3Event handles creating or syncing a user profile in HubSpot, and if provided,
8986
// logs a V3 custom event along with the event params.
90-
func SyncUserWithV3Event(email, eventName string, contactParams *hubspot.ContactProperties, eventProperties any) {
87+
func SyncUserWithV3Event(l logger.Logger, email, eventName string, contactParams *hubspot.ContactProperties, eventProperties any) {
9188
defer func() {
9289
if err := recover(); err != nil {
9390
log.Printf("panic in tracking.SyncUserWithV3Event: %s", err)
@@ -101,14 +98,14 @@ func SyncUserWithV3Event(email, eventName string, contactParams *hubspot.Contact
10198

10299
// Update or create user contact information in HubSpot, and we want to sync the
103100
// contact independent of the request lifecycle.
104-
err := syncHubSpotContact(context.Background(), email, "", contactParams)
101+
err := syncHubSpotContact(context.Background(), l, email, "", contactParams)
105102
if err != nil {
106103
log15.Warn("syncHubSpotContact: failed to create or update HubSpot contact", "source", "HubSpot", "eventName", eventName, "error", err)
107104
}
108105

109106
// Log the V3 event
110107
if eventName != "" {
111-
c := Client()
108+
c := Client(l)
112109
err = c.LogV3Event(email, eventName, eventProperties)
113110
if err != nil {
114111
log.Printf("LOGV3Event: failed to event %s", err)
@@ -117,7 +114,7 @@ func SyncUserWithV3Event(email, eventName string, contactParams *hubspot.Contact
117114
}
118115
}
119116

120-
func syncHubSpotContact(ctx context.Context, email, eventID string, contactParams *hubspot.ContactProperties) error {
117+
func syncHubSpotContact(ctx context.Context, l logger.Logger, email, eventID string, contactParams *hubspot.ContactProperties) error {
121118
if email == "" {
122119
return errors.New("user must have a valid email address")
123120
}
@@ -128,7 +125,7 @@ func syncHubSpotContact(ctx context.Context, email, eventID string, contactParam
128125
}
129126
contactParams.UserID = email
130127

131-
c := Client()
128+
c := Client(l)
132129

133130
// Create or update the contact
134131
_, err := c.CreateOrUpdateContact(email, contactParams)

Diff for: ‎cmd/frontend/internal/app/ui/handlers.go

+65-61
Original file line numberDiff line numberDiff line change
@@ -563,68 +563,72 @@ func searchBadgeHandler() *httputil.ReverseProxy {
563563
}
564564
}
565565

566-
func servePingFromSelfHosted(w http.ResponseWriter, r *http.Request) error {
567-
// CORS to allow request from anywhere
568-
u, err := url.Parse(r.Referer())
569-
if err != nil {
570-
return err
571-
}
572-
w.Header().Add("Access-Control-Allow-Origin", u.Host)
573-
w.Header().Add("Access-Control-Allow-Credentials", "true")
574-
if r.Method == http.MethodOptions {
575-
// CORS preflight request, respond 204 and allow origin header
576-
w.WriteHeader(http.StatusNoContent)
577-
return nil
578-
}
579-
email := r.URL.Query().Get("email")
580-
tosAccepted := r.URL.Query().Get("tos_accepted")
566+
func servePingFromSelfHosted(l log.Logger) func(w http.ResponseWriter, r *http.Request) error {
567+
l = l.Scoped("servePingFromSelfHosted")
581568

582-
getCookie := func(name string) string {
583-
c, err := r.Cookie(name)
584-
if err != nil || c == nil {
585-
return ""
569+
return func(w http.ResponseWriter, r *http.Request) error {
570+
// CORS to allow request from anywhere
571+
u, err := url.Parse(r.Referer())
572+
if err != nil {
573+
return err
586574
}
587-
return c.Value
588-
}
575+
w.Header().Add("Access-Control-Allow-Origin", u.Host)
576+
w.Header().Add("Access-Control-Allow-Credentials", "true")
577+
if r.Method == http.MethodOptions {
578+
// CORS preflight request, respond 204 and allow origin header
579+
w.WriteHeader(http.StatusNoContent)
580+
return nil
581+
}
582+
email := r.URL.Query().Get("email")
583+
tosAccepted := r.URL.Query().Get("tos_accepted")
589584

590-
anonymousUserId, _ := cookie.AnonymousUID(r)
591-
592-
hubspotutil.SyncUser(email, hubspotutil.SelfHostedSiteInitEventID, &hubspot.ContactProperties{
593-
IsServerAdmin: true,
594-
AnonymousUserID: anonymousUserId,
595-
FirstSourceURL: getCookie("first_page_seen_url"),
596-
LastSourceURL: getCookie("last_page_seen_url"),
597-
LastPageSeenShort: getCookie("last_page_seen_url_short"),
598-
LastPageSeenMid: getCookie("last_page_seen_url_mid"),
599-
LastPageSeenLong: getCookie("last_page_seen_url_long"),
600-
MostRecentReferrerUrl: getCookie("most_recent_referrer_url"),
601-
MostRecentReferrerUrlShort: getCookie("most_recent_referrer_url_short"),
602-
MostRecentReferrerUrlLong: getCookie("most_recent_referrer_url_long"),
603-
SignupSessionSourceURL: getCookie("sourcegraphSignupSourceUrl"),
604-
SignupSessionReferrer: getCookie("sourcegraphSignupReferrer"),
605-
SessionUTMCampaign: getCookie("utm_campaign"),
606-
SessionUTMSource: getCookie("utm_source"),
607-
SessionUTMMedium: getCookie("utm_medium"),
608-
SessionUTMContent: getCookie("utm_content"),
609-
SessionUTMTerm: getCookie("utm_term"),
610-
UtmCampaignShort: getCookie("utm_campaign_short"),
611-
UtmCampaignMid: getCookie("utm_campaign_mid"),
612-
UtmCampaignLong: getCookie("utm_campaign_long"),
613-
UtmSourceShort: getCookie("utm_source_short"),
614-
UtmSourceMid: getCookie("utm_source_mid"),
615-
UtmSourceLong: getCookie("utm_source_long"),
616-
UtmMediumShort: getCookie("utm_medium_short"),
617-
UtmMediumMid: getCookie("utm_medium_mid"),
618-
UtmMediumLong: getCookie("utm_medium_long"),
619-
UtmContentShort: getCookie("utm_content_short"),
620-
UtmContentMid: getCookie("utm_content_mid"),
621-
UtmContentLong: getCookie("utm_content_long"),
622-
UtmTermShort: getCookie("utm_term_short"),
623-
UtmTermMid: getCookie("utm_term_mid"),
624-
UtmTermLong: getCookie("utm_term_long"),
625-
GoogleClickID: getCookie("gclid"),
626-
MicrosoftClickID: getCookie("msclkid"),
627-
HasAgreedToToS: tosAccepted == "true",
628-
})
629-
return nil
585+
getCookie := func(name string) string {
586+
c, err := r.Cookie(name)
587+
if err != nil || c == nil {
588+
return ""
589+
}
590+
return c.Value
591+
}
592+
593+
anonymousUserId, _ := cookie.AnonymousUID(r)
594+
595+
hubspotutil.SyncUser(l, email, hubspotutil.SelfHostedSiteInitEventID, &hubspot.ContactProperties{
596+
IsServerAdmin: true,
597+
AnonymousUserID: anonymousUserId,
598+
FirstSourceURL: getCookie("first_page_seen_url"),
599+
LastSourceURL: getCookie("last_page_seen_url"),
600+
LastPageSeenShort: getCookie("last_page_seen_url_short"),
601+
LastPageSeenMid: getCookie("last_page_seen_url_mid"),
602+
LastPageSeenLong: getCookie("last_page_seen_url_long"),
603+
MostRecentReferrerUrl: getCookie("most_recent_referrer_url"),
604+
MostRecentReferrerUrlShort: getCookie("most_recent_referrer_url_short"),
605+
MostRecentReferrerUrlLong: getCookie("most_recent_referrer_url_long"),
606+
SignupSessionSourceURL: getCookie("sourcegraphSignupSourceUrl"),
607+
SignupSessionReferrer: getCookie("sourcegraphSignupReferrer"),
608+
SessionUTMCampaign: getCookie("utm_campaign"),
609+
SessionUTMSource: getCookie("utm_source"),
610+
SessionUTMMedium: getCookie("utm_medium"),
611+
SessionUTMContent: getCookie("utm_content"),
612+
SessionUTMTerm: getCookie("utm_term"),
613+
UtmCampaignShort: getCookie("utm_campaign_short"),
614+
UtmCampaignMid: getCookie("utm_campaign_mid"),
615+
UtmCampaignLong: getCookie("utm_campaign_long"),
616+
UtmSourceShort: getCookie("utm_source_short"),
617+
UtmSourceMid: getCookie("utm_source_mid"),
618+
UtmSourceLong: getCookie("utm_source_long"),
619+
UtmMediumShort: getCookie("utm_medium_short"),
620+
UtmMediumMid: getCookie("utm_medium_mid"),
621+
UtmMediumLong: getCookie("utm_medium_long"),
622+
UtmContentShort: getCookie("utm_content_short"),
623+
UtmContentMid: getCookie("utm_content_mid"),
624+
UtmContentLong: getCookie("utm_content_long"),
625+
UtmTermShort: getCookie("utm_term_short"),
626+
UtmTermMid: getCookie("utm_term_mid"),
627+
UtmTermLong: getCookie("utm_term_long"),
628+
GoogleClickID: getCookie("gclid"),
629+
MicrosoftClickID: getCookie("msclkid"),
630+
HasAgreedToToS: tosAccepted == "true",
631+
})
632+
return nil
633+
}
630634
}

0 commit comments

Comments
 (0)