feat: track external client logs (#183)

* feat: track external client logs * feat: track external client logs * fix bigquery provider test * fix: some test cases * add gitlab otel * add oauth scope for each service * add metric into external client * refactor otel http client * coverage test * coverage test * coverage test * increase metric interval to 15s * fix data race * fix go mod * resolve comments --------- Co-authored-by: Lifosmin Simon <[email protected]>
goto · Nov 1, 2024 · 8f8b145 · 8f8b145
1 parent 8ac4398
commit 8f8b145
Show file tree

Hide file tree

Showing 29 changed files with 417 additions and 72 deletions.
diff --git a/core/appeal/service.go b/core/appeal/service.go
@@ -1578,9 +1578,9 @@ func (s *Service) populateAppealMetadata(ctx context.Context, a *domain.Appeal,
 				if err != nil {
 					return err
 				}
-				clientCreator := &http.HttpClientCreatorStruct{}
-				metadataCl, err := http.NewHTTPClient(&cfg.HTTPClientConfig, clientCreator)
 
+				clientCreator := &http.HttpClientCreatorStruct{}
+				metadataCl, err := http.NewHTTPClient(&cfg.HTTPClientConfig, clientCreator, "AppealMetadata")
 				if err != nil {
 					return fmt.Errorf("key: %s, %w", key, err)
 				}

diff --git a/go.mod b/go.mod
@@ -42,6 +42,7 @@ require (
 	go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.45.0
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.22.0
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.22.0
+	go.opentelemetry.io/otel/metric v1.29.0
 	go.opentelemetry.io/otel/sdk v1.29.0
 	go.opentelemetry.io/otel/sdk/metric v1.27.0
 	golang.org/x/net v0.22.0
@@ -170,7 +171,6 @@ require (
 	github.com/yusufpapurcu/wmi v1.2.4 // indirect
 	github.com/zeebo/xxh3 v1.0.2 // indirect
 	go.opencensus.io v0.24.0 // indirect
-	go.opentelemetry.io/otel/metric v1.29.0 // indirect
 	go.opentelemetry.io/otel/trace v1.29.0 // indirect
 	go.opentelemetry.io/proto/otlp v1.0.0 // indirect
 	go.uber.org/atomic v1.10.0 // indirect

diff --git a/pkg/gate/client.go b/pkg/gate/client.go
@@ -9,6 +9,8 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+
+	"github.com/goto/guardian/pkg/opentelemetry/otelhttpclient"
 )
 
 type Client struct {
@@ -25,7 +27,7 @@ func NewClient(baseURL string, opts ...ClientOption) (*Client, error) {
 	client := &Client{
 		baseURL: url,
 		options: &options{
-			httpClient: http.DefaultClient,
+			httpClient: otelhttpclient.New("GateClient", nil),
 		},
 	}
 	for _, o := range opts {

diff --git a/pkg/http/http.go b/pkg/http/http.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 
 	validator "github.com/go-playground/validator/v10"
+	"github.com/goto/guardian/pkg/opentelemetry/otelhttpclient"
 	defaults "github.com/mcuadros/go-defaults"
 	"golang.org/x/oauth2"
 	"golang.org/x/oauth2/google"
@@ -61,7 +62,7 @@ type HttpClientCreator interface {
 }
 
 // NewHTTPClient returns *iam.Client
-func NewHTTPClient(config *HTTPClientConfig, clientCreator HttpClientCreator) (*HTTPClient, error) {
+func NewHTTPClient(config *HTTPClientConfig, clientCreator HttpClientCreator, serviceName string) (*HTTPClient, error) {
 	defaults.SetDefaults(config)
 	if err := validator.New().Struct(config); err != nil {
 		return nil, err
@@ -70,6 +71,9 @@ func NewHTTPClient(config *HTTPClientConfig, clientCreator HttpClientCreator) (*
 	if httpClient == nil {
 		httpClient = http.DefaultClient
 	}
+	if serviceName != "" {
+		httpClient = otelhttpclient.New(serviceName, nil)
+	}
 
 	if config.Auth != nil && (config.Auth.Type == "google_idtoken" || config.Auth.Type == "google_oauth2") {
 		var creds []byte

diff --git a/pkg/http/http_test.go b/pkg/http/http_test.go
@@ -34,7 +34,7 @@ func TestMakeRequestForGet(t *testing.T) {
 			Value:    "test_value",
 		},
 		Method: "GET",
-	}, nil)
+	}, nil, "metabase")
 	if err != nil {
 		t.Fatalf("Failed to create HTTPClient: %v", err)
 	}
@@ -78,7 +78,7 @@ func TestMakeRequestForPostWithPayload(t *testing.T) {
 		},
 		Method: "POST",
 		Body:   "test",
-	}, nil)
+	}, nil, "metabase")
 	if err != nil {
 		t.Fatalf("Failed to create HTTPClient: %v", err)
 	}
@@ -121,7 +121,7 @@ func TestMakeRequestForPostWithoutPayload(t *testing.T) {
 			Token:    "test_token",
 		},
 		Method: "POST",
-	}, nil)
+	}, nil, "metabase")
 	if err != nil {
 		t.Fatalf("Failed to create HTTPClient: %v", err)
 	}
