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

fix/http: add logging middleware to default exported external http clients #64402

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions cmd/frontend/graphqlbackend/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ go_test(
"//internal/gitserver/gitdomain",
"//internal/gitserver/protocol",
"//internal/gqlutil",
"//internal/httpcli",
"//internal/observation",
"//internal/oobmigration",
"//internal/ratelimit",
Expand Down
3 changes: 2 additions & 1 deletion cmd/frontend/graphqlbackend/cody_gateway_rate_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ func (r *siteResolver) CodyGatewayRateLimitStatus(ctx context.Context) (*[]RateL
return nil, err
}

cgc, ok := codygateway.NewClientFromSiteConfig(httpcli.ExternalDoer)
logger := r.logger.Scoped("CodyGatewayRateLimitStatus")
cgc, ok := codygateway.NewClientFromSiteConfig(httpcli.ExternalDoer(logger))
if !ok {
// Not configured for chat/autocomplete.
return nil, nil
Expand Down
2 changes: 1 addition & 1 deletion cmd/frontend/graphqlbackend/external_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (r *schemaResolver) AddExternalAccount(ctx context.Context, args *struct {

switch args.ServiceType {
case extsvc.TypeGerrit:
err := gext.AddGerritExternalAccount(ctx, r.db, a.UID, args.ServiceID, args.AccountDetails)
err := gext.AddGerritExternalAccount(ctx, r.db, r.logger, a.UID, args.ServiceID, args.AccountDetails)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/frontend/graphqlbackend/external_accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func TestExternalAccounts_AddExternalAccount(t *testing.T) {
}
}

gerrit.MockVerifyAccount = func(_ context.Context, _ *url.URL, _ *gerrit.AccountCredentials) (*gerrit.Account, error) {
gerrit.MockVerifyAccount = func(_ context.Context, _ log.Logger, _ *url.URL, _ *gerrit.AccountCredentials) (*gerrit.Account, error) {
return &gerrit.Account{
ID: 1234,
Username: "alice",
Expand Down
6 changes: 4 additions & 2 deletions cmd/frontend/graphqlbackend/external_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,14 @@ func (r *externalServiceResolver) CheckConnection(ctx context.Context) (*externa
ctx, cancel := context.WithTimeout(ctx, 15*time.Second)
defer cancel()

logger := r.logger.Scoped("externalServiceResolver.CheckConnection")

source, err := repos.NewSource(
ctx,
log.Scoped("externalServiceResolver.CheckConnection"),
logger,
r.db,
r.externalService,
httpcli.ExternalClientFactory,
httpcli.ExternalClientFactory(logger.Scoped("httpclient")),
gitserver.NewClient("graphql.check-connection"),
)
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion cmd/frontend/graphqlbackend/guardrails_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"github.com/graph-gophers/graphql-go"
"github.com/sourcegraph/log"
"github.com/stretchr/testify/require"

"github.com/sourcegraph/sourcegraph/cmd/frontend/enterprise"
Expand All @@ -20,6 +21,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/database/dbmocks"
"github.com/sourcegraph/sourcegraph/internal/gitserver"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
"github.com/sourcegraph/sourcegraph/internal/observation"
"github.com/sourcegraph/sourcegraph/lib/errors"
"github.com/sourcegraph/sourcegraph/lib/pointers"
Expand Down Expand Up @@ -194,7 +196,9 @@ func makeGatewayEndpoint(t *testing.T) string {

func TestSnippetAttributionReactsToSiteConfigChanges(t *testing.T) {
// Use a regular HTTP client as default external doer cannot hit localhost.
guardrails.MockHttpClient = http.DefaultClient
guardrails.MockHttpClient = func(_ log.Logger) httpcli.Doer {
return http.DefaultClient
}
t.Cleanup(func() { guardrails.MockHttpClient = nil })
// Starting attribution configuration has no endpoints to use.
noAttributionConfigured := schema.SiteConfiguration{
Expand Down
4 changes: 2 additions & 2 deletions cmd/frontend/graphqlbackend/survey_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (r *schemaResolver) SubmitSurvey(ctx context.Context, args *struct {
}

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

// Submit form to HubSpot
if err := hubspotutil.Client().SubmitForm(hubspotutil.HappinessFeedbackFormID, &data); err != nil {
if err := hubspotutil.Client(r.logger).SubmitForm(hubspotutil.HappinessFeedbackFormID, &data); err != nil {
// Log an error, but don't return one if the only failure was in submitting feedback results to HubSpot.
log15.Error("Unable to submit happiness feedback results to Sourcegraph remote", "error", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/frontend/graphqlbackend/user_usage_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,11 @@ func (r *schemaResolver) LogEvents(ctx context.Context, args *EventBatch) (*Empt
}
}

hubspotutil.SyncUser(userPrimaryEmail, hubspotutil.CodyClientInstalledEventID, &hubspot.ContactProperties{
hubspotutil.SyncUser(r.logger, userPrimaryEmail, hubspotutil.CodyClientInstalledEventID, &hubspot.ContactProperties{
DatabaseID: userID,
})

hubspotutil.SyncUserWithV3Event(userPrimaryEmail, hubspotutil.CodyClientInstalledV3EventID,
hubspotutil.SyncUserWithV3Event(r.logger, userPrimaryEmail, hubspotutil.CodyClientInstalledV3EventID,
&hubspot.ContactProperties{
DatabaseID: userID,
},
Expand Down
1 change: 1 addition & 0 deletions cmd/frontend/hubspot/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ go_library(
"//internal/httpcli",
"//lib/errors",
"@com_github_google_go_querystring//query",
"@com_github_sourcegraph_log//:log",
"@org_uber_go_atomic//:atomic",
],
)
11 changes: 7 additions & 4 deletions cmd/frontend/hubspot/hubspot.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"time"

"github.com/google/go-querystring/query"
"github.com/sourcegraph/log"
"go.uber.org/atomic"

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

lastPing atomic.Time
lastPingResult atomic.Error
logger log.Logger
}

// New returns a new HubSpot client using the given Portal ID.
func New(portalID, accessToken string) *Client {
func New(portalID, accessToken string, logger log.Logger) *Client {
return &Client{
portalID: portalID,
accessToken: accessToken,
logger: logger.Scoped("HubSpotClient"),
}
}

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

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

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

resp, err := httpcli.ExternalDoer.Do(req.WithContext(ctx))
resp, err := httpcli.ExternalDoer(c.logger).Do(req.WithContext(ctx))
if err != nil {
return wrapError(methodName, err)
}
Expand Down
1 change: 1 addition & 0 deletions cmd/frontend/hubspot/hubspotutil/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ go_library(
"//internal/env",
"//lib/errors",
"@com_github_inconshreveable_log15//:log15",
"@com_github_sourcegraph_log//:log",
],
)
25 changes: 11 additions & 14 deletions cmd/frontend/hubspot/hubspotutil/hubspotutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"log" //nolint:logging // TODO move all logging to sourcegraph/log

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

"github.com/sourcegraph/sourcegraph/cmd/frontend/hubspot"
"github.com/sourcegraph/sourcegraph/internal/dotcom"
Expand Down Expand Up @@ -52,21 +53,17 @@ func HasAPIKey() bool {
return HubSpotAccessToken != ""
}

func init() {
// Client returns a hubspot client
func Client(l logger.Logger) *hubspot.Client {
// The HubSpot access token will only be available in the production sourcegraph.com environment.
// Not having this access token only restricts certain requests (e.g. GET requests to the Contacts API),
// while others (e.g. POST requests to the Forms API) will still go through.
client = hubspot.New("2762526", HubSpotAccessToken)
}

// Client returns a hubspot client
func Client() *hubspot.Client {
return client
return hubspot.New("2762526", HubSpotAccessToken, l.Scoped("HubSpotClient"))
}

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

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

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

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

// Log the V3 event
if eventName != "" {
c := Client()
c := Client(l)
err = c.LogV3Event(email, eventName, eventProperties)
if err != nil {
log.Printf("LOGV3Event: failed to event %s", err)
Expand All @@ -117,7 +114,7 @@ func SyncUserWithV3Event(email, eventName string, contactParams *hubspot.Contact
}
}

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

c := Client()
c := Client(l)

// Create or update the contact
_, err := c.CreateOrUpdateContact(email, contactParams)
Expand Down
126 changes: 65 additions & 61 deletions cmd/frontend/internal/app/ui/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,68 +563,72 @@ func searchBadgeHandler() *httputil.ReverseProxy {
}
}

func servePingFromSelfHosted(w http.ResponseWriter, r *http.Request) error {
// CORS to allow request from anywhere
u, err := url.Parse(r.Referer())
if err != nil {
return err
}
w.Header().Add("Access-Control-Allow-Origin", u.Host)
w.Header().Add("Access-Control-Allow-Credentials", "true")
if r.Method == http.MethodOptions {
// CORS preflight request, respond 204 and allow origin header
w.WriteHeader(http.StatusNoContent)
return nil
}
email := r.URL.Query().Get("email")
tosAccepted := r.URL.Query().Get("tos_accepted")
func servePingFromSelfHosted(l log.Logger) func(w http.ResponseWriter, r *http.Request) error {
l = l.Scoped("servePingFromSelfHosted")

getCookie := func(name string) string {
c, err := r.Cookie(name)
if err != nil || c == nil {
return ""
return func(w http.ResponseWriter, r *http.Request) error {
// CORS to allow request from anywhere
u, err := url.Parse(r.Referer())
if err != nil {
return err
}
return c.Value
}
w.Header().Add("Access-Control-Allow-Origin", u.Host)
w.Header().Add("Access-Control-Allow-Credentials", "true")
if r.Method == http.MethodOptions {
// CORS preflight request, respond 204 and allow origin header
w.WriteHeader(http.StatusNoContent)
return nil
}
email := r.URL.Query().Get("email")
tosAccepted := r.URL.Query().Get("tos_accepted")

anonymousUserId, _ := cookie.AnonymousUID(r)

hubspotutil.SyncUser(email, hubspotutil.SelfHostedSiteInitEventID, &hubspot.ContactProperties{
IsServerAdmin: true,
AnonymousUserID: anonymousUserId,
FirstSourceURL: getCookie("first_page_seen_url"),
LastSourceURL: getCookie("last_page_seen_url"),
LastPageSeenShort: getCookie("last_page_seen_url_short"),
LastPageSeenMid: getCookie("last_page_seen_url_mid"),
LastPageSeenLong: getCookie("last_page_seen_url_long"),
MostRecentReferrerUrl: getCookie("most_recent_referrer_url"),
MostRecentReferrerUrlShort: getCookie("most_recent_referrer_url_short"),
MostRecentReferrerUrlLong: getCookie("most_recent_referrer_url_long"),
SignupSessionSourceURL: getCookie("sourcegraphSignupSourceUrl"),
SignupSessionReferrer: getCookie("sourcegraphSignupReferrer"),
SessionUTMCampaign: getCookie("utm_campaign"),
SessionUTMSource: getCookie("utm_source"),
SessionUTMMedium: getCookie("utm_medium"),
SessionUTMContent: getCookie("utm_content"),
SessionUTMTerm: getCookie("utm_term"),
UtmCampaignShort: getCookie("utm_campaign_short"),
UtmCampaignMid: getCookie("utm_campaign_mid"),
UtmCampaignLong: getCookie("utm_campaign_long"),
UtmSourceShort: getCookie("utm_source_short"),
UtmSourceMid: getCookie("utm_source_mid"),
UtmSourceLong: getCookie("utm_source_long"),
UtmMediumShort: getCookie("utm_medium_short"),
UtmMediumMid: getCookie("utm_medium_mid"),
UtmMediumLong: getCookie("utm_medium_long"),
UtmContentShort: getCookie("utm_content_short"),
UtmContentMid: getCookie("utm_content_mid"),
UtmContentLong: getCookie("utm_content_long"),
UtmTermShort: getCookie("utm_term_short"),
UtmTermMid: getCookie("utm_term_mid"),
UtmTermLong: getCookie("utm_term_long"),
GoogleClickID: getCookie("gclid"),
MicrosoftClickID: getCookie("msclkid"),
HasAgreedToToS: tosAccepted == "true",
})
return nil
getCookie := func(name string) string {
c, err := r.Cookie(name)
if err != nil || c == nil {
return ""
}
return c.Value
}

anonymousUserId, _ := cookie.AnonymousUID(r)

hubspotutil.SyncUser(l, email, hubspotutil.SelfHostedSiteInitEventID, &hubspot.ContactProperties{
IsServerAdmin: true,
AnonymousUserID: anonymousUserId,
FirstSourceURL: getCookie("first_page_seen_url"),
LastSourceURL: getCookie("last_page_seen_url"),
LastPageSeenShort: getCookie("last_page_seen_url_short"),
LastPageSeenMid: getCookie("last_page_seen_url_mid"),
LastPageSeenLong: getCookie("last_page_seen_url_long"),
MostRecentReferrerUrl: getCookie("most_recent_referrer_url"),
MostRecentReferrerUrlShort: getCookie("most_recent_referrer_url_short"),
MostRecentReferrerUrlLong: getCookie("most_recent_referrer_url_long"),
SignupSessionSourceURL: getCookie("sourcegraphSignupSourceUrl"),
SignupSessionReferrer: getCookie("sourcegraphSignupReferrer"),
SessionUTMCampaign: getCookie("utm_campaign"),
SessionUTMSource: getCookie("utm_source"),
SessionUTMMedium: getCookie("utm_medium"),
SessionUTMContent: getCookie("utm_content"),
SessionUTMTerm: getCookie("utm_term"),
UtmCampaignShort: getCookie("utm_campaign_short"),
UtmCampaignMid: getCookie("utm_campaign_mid"),
UtmCampaignLong: getCookie("utm_campaign_long"),
UtmSourceShort: getCookie("utm_source_short"),
UtmSourceMid: getCookie("utm_source_mid"),
UtmSourceLong: getCookie("utm_source_long"),
UtmMediumShort: getCookie("utm_medium_short"),
UtmMediumMid: getCookie("utm_medium_mid"),
UtmMediumLong: getCookie("utm_medium_long"),
UtmContentShort: getCookie("utm_content_short"),
UtmContentMid: getCookie("utm_content_mid"),
UtmContentLong: getCookie("utm_content_long"),
UtmTermShort: getCookie("utm_term_short"),
UtmTermMid: getCookie("utm_term_mid"),
UtmTermLong: getCookie("utm_term_long"),
GoogleClickID: getCookie("gclid"),
MicrosoftClickID: getCookie("msclkid"),
HasAgreedToToS: tosAccepted == "true",
})
return nil
}
}
Loading
Loading