fix: external metrics

goto · Sep 18, 2024 · 9bc9768 · 9bc9768
1 parent d2cfe3f
commit 9bc9768
Show file tree

Hide file tree

Showing 12 changed files with 26 additions and 63 deletions.
diff --git a/cli/config.go b/cli/config.go
@@ -4,16 +4,16 @@ import (
 	"fmt"
 
 	"github.com/MakeNowJust/heredoc"
-	"github.com/goto/guardian/config"
+	"github.com/goto/guardian/pkg/opentelemetry"
 	"github.com/goto/salt/cmdx"
 	"github.com/spf13/cobra"
 )
 
 var cliConfig *Config
 
 type Config struct {
-	Host      string                     `mapstructure:"host"`
-	Telemetry config.OpenTelemetryConfig `mapstructure:"telemetry"`
+	Host      string               `mapstructure:"host"`
+	Telemetry opentelemetry.Config `mapstructure:"telemetry"`
 }
 
 func LoadConfig() (*Config, error) {

diff --git a/cli/root.go b/cli/root.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"log"
 
 	"github.com/MakeNowJust/heredoc"
 	handlerv1beta1 "github.com/goto/guardian/api/handler/v1beta1"
@@ -36,20 +37,16 @@ func New(cfg *Config) *cobra.Command {
 
 			if cliConfig.Telemetry.Enabled {
 				var err error
-				shutdownOtel, err = opentelemetry.Init(ctx, opentelemetry.Config{
-					ServiceName:      cfg.Telemetry.ServiceName,
-					ServiceVersion:   cfg.Telemetry.ServiceVersion,
-					SamplingFraction: cfg.Telemetry.SamplingFraction,
-					MetricInterval:   cfg.Telemetry.MetricInterval,
-					CollectorAddr:    cfg.Telemetry.OTLP.Endpoint,
-				})
+				shutdownOtel, err = opentelemetry.Init(ctx, cfg.Telemetry)
 				if err != nil {
 					return err
 				}
 			}
 
 			defer func() {
-				_ = shutdownOtel() // safely ignore the error if telemetry is disabled
+				if err := shutdownOtel(); err != nil {
+					log.Printf("telemetry is disabled: %v", err)
+				}
 			}()
 
 			return nil

diff --git a/internal/server/config.go b/internal/server/config.go
@@ -4,10 +4,10 @@ import (
 	"errors"
 	"fmt"
 
-	gConfig "github.com/goto/guardian/config"
 	"github.com/goto/guardian/internal/store"
 	"github.com/goto/guardian/jobs"
 	"github.com/goto/guardian/pkg/auth"
+	"github.com/goto/guardian/pkg/opentelemetry"
 	"github.com/goto/guardian/plugins/notifiers"
 	"github.com/goto/salt/config"
 )
@@ -41,10 +41,10 @@ type Config struct {
 	LogLevel string           `mapstructure:"log_level" default:"info"`
 	DB       store.Config     `mapstructure:"db"`
 	// Deprecated: use Auth.Default.HeaderKey instead note on the AuthenticatedUserHeaderKey
-	AuthenticatedUserHeaderKey string                      `mapstructure:"authenticated_user_header_key"`
-	AuditLogTraceIDHeaderKey   string                      `mapstructure:"audit_log_trace_id_header_key" default:"X-Trace-Id"`
-	Jobs                       Jobs                        `mapstructure:"jobs"`
-	OpenTelemetry              gConfig.OpenTelemetryConfig `mapstructure:"opentelemetry"`
+	AuthenticatedUserHeaderKey string               `mapstructure:"authenticated_user_header_key"`
+	AuditLogTraceIDHeaderKey   string               `mapstructure:"audit_log_trace_id_header_key" default:"X-Trace-Id"`
+	Jobs                       Jobs                 `mapstructure:"jobs"`
+	Telemetry                  opentelemetry.Config `mapstructure:"telemetry"`
 
 	Auth Auth `mapstructure:"auth"`
 }

diff --git a/internal/server/server.go b/internal/server/server.go
@@ -87,15 +87,9 @@ func RunServer(config *Config) error {
 	ctx := context.Background()
 
 	// var shutdownOtel = func() error { return nil }
-	if config.OpenTelemetry.Enabled {
+	if config.Telemetry.Enabled {
 		logger.Info(ctx, "open telemetry is initiating...")
-		shutdownOtel, err := opentelemetry.Init(ctx, opentelemetry.Config{
-			ServiceName:      config.OpenTelemetry.ServiceName,
-			ServiceVersion:   config.OpenTelemetry.ServiceVersion,
-			SamplingFraction: config.OpenTelemetry.SamplingFraction,
-			MetricInterval:   config.OpenTelemetry.MetricInterval,
-			CollectorAddr:    config.OpenTelemetry.OTLP.Endpoint,
-		})
+		shutdownOtel, err := opentelemetry.Init(ctx, config.Telemetry)
 		if err != nil {
 			return fmt.Errorf("error initiating open telemetry: %w", err)
 		}
@@ -240,7 +234,7 @@ func Migrate(c *Config) error {
 
 func getStore(c *Config) (*postgres.Store, error) {
 	store, err := postgres.NewStore(&c.DB)
-	if c.OpenTelemetry.Enabled {
+	if c.Telemetry.Enabled {
 		sqlDB, err := store.DB().DB()
 		if err != nil {
 			return nil, err

diff --git a/config/telemetry.go → pkg/opentelemetry/config.go b/config/telemetry.go → pkg/opentelemetry/config.go
@@ -1,8 +1,8 @@
-package config
+package opentelemetry
 
 import "time"
 
-type OpenTelemetryConfig struct {
+type Config struct {
 	Enabled        bool              `mapstructure:"enabled" default:"false"`
 	ServiceName    string            `mapstructure:"service_name" default:"guardian"`
 	ServiceVersion string            `mapstructure:"service_version"`

diff --git a/pkg/opentelemetry/init.go b/pkg/opentelemetry/init.go
@@ -23,14 +23,6 @@ import (
 
 const gracePeriod = 5 * time.Second
 
-type Config struct {
-	ServiceName      string
-	ServiceVersion   string
-	SamplingFraction int
-	MetricInterval   time.Duration
-	CollectorAddr    string
-}
-
 func Init(ctx context.Context, cfg Config) (func() error, error) {
 	res, err := resource.New(ctx,
 		resource.WithFromEnv(),
@@ -83,7 +75,7 @@ func Init(ctx context.Context, cfg Config) (func() error, error) {
 
 func initGlobalMetrics(ctx context.Context, res *resource.Resource, cfg Config) (func() error, error) {
 	exporter, err := otlpmetricgrpc.New(ctx,
-		otlpmetricgrpc.WithEndpoint(cfg.CollectorAddr),
+		otlpmetricgrpc.WithEndpoint(cfg.OTLP.Endpoint),
 		otlpmetricgrpc.WithCompressor(gzip.Name),
 		otlpmetricgrpc.WithInsecure(),
 	)
@@ -108,7 +100,7 @@ func initGlobalMetrics(ctx context.Context, res *resource.Resource, cfg Config)
 func initGlobalTracer(ctx context.Context, res *resource.Resource, cfg Config) (func() error, error) {
 	exporter, err := otlptrace.New(ctx, otlptracegrpc.NewClient(
 		otlptracegrpc.WithInsecure(),
-		otlptracegrpc.WithEndpoint(cfg.CollectorAddr),
+		otlptracegrpc.WithEndpoint(cfg.OTLP.Endpoint),
 		otlptracegrpc.WithCompressor(gzip.Name),
 	))
 	if err != nil {

diff --git a/plugins/providers/bigquery/client.go b/plugins/providers/bigquery/client.go
@@ -8,7 +8,6 @@ import (
 
 	bq "cloud.google.com/go/bigquery"
 	"github.com/goto/guardian/domain"
-	"github.com/goto/guardian/pkg/opentelemetry"
 	bqApi "google.golang.org/api/bigquery/v2"
 	"google.golang.org/api/cloudresourcemanager/v1"
 	"google.golang.org/api/iam/v1"
@@ -36,16 +35,12 @@ func NewBigQueryClient(projectID string, opts ...option.ClientOption) (*bigQuery
 		return nil, err
 	}
 
-	iamOpt := option.WithHTTPClient(opentelemetry.NewHttpClient("IAMClient"))
-	iamOpts := append(opts, iamOpt)
-	iamService, err := iam.NewService(ctx, iamOpts...)
+	iamService, err := iam.NewService(ctx, opts...)
 	if err != nil {
 		return nil, err
 	}
 
-	crmOpt := option.WithHTTPClient(opentelemetry.NewHttpClient("CloudResourceManagerClient"))
-	crmOpts := append(opts, crmOpt)
-	crmService, err := cloudresourcemanager.NewService(ctx, crmOpts...)
+	crmService, err := cloudresourcemanager.NewService(ctx, opts...)
 	if err != nil {
 		return nil, err
 	}

diff --git a/plugins/providers/dataplex/client.go b/plugins/providers/dataplex/client.go
@@ -7,12 +7,10 @@ import (
 
 	datacatalog "cloud.google.com/go/datacatalog/apiv1"
 	"github.com/goto/guardian/domain"
-	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
 	"google.golang.org/api/iterator"
 	"google.golang.org/api/option"
 	pb "google.golang.org/genproto/googleapis/cloud/datacatalog/v1"
 	iampb "google.golang.org/genproto/googleapis/iam/v1"
-	"google.golang.org/grpc"
 )
 
 type policyTagClient struct {
@@ -25,8 +23,6 @@ func newPolicyTagClient(projectID, location string, credentialsJSON []byte) (*po
 	ctx := context.Background()
 	opts := []option.ClientOption{
 		option.WithCredentialsJSON(credentialsJSON),
-		option.WithGRPCDialOption(grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor())),
-		option.WithGRPCDialOption(grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor())),
 	}
 	policyManager, err := datacatalog.NewPolicyTagManagerClient(ctx, opts...)
 	if err != nil {

diff --git a/plugins/providers/gate/provider.go b/plugins/providers/gate/provider.go
@@ -12,7 +12,6 @@ import (
 	pv "github.com/goto/guardian/core/provider"
 	"github.com/goto/guardian/domain"
 	"github.com/goto/guardian/pkg/gate"
-	"github.com/goto/guardian/pkg/opentelemetry"
 	"github.com/goto/guardian/utils"
 	"github.com/mitchellh/mapstructure"
 )
@@ -281,7 +280,6 @@ func (p *provider) getClient(pc *domain.ProviderConfig) (*gate.Client, error) {
 	opts := []gate.ClientOption{
 		gate.WithAPIKey(creds.APIKey),
 		gate.WithQueryParamAuthMethod(),
-		gate.WithHTTPClient(opentelemetry.NewHttpClient("PolicyTagManagerClient")),
 	}
 
 	client, err := gate.NewClient(creds.Host, opts...)

diff --git a/plugins/providers/gcloudiam/client.go b/plugins/providers/gcloudiam/client.go
@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/goto/guardian/domain"
-	"github.com/goto/guardian/pkg/opentelemetry"
 	"google.golang.org/api/cloudresourcemanager/v1"
 	"google.golang.org/api/iam/v1"
 	"google.golang.org/api/option"
@@ -26,16 +25,12 @@ type iamClient struct {
 func newIamClient(credentialsJSON []byte, resourceName string) (*iamClient, error) {
 	ctx := context.Background()
 
-	crmClient := opentelemetry.NewHttpClient("CloudResourceManagerClient")
-
-	cloudResourceManagerService, err := cloudresourcemanager.NewService(ctx, option.WithCredentialsJSON(credentialsJSON), option.WithHTTPClient(crmClient))
+	cloudResourceManagerService, err := cloudresourcemanager.NewService(ctx, option.WithCredentialsJSON(credentialsJSON))
 	if err != nil {
 		return nil, err
 	}
 
-	iamHTTPClient := opentelemetry.NewHttpClient("IAMClient")
-
-	iamService, err := iam.NewService(ctx, option.WithCredentialsJSON(credentialsJSON), option.WithHTTPClient(iamHTTPClient))
+	iamService, err := iam.NewService(ctx, option.WithCredentialsJSON(credentialsJSON))
 	if err != nil {
 		return nil, err
 	}

diff --git a/plugins/providers/gcs/client.go b/plugins/providers/gcs/client.go
@@ -8,7 +8,6 @@ import (
 	"cloud.google.com/go/iam"
 	"cloud.google.com/go/storage"
 	"github.com/goto/guardian/domain"
-	"github.com/goto/guardian/pkg/opentelemetry"
 	"github.com/goto/guardian/utils"
 	"golang.org/x/sync/errgroup"
 	"google.golang.org/api/iterator"
@@ -21,8 +20,7 @@ type gcsClient struct {
 }
 
 func newGCSClient(ctx context.Context, projectID string, credentialsJSON []byte) (*gcsClient, error) {
-	httpClient := opentelemetry.NewHttpClient("MetabaseHttpClient")
-	client, err := storage.NewClient(ctx, option.WithCredentialsJSON(credentialsJSON), option.WithHTTPClient(httpClient))
+	client, err := storage.NewClient(ctx, option.WithCredentialsJSON(credentialsJSON))
 	if err != nil {
 		return nil, err
 	}

diff --git a/plugins/providers/gitlab/provider.go b/plugins/providers/gitlab/provider.go
@@ -10,7 +10,6 @@ import (
 	pv "github.com/goto/guardian/core/provider"
 	"github.com/goto/guardian/domain"
 	"github.com/goto/guardian/pkg/log"
-	"github.com/goto/guardian/pkg/opentelemetry"
 	"github.com/goto/guardian/utils"
 	"github.com/xanzy/go-gitlab"
 	"golang.org/x/sync/errgroup"
@@ -272,9 +271,8 @@ func (p *provider) getClient(pc domain.ProviderConfig) (*gitlab.Client, error) {
 		return nil, fmt.Errorf("unable to decrypt credentials: %w", err)
 	}
 
-	httpClient := opentelemetry.NewHttpClient("GitlabClient")
 	// add trace for otel instrumentation
-	client, err := gitlab.NewClient(creds.AccessToken, gitlab.WithBaseURL(creds.Host), gitlab.WithHTTPClient(httpClient))
+	client, err := gitlab.NewClient(creds.AccessToken, gitlab.WithBaseURL(creds.Host))
 	if err != nil {
 		return nil, fmt.Errorf("unable to create gitlab client: %w", err)
 	}