Skip to content

Commit 0f5c50c

Browse files
stephan-raynerstephan-rayner
authored
maint: Standardize Collector Config Structs, Constructors, and Error Handling (#914)
1 parent 0d74000 commit 0f5c50c

35 files changed

Lines changed: 227 additions & 254 deletions

‎cmd/exporter/config/config.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type Config struct {
2525
}
2626
Azure struct {
2727
Services StringSliceFlag
28-
SubscriptionId string
28+
SubscriptionID string
2929
Region string
3030
}
3131
}

‎cmd/exporter/exporter.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func providerFlags(fs *flag.FlagSet, cfg *config.Config) {
8181
flag.StringVar(&cfg.Providers.AWS.RoleARN, "aws.roleARN", "", "Optional AWS role ARN to assume for cross-account access.")
8282
// TODO - PUT PROJECT-ID UNDER GCP
8383
flag.StringVar(&cfg.ProjectID, "project-id", "", "Project ID to target.")
84-
flag.StringVar(&cfg.Providers.Azure.SubscriptionId, "azure.subscription-id", "", "Azure subscription ID to pull data from.")
84+
flag.StringVar(&cfg.Providers.Azure.SubscriptionID, "azure.subscription-id", "", "Azure subscription ID to pull data from.")
8585
flag.IntVar(&cfg.Providers.GCP.DefaultGCSDiscount, "gcp.default-discount", 19, "GCP default discount")
8686
}
8787