@@ -167,7 +167,7 @@ func TestMakeRequestForPostWithPayloadForHeaderAuth(t *testing.T) {
 		},
 		Method: "POST",
 		Body:   "test",
-	}, nil)
+	}, nil, "metabase")
 	if err != nil {
 		t.Fatalf("Failed to create HTTPClient: %v", err)
 	}
@@ -210,7 +210,7 @@ func TestMakeRequestForPostWithPayloadForBasicAuth(t *testing.T) {
 		},
 		Method: "POST",
 		Body:   "test",
-	}, nil)
+	}, nil, "metabase")
 	if err != nil {
 		t.Fatalf("Failed to create HTTPClient: %v", err)
 	}
@@ -260,7 +260,7 @@ func TestNewHTTPClient_GoogleOAuth2(t *testing.T) {
 	expectedClient := &http.Client{}
 
 	mockCreator := new(MockHttpClientCreator)
-	mockCreator.On("GetHttpClientForGoogleOAuth2", mock.Anything, []byte(credsJSON)).Return(expectedClient, nil)
+	mockCreator.On("GetHttpClientForGoogleOAuth2", mock.Anything, []byte(credsJSON)).Return(expectedClient, nil, "metabase")
 
 	config := &HTTPClientConfig{
 		URL: "https://example.com",
@@ -270,7 +270,7 @@ func TestNewHTTPClient_GoogleOAuth2(t *testing.T) {
 		},
 	}
 
-	client, err := NewHTTPClient(config, mockCreator)
+	client, err := NewHTTPClient(config, mockCreator, "metabase")
 
 	assert.NoError(t, err)
 	assert.NotNil(t, client)
@@ -288,7 +288,7 @@ func TestNewHTTPClient_GoogleOAuth2WithEmptyCredentialJson(t *testing.T) {
 		},
 	}
 
-	client, err := NewHTTPClient(config, mockCreator)
+	client, err := NewHTTPClient(config, mockCreator, "metabase")
 
 	assert.Error(t, err, "missing credentials for google_idtoken or  google_oauth2 auth")
 	assert.Nil(t, client)
@@ -301,7 +301,7 @@ func TestNewHTTPClient_GoogleIdToken(t *testing.T) {
 	expectedClient := &http.Client{}
 
 	mockCreator := new(MockHttpClientCreator)
-	mockCreator.On("GetHttpClientForGoogleIdToken", mock.Anything, []byte(credsJSON), "audience").Return(expectedClient, nil)
+	mockCreator.On("GetHttpClientForGoogleIdToken", mock.Anything, []byte(credsJSON), "audience").Return(expectedClient, nil, "metabase")
 
 	config := &HTTPClientConfig{
 		URL: "https://example.com",
@@ -312,7 +312,7 @@ func TestNewHTTPClient_GoogleIdToken(t *testing.T) {
 		},
 	}
 
-	client, err := NewHTTPClient(config, mockCreator)
+	client, err := NewHTTPClient(config, mockCreator, "metabase")
 
 	assert.NoError(t, err)
 	assert.NotNil(t, client)
@@ -336,7 +336,7 @@ func TestNewHTTPClient_GoogleIdTokenErrorScenario(t *testing.T) {
 		},
 	}
 
-	_, err := NewHTTPClient(config, mockCreator)
+	_, err := NewHTTPClient(config, mockCreator, "metabase")
 
 	assert.Equal(t, err.Error(), "error creating http client for google_idtoken")
 	mockCreator.AssertExpectations(t)
@@ -358,7 +358,7 @@ func TestNewHTTPClient_GoogleOAuth2ErrorScenario(t *testing.T) {
 		},
 	}
 
-	_, err := NewHTTPClient(config, mockCreator)
+	_, err := NewHTTPClient(config, mockCreator, "metabase")
 
 	assert.Equal(t, err.Error(), "error creating http client for google_oauth2")
 	mockCreator.AssertExpectations(t)

