From 9bc9768ec8b2c30cc4afa8ac8c9476d87450d906 Mon Sep 17 00:00:00 2001 From: Lifosmin Simon Date: Wed, 18 Sep 2024 13:56:34 +0700 Subject: [PATCH] fix: external metrics --- cli/config.go | 6 +++--- cli/root.go | 13 +++++-------- internal/server/config.go | 10 +++++----- internal/server/server.go | 12 +++--------- config/telemetry.go => pkg/opentelemetry/config.go | 4 ++-- pkg/opentelemetry/init.go | 12 ++---------- plugins/providers/bigquery/client.go | 9 ++------- plugins/providers/dataplex/client.go | 4 ---- plugins/providers/gate/provider.go | 2 -- plugins/providers/gcloudiam/client.go | 9 ++------- plugins/providers/gcs/client.go | 4 +--- plugins/providers/gitlab/provider.go | 4 +--- 12 files changed, 26 insertions(+), 63 deletions(-) rename config/telemetry.go => pkg/opentelemetry/config.go (93%) diff --git a/cli/config.go b/cli/config.go index d8991c680..fad80b254 100644 --- a/cli/config.go +++ b/cli/config.go @@ -4,7 +4,7 @@ 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" ) @@ -12,8 +12,8 @@ import ( 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 index f1ad05646..d464590e8 100644 --- 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 index 0f40fca39..464e94896 100644 --- 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 index 8673cc5a0..ff7f77429 100644 --- 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 b/pkg/opentelemetry/config.go similarity index 93% rename from config/telemetry.go rename to pkg/opentelemetry/config.go index edb665541..9910b5739 100644 --- a/config/telemetry.go +++ b/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 index f92edb151..39ea179da 100644 --- 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 index 4bbc1f181..a7fd6dd52 100644 --- 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 index 131900157..6c72c5329 100644 --- 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 index 17c7421f9..6ecf6c766 100644 --- 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 index 21c7ed30e..15933085e 100644 --- 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 index da0f62961..04ba3571f 100644 --- 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 index e3c6c0197..140b571d2 100644 --- 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) }