@@ -241,7 +241,7 @@ func selectProviderWith(
241241
case "azure":
242242
return newAzure(ctx, &azure.Config{
243243
Logger: cfg.Logger,
244-
SubscriptionId: cfg.Providers.Azure.SubscriptionId,
244+
SubscriptionID: cfg.Providers.Azure.SubscriptionID,
245245
ScrapeInterval: cfg.Collector.ScrapeInterval,
246246
Services: strings.Split(cfg.Providers.Azure.Services.String(), ","),
247247
CollectorTimeout: collectorTimeout,

‎pkg/aws/aws.go‎

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,10 @@ func newWithDependencies(ctx context.Context, config *Config, awsClient client.C
157157

158158
switch service {
159159
case serviceS3:
160-
collector, err := s3.New(ctx, config.ScrapeInterval, awsClient, config.AccountID)
160+
collector, err := s3.New(ctx, &s3.Config{
161+
ScrapeInterval: config.ScrapeInterval,
162+
AccountID: config.AccountID,
163+
}, logger, awsClient)
161164
if err != nil {
162165
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
163166
slog.String("service", service),
@@ -168,11 +171,10 @@ func newWithDependencies(ctx context.Context, config *Config, awsClient client.C
168171
case serviceEC2:
169172
collector, err := ec2Collector.New(ctx, &ec2Collector.Config{
170173
Regions: regions,
171-
Logger: logger,
172174
ScrapeInterval: config.ScrapeInterval,
173175
RegionMap: regionClients,
174176
AccountID: config.AccountID,
175-
})
177+
}, logger)
176178
if err != nil {
177179
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
178180
slog.String("service", service),
@@ -181,26 +183,40 @@ func newWithDependencies(ctx context.Context, config *Config, awsClient client.C
181183
}
182184
collectors = append(collectors, collector)
183185
case serviceRDS:
186+
// pricing API for RDS client needs to use always the same region
187+
// as for RDS, the pricing data is only available in the us-east-1
188+
pricingConfig, err := createAWSConfig(ctx, "us-east-1", config.Profile, config.RoleARN)
189+
if err != nil {
190+
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
191+
slog.String("service", service),
192+
slog.String("message", err.Error()))
193+
continue
194+
}
184195
awsRDSClient := client.NewAWSClient(client.Config{
185196
PricingService: awsPricing.NewFromConfig(pricingConfig),
186197
RDSService: rds.NewFromConfig(awsConfig),
187198
})
188-
collector := rdsCollector.New(&rdsCollector.Config{
199+
collector, err := rdsCollector.New(ctx, &rdsCollector.Config{
189200
ScrapeInterval: config.ScrapeInterval,
190201
Regions: regions,
191202
RegionMap: regionClients,
192203
Client: awsRDSClient,
193204
AccountID: config.AccountID,
194-
})
205+
}, logger)
206+
if err != nil {
207+
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
208+
slog.String("service", service),
209+
slog.String("message", err.Error()))
210+
continue
211+
}
195212
collectors = append(collectors, collector)
196213
case serviceNATGW:
197214
collector, err := awsgwnat.New(ctx, &awsgwnat.Config{
198215
ScrapeInterval: config.ScrapeInterval,
199-
Logger: logger,
200216
Regions: regions,
201217
RegionMap: regionClients,
202218
AccountID: config.AccountID,
203-
})
219+
}, logger)
204220
if err != nil {
205221
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
206222
slog.String("service", service),
@@ -209,30 +225,52 @@ func newWithDependencies(ctx context.Context, config *Config, awsClient client.C
209225
}
210226
collectors = append(collectors, collector)
211227
case serviceELB:
228+
// pricing API for ELB client needs to use always the same region
229+
// as the pricing data is only available in us-east-1
230+
elbPricingConfig, err := createAWSConfig(ctx, "us-east-1", config.Profile, config.RoleARN)
231+
if err != nil {
232+
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
233+
slog.String("service", service),
234+
slog.String("message", err.Error()))
235+
continue
236+
}
212237
awsELBPricingClient := client.NewAWSClient(client.Config{
213-
PricingService: awsPricing.NewFromConfig(pricingConfig),
238+
PricingService: awsPricing.NewFromConfig(elbPricingConfig),
214239
})
215-
collector := elb.New(&elb.Config{
240+
collector, err := elb.New(ctx, &elb.Config{
216241
Regions: regions,
217242
PricingClient: awsELBPricingClient,
218-
RegionClients: regionClients,
243+
RegionMap: regionClients,
219244
ScrapeInterval: config.ScrapeInterval,
220-
Logger: logger,
221245
AccountID: config.AccountID,
222-
})
246+
}, logger)
247+
if err != nil {
248+
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
249+
slog.String("service", service),
250+
slog.String("message", err.Error()))
251+
continue
252+
}
223253
collectors = append(collectors, collector)
224254
case serviceVPC:
255+
// pricing API for VPC client needs to use always the same region
256+
// as for VPC, the pricing data is only available in the us-east-1
257+
pricingConfig, err := createAWSConfig(ctx, "us-east-1", config.Profile, config.RoleARN)
258+
if err != nil {
259+
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
260+
slog.String("service", service),
261+
slog.String("message", err.Error()))
262+
continue
263+
}
225264
awsVPCClient := client.NewAWSClient(client.Config{
226265
PricingService: awsPricing.NewFromConfig(pricingConfig),
227266
EC2Service: ec2.NewFromConfig(pricingConfig),
228267
})
229268
collector, err := awsvpc.New(ctx, &awsvpc.Config{
230269
ScrapeInterval: config.ScrapeInterval,
231-
Logger: logger,
232270
Regions: regions,
233271
Client: awsVPCClient,
234272
AccountID: config.AccountID,
235-
})
273+
}, logger)
236274
if err != nil {
237275
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
238276
slog.String("service", service),
@@ -248,9 +286,8 @@ func newWithDependencies(ctx context.Context, config *Config, awsClient client.C
248286
Regions: regions,
249287
RegionMap: regionClients,
250288
Client: awsMSKClient,
251-
Logger: logger,
252289
AccountID: config.AccountID,
253-
})
290+
}, logger)
254291
if err != nil {
255292
logger.LogAttrs(ctx, slog.LevelError, "Error creating collector",
256293
slog.String("service", service),

‎pkg/aws/ec2/ec2.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,13 @@ type Collector struct {
7676
type Config struct {
7777
ScrapeInterval time.Duration
7878
Regions []ec2Types.Region
79-
Logger *slog.Logger
8079
RegionMap map[string]client.Client
8180
AccountID string
8281
}
8382

8483
// New creates an ec2 collector
85-
func New(ctx context.Context, config *Config) (*Collector, error) {
86-
logger := config.Logger.With("logger", "ec2")
84+
func New(ctx context.Context, config *Config, logger *slog.Logger) (*Collector, error) {
85+
logger = logger.With("collector", "ec2")
8786
computeMap := NewComputePricingMap(logger, config)
8887
storageMap := NewStoragePricingMap(logger, config)
8988

‎pkg/aws/ec2/ec2_test.go‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,9 @@ var (
2828
func TestCollector_Name(t *testing.T) {
2929
t.Run("Name should return the same name as the subsystem const", func(t *testing.T) {
3030
collector, err := New(context.Background(), &Config{
31-
Logger: logger,
3231
ScrapeInterval: time.Minute,
3332
AccountID: "123456789012",
34-
})
33+
}, logger)
3534
require.NoError(t, err)
3635
assert.Equal(t, subsystem, collector.Name())
3736
})
@@ -42,11 +41,10 @@ func TestNew(t *testing.T) {
4241
t.Run("New should return ClientNotFound error when RegionMap is empty", func(t *testing.T) {
4342
_, err := New(context.Background(), &Config{
4443
Regions: regions,
45-
Logger: logger,
4644
ScrapeInterval: time.Minute,
4745
AccountID: "123456789012",
4846
RegionMap: map[string]client.Client{}, // Empty map - no client
49-
})
47+
}, logger)
5048
assert.ErrorIs(t, err, ErrClientNotFound)
5149
})
5250

@@ -57,13 +55,12 @@ func TestNew(t *testing.T) {
5755

5856
_, err := New(context.Background(), &Config{
5957
Regions: regions,
60-
Logger: logger,
6158
ScrapeInterval: time.Minute,
6259
AccountID: "123456789012",
6360
RegionMap: map[string]client.Client{
6461
"us-east-1": mock,
6562
},
66-
})
63+
}, logger)
6764
assert.Error(t, err)
6865
assert.ErrorIs(t, err, ErrListOnDemandPrices)
6966
})
@@ -79,13 +76,12 @@ func TestNew(t *testing.T) {
7976

8077
_, err := New(context.Background(), &Config{
8178
Regions: regions,
82-
Logger: logger,
8379
ScrapeInterval: time.Minute,
8480
AccountID: "123456789012",
8581
RegionMap: map[string]client.Client{
8682
"us-east-1": mock,
8783
},
88-
})
84+
}, logger)
8985
assert.Error(t, err)
9086
assert.ErrorIs(t, err, ErrListStoragePrices)
9187
})
@@ -103,13 +99,12 @@ func TestNew(t *testing.T) {
10399

104100
collector, err := New(context.Background(), &Config{
105101
Regions: regions,
106-
Logger: logger,
107102
ScrapeInterval: time.Minute,
108103
AccountID: "123456789012",
109104
RegionMap: map[string]client.Client{
110105
"us-east-1": mock,
111106
},
112-
})
107+
}, logger)
113108
require.NoError(t, err)
114109
assert.NotNil(t, collector)
115110