diff --git a/pkg/opentelemetry/config.go b/pkg/opentelemetry/config.go
@@ -13,5 +13,5 @@ type Config struct {
 		Endpoint string            `mapstructure:"endpoint" default:"127.0.0.1:4317"`
 	} `mapstructure:"otlp"`
 	SamplingFraction int           `mapstructure:"sampling_fraction"`
-	MetricInterval   time.Duration `mapstructure:"metric_interval"`
+	MetricInterval   time.Duration `mapstructure:"metric_interval" default:"15s"`
 }
diff --git a/pkg/opentelemetry/init.go b/pkg/opentelemetry/init.go
@@ -3,18 +3,17 @@ package opentelemetry
 import (
 	"context"
 	"fmt"
-	"net/http"
 	"time"
 
 	"go.opentelemetry.io/contrib/instrumentation/host"
-	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 	"go.opentelemetry.io/contrib/instrumentation/runtime"
 	"go.opentelemetry.io/otel"
+
 	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
 	"go.opentelemetry.io/otel/propagation"
-	"go.opentelemetry.io/otel/sdk/metric"
+	sdkMetric "go.opentelemetry.io/otel/sdk/metric"
 	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	semconv "go.opentelemetry.io/otel/semconv/v1.20.0"
@@ -83,8 +82,8 @@ func initGlobalMetrics(ctx context.Context, res *resource.Resource, cfg Config)
 		return nil, fmt.Errorf("create metric exporter: %w", err)
 	}
 
-	reader := metric.NewPeriodicReader(exporter, metric.WithInterval(cfg.MetricInterval))
-	provider := metric.NewMeterProvider(metric.WithReader(reader), metric.WithResource(res))
+	reader := sdkMetric.NewPeriodicReader(exporter, sdkMetric.WithInterval(cfg.MetricInterval))
+	provider := sdkMetric.NewMeterProvider(sdkMetric.WithReader(reader), sdkMetric.WithResource(res))
 	otel.SetMeterProvider(provider)
 
 	return func() error {
@@ -107,6 +106,10 @@ func initGlobalTracer(ctx context.Context, res *resource.Resource, cfg Config) (
 		return nil, fmt.Errorf("create trace exporter: %w", err)
 	}
 
+	if err != nil {
+		return nil, fmt.Errorf("create stdout trace exporter: %w", err)
+	}
+
 	tracerProvider := sdktrace.NewTracerProvider(
 		sdktrace.WithSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithResource(res),
@@ -127,14 +130,3 @@ func initGlobalTracer(ctx context.Context, res *resource.Resource, cfg Config) (
 		return nil
 	}, nil
 }
-
-func NewHttpClient(name string) *http.Client {
-	opts := []otelhttp.Option{
-		otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string {
-			return fmt.Sprintf("%s %s", name, operation)
-		}),
-	}
-	return &http.Client{
-		Transport: otelhttp.NewTransport(http.DefaultTransport, opts...),
-	}
-}
diff --git a/pkg/opentelemetry/otelhttpclient/annotations.go b/pkg/opentelemetry/otelhttpclient/annotations.go
@@ -0,0 +1,35 @@
+package otelhttpclient
+
+import (
+	"context"
+	"net/http"
+
+	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
+	"go.opentelemetry.io/otel/attribute"
+)
+
+type labelerContextKeyType int
+
+const lablelerContextKey labelerContextKeyType = 0
+
+// AnnotateRequest adds telemetry related annotations to request context and returns.
+// The request context on the returned request should be retained.
+// Ensure `route` is a route template and not actual URL to prevent high cardinality
+// on the metrics.
+func AnnotateRequest(req *http.Request, route string) *http.Request {
+	ctx := req.Context()
+
+	l := &otelhttp.Labeler{}
+	l.Add(attribute.String(attributeHTTPRoute, route))
+
+	return req.WithContext(context.WithValue(ctx, lablelerContextKey, l))
+}
+
+// LabelerFromContext returns the labeler annotation from the context if exists.
+func LabelerFromContext(ctx context.Context) (*otelhttp.Labeler, bool) {
+	l, ok := ctx.Value(lablelerContextKey).(*otelhttp.Labeler)
+	if !ok {
+		l = &otelhttp.Labeler{}
+	}
+	return l, ok
+}
diff --git a/pkg/opentelemetry/otelhttpclient/client.go b/pkg/opentelemetry/otelhttpclient/client.go
@@ -0,0 +1,15 @@
+package otelhttpclient
+
+import (
+	"net/http"
+)
+
+func New(name string, client *http.Client) *http.Client {
+	if client == nil {
+		return &http.Client{
+			Transport: NewHTTPTransport(nil, name),
+		}
+	}
+	client.Transport = NewHTTPTransport(client.Transport, name)
+	return client
+}