@@ -128,10 +123,9 @@ func TestCollector_Collect(t *testing.T) {
128123
}
129124
t.Run("Collect should return no error", func(t *testing.T) {
130125
collector, err := New(context.Background(), &Config{
131-
Logger: logger,
132126
ScrapeInterval: time.Minute,
133127
AccountID: "123456789012",
134-
})
128+
}, logger)
135129
require.NoError(t, err)
136130
ch := make(chan prometheus.Metric)
137131
go func() {
@@ -234,13 +228,12 @@ func TestCollector_Collect(t *testing.T) {
234228

235229
collector, err := New(ctx, &Config{
236230
Regions: regions,
237-
Logger: logger,
238231
ScrapeInterval: time.Minute,
239232
AccountID: "123456789012",
240233
RegionMap: map[string]client.Client{
241234
"us-east-1": c,
242235
},
243-
})
236+
}, logger)
244237
require.NoError(t, err)
245238

246239
ch := make(chan prometheus.Metric)
@@ -379,13 +372,12 @@ func Test_FetchVolumesData(t *testing.T) {
379372

380373
collector, err := New(context.Background(), &Config{
381374
Regions: []ec2Types.Region{region},
382-
Logger: logger,
383375
ScrapeInterval: time.Minute,
384376
AccountID: "123456789012",
385377
RegionMap: map[string]client.Client{
386378
regionName: c,
387379
},
388-
})
380+
}, logger)
389381
require.NoError(t, err)
390382

391383
c.EXPECT().
@@ -434,13 +426,12 @@ func Test_EmitMetricsFromVolumesChannel(t *testing.T) {
434426

435427
collector, err := New(context.Background(), &Config{
436428
Regions: []ec2Types.Region{region},
437-
Logger: logger,
438429
ScrapeInterval: time.Minute,
439430
AccountID: "123456789012",
440431
RegionMap: map[string]client.Client{
441432
regionName: c,
442433
},
443-
})
434+
}, logger)
444435
require.NoError(t, err)
445436

446437
collector.storagePricingMap = &StoragePricingMap{

‎pkg/aws/elb/elb.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ type Config struct {
5757
// PricingClient must be a client configured for us-east-1: the AWS Pricing API
5858
// is only available in us-east-1 and ap-south-1.
5959
PricingClient client.Client
60-
RegionClients map[string]client.Client
61-
Logger *slog.Logger
60+
RegionMap map[string]client.Client
6261
AccountID string
6362
}
6463

@@ -87,17 +86,17 @@ type elbProduct struct {
8786
}
8887
}
8988

90-
func New(config *Config) *Collector {
91-
logger := config.Logger.With("logger", serviceName)
89+
func New(_ context.Context, config *Config, logger *slog.Logger) (*Collector, error) {
90+
logger = logger.With("collector", serviceName)
9291
return &Collector{
9392
regions: config.Regions,
9493
ScrapeInterval: config.ScrapeInterval,
9594
pricingClient: config.PricingClient,
96-
awsRegionClientMap: config.RegionClients,
95+
awsRegionClientMap: config.RegionMap,
9796
logger: logger,
9897
pricingMap: NewELBPricingMap(logger),
9998
accountID: config.AccountID,
100-
}
99+
}, nil
101100
}
102101

103102
func (c *Collector) Register(_ provider.Registry) error {

0 commit comments

Comments
 (